From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56484) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRTH9-00024i-Pw for qemu-devel@nongnu.org; Fri, 27 Feb 2015 17:20:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YRTH6-0005LT-Jt for qemu-devel@nongnu.org; Fri, 27 Feb 2015 17:20:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50469) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRTH6-0005LF-DN for qemu-devel@nongnu.org; Fri, 27 Feb 2015 17:20:48 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1RMKk9Q032465 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Fri, 27 Feb 2015 17:20:47 -0500 Message-ID: <54F0EDBC.7020206@redhat.com> Date: Fri, 27 Feb 2015 17:20:44 -0500 From: Max Reitz MIME-Version: 1.0 References: <1423509950-7468-1-git-send-email-mreitz@redhat.com> In-Reply-To: <1423509950-7468-1-git-send-email-mreitz@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 00/12] qcow2: Add new overlap check functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi Ping On 2015-02-09 at 14:25, Max Reitz wrote: > As has been requested, this series adds new overlap check functions to > the qcow2 code. My local branch is called "qcow2-improved-overlap-v1", > but I am not so sure whether it is actually an improvement; that is left > for you to decide, dear reviewers. > > See patch 1 for an explanation of why this series exists and what it > does. Patch 1 is basically the core of this series, the rest just > employs the functions introduced there. > > In a later patch, we may want to change the meaning of the "constant" > overlap checking option to mean the same as "cached", which is > everything except for inactive L2 tables. This series does make > checking for overlaps with inactive L2 tables at runtime just as cheap > as everything else (constant time plus caching), but using these checks > means qemu has to read all the snapshot L1 tables when opening a qcow2 > file. This does not take long, of course, but it does result in a bit of > overhead so I did not want to enable it by default. > > I think just enabling all overlap checks by default after this series > should be fine and useful, though. > > > For benchmarks, please see my cover letter for v2: > http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg03430.html > > I'll just quote the (fixed) tl;dr here: > > * CPU usage at runtime decreased by 15000 to 27500 percent on > overlap-check-heavy tasks > * No additional performance problems at loading time (in theory has the > same runtime complexity as a single overlap check right now; in > practice I could not find any problems) > * Decent RAM usage (170 kB for a 1 TB image with 64 kB clusters; 170 MB > for a 1 PB image etc. pp.) > > > v3: > - Patch 1: > - s/few memory/little memory/ [Eric] > - s/2014/2015/ [Eric] > - s/of a or a/of or a/ [Eric] > - Use nb_clusters_minus_one instead of nb_clusters, because > nb_clusters == 0 is invalid [Eric] > - Drop cached_windows_index [Stefan] > - Reset current_nb_clusters to 1 when a new range starts in the > algorithm which rebuilds the fragment list from the bitmap > representation > - Patch 3: Added a TODO comment that the user should be able to override > the default cache size limit > - Patch 8: As the patch "qcow2: Buffer L1 table in snapshot refcount > update" has been dropped, this patch was changed to no longer rely on > it > - Patch 11: Replaced the size limit for a snapshot's L1 table by > QCOW_MAX_L1_SIZE (was: INT_MAX / sizeof(uint64_t)) [Eric] > > > git-backport-diff against v2: > > Key: > [----] : patches are identical > [####] : number of functional differences between upstream/downstream patch > [down] : patch is downstream-only > The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively > > 001/12:[0026] [FC] 'qcow2: Add new overlap check functions' > 002/12:[----] [--] 'qcow2: Pull up overlap check option evaluation' > 003/12:[0001] [FC] 'qcow2: Create metadata list' > 004/12:[----] [--] 'qcow2/overlaps: Protect image header' > 005/12:[----] [--] 'qcow2/overlaps: Protect refcount table' > 006/12:[----] [--] 'qcow2/overlaps: Protect refcount blocks' > 007/12:[----] [--] 'qcow2/overlaps: Protect active L1 table' > 008/12:[0002] [FC] 'qcow2/overlaps: Protect active L2 tables' > 009/12:[----] [--] 'qcow2/overlaps: Protect snapshot table' > 010/12:[----] [--] 'qcow2/overlaps: Protect inactive L1 tables' > 011/12:[0002] [FC] 'qcow2/overlaps: Protect inactive L2 tables' > 012/12:[----] [--] 'qcow2: Use new metadata overlap check function' > > > Max Reitz (12): > qcow2: Add new overlap check functions > qcow2: Pull up overlap check option evaluation > qcow2: Create metadata list > qcow2/overlaps: Protect image header > qcow2/overlaps: Protect refcount table > qcow2/overlaps: Protect refcount blocks > qcow2/overlaps: Protect active L1 table > qcow2/overlaps: Protect active L2 tables > qcow2/overlaps: Protect snapshot table > qcow2/overlaps: Protect inactive L1 tables > qcow2/overlaps: Protect inactive L2 tables > qcow2: Use new metadata overlap check function > > block/Makefile.objs | 3 +- > block/qcow2-cluster.c | 13 ++ > block/qcow2-overlap.c | 396 +++++++++++++++++++++++++++++++++++++++++++++++++ > block/qcow2-refcount.c | 202 ++++++++++--------------- > block/qcow2-snapshot.c | 105 ++++++++++++- > block/qcow2.c | 131 ++++++++++------ > block/qcow2.h | 13 ++ > 7 files changed, 691 insertions(+), 172 deletions(-) > create mode 100644 block/qcow2-overlap.c