From nobody Thu May  1 22:08:24 2025
Delivered-To: importer@patchew.org
Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as
 permitted sender) client-ip=208.118.235.17;
 envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org;
 helo=lists.gnu.org;
Authentication-Results: mx.zohomail.com;
	spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted
 sender)  smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org
Return-Path: <qemu-devel-bounces+importer=patchew.org@nongnu.org>
Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by
 mx.zohomail.com
	with SMTPS id 1509024037438977.4948867476887;
 Thu, 26 Oct 2017 06:20:37 -0700 (PDT)
Received: from localhost ([::1]:52860 helo=lists.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <qemu-devel-bounces+importer=patchew.org@nongnu.org>)
	id 1e7i5A-0001g7-2N
	for importer@patchew.org; Thu, 26 Oct 2017 09:20:24 -0400
Received: from eggs.gnu.org ([2001:4830:134:3::10]:58544)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1e7i2x-0000HA-En
	for qemu-devel@nongnu.org; Thu, 26 Oct 2017 09:18:12 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1e7i2v-0001ky-Q0
	for qemu-devel@nongnu.org; Thu, 26 Oct 2017 09:18:07 -0400
Received: from mx1.redhat.com ([209.132.183.28]:48207)
	by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)
	(Exim 4.71) (envelope-from <kwolf@redhat.com>)
	id 1e7i2n-0001by-La; Thu, 26 Oct 2017 09:17:57 -0400
Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com
	[10.5.11.12])
	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
	(No client certificate requested)
	by mx1.redhat.com (Postfix) with ESMTPS id C1C8B6868A;
	Thu, 26 Oct 2017 13:17:56 +0000 (UTC)
Received: from localhost.localdomain.com (unknown [10.36.118.24])
	by smtp.corp.redhat.com (Postfix) with ESMTP id 2DDF47FB80;
	Thu, 26 Oct 2017 13:17:54 +0000 (UTC)
DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C1C8B6868A
Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com;
	dmarc=none (p=none dis=none) header.from=redhat.com
Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com;
	spf=fail smtp.mailfrom=kwolf@redhat.com
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Date: Thu, 26 Oct 2017 15:17:10 +0200
Message-Id: <20171026131741.5059-5-kwolf@redhat.com>
In-Reply-To: <20171026131741.5059-1-kwolf@redhat.com>
References: <20171026131741.5059-1-kwolf@redhat.com>
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
	(mx1.redhat.com [10.5.110.27]);
	Thu, 26 Oct 2017 13:17:56 +0000 (UTC)
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]
	[fuzzy]
X-Received-From: 209.132.183.28
Subject: [Qemu-devel] [PULL 04/35] block: Add flag to avoid wasted work in
 bdrv_is_allocated()
X-BeenThere: qemu-devel@nongnu.org
X-Mailman-Version: 2.1.21
Precedence: list
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org
Sender: "Qemu-devel" <qemu-devel-bounces+importer=patchew.org@nongnu.org>
X-ZohoMail: RSF_0  Z_629925259 SPT_0
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"

From: Eric Blake <eblake@redhat.com>

Not all callers care about which BDS owns the mapping for a given
range of the file, or where the zeroes lie within that mapping.  In
particular, bdrv_is_allocated() cares more about finding the
largest run of allocated data from the guest perspective, whether
or not that data is consecutive from the host perspective, and
whether or not the data reads as zero.  Therefore, doing subsequent
refinements such as checking how much of the format-layer
allocation also satisfies BDRV_BLOCK_ZERO at the protocol layer is
wasted work - in the best case, it just costs extra CPU cycles
during a single bdrv_is_allocated(), but in the worst case, it
results in a smaller *pnum, and forces callers to iterate through
more status probes when visiting the entire file for even more
extra CPU cycles.

This patch only optimizes the block layer (no behavior change when
want_zero is true, but skip unnecessary effort when it is false).
Then when subsequent patches tweak the driver callback to be
byte-based, we can also pass this hint through to the driver.

Tweak BdrvCoGetBlockStatusData to declare arguments in parameter
order, rather than mixing things up (minimizing padding is not
necessary here).

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/io.c | 57 +++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 16 deletions(-)

diff --git a/block/io.c b/block/io.c
index 93231b0e26..8ba408c813 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1769,10 +1769,11 @@ int bdrv_flush_all(void)
 typedef struct BdrvCoGetBlockStatusData {
     BlockDriverState *bs;
     BlockDriverState *base;
-    BlockDriverState **file;
+    bool want_zero;
     int64_t sector_num;
     int nb_sectors;
     int *pnum;
+    BlockDriverState **file;
     int64_t ret;
     bool done;
 } BdrvCoGetBlockStatusData;
@@ -1808,6 +1809,11 @@ int64_t coroutine_fn bdrv_co_get_block_status_from_b=
acking(BlockDriverState *bs,
  * Drivers not implementing the functionality are assumed to not support
  * backing files, hence all their sectors are reported as allocated.
  *
+ * If 'want_zero' is true, the caller is querying for mapping purposes,
+ * and the result should include BDRV_BLOCK_OFFSET_VALID and
+ * BDRV_BLOCK_ZERO where possible; otherwise, the result may omit those
+ * bits particularly if it allows for a larger value in 'pnum'.
+ *
  * If 'sector_num' is beyond the end of the disk image the return value is
  * BDRV_BLOCK_EOF and 'pnum' is set to 0.
  *
@@ -1824,6 +1830,7 @@ int64_t coroutine_fn bdrv_co_get_block_status_from_ba=
cking(BlockDriverState *bs,
  * is allocated in.
  */
 static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
+                                                     bool want_zero,
                                                      int64_t sector_num,
                                                      int nb_sectors, int *=
pnum,
                                                      BlockDriverState **fi=
le)
@@ -1878,31 +1885,34 @@ static int64_t coroutine_fn bdrv_co_get_block_statu=
s(BlockDriverState *bs,
=20
     if (ret & BDRV_BLOCK_RAW) {
         assert(ret & BDRV_BLOCK_OFFSET_VALID && local_file);
-        ret =3D bdrv_co_get_block_status(local_file, ret >> BDRV_SECTOR_BI=
TS,
+        ret =3D bdrv_co_get_block_status(local_file, want_zero,
+                                       ret >> BDRV_SECTOR_BITS,
                                        *pnum, pnum, &local_file);
         goto out;
     }
=20
     if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
         ret |=3D BDRV_BLOCK_ALLOCATED;
-    } else {
+    } else if (want_zero) {
         if (bdrv_unallocated_blocks_are_zero(bs)) {
             ret |=3D BDRV_BLOCK_ZERO;
         } else if (bs->backing) {
             BlockDriverState *bs2 =3D bs->backing->bs;
             int64_t nb_sectors2 =3D bdrv_nb_sectors(bs2);
+
             if (nb_sectors2 >=3D 0 && sector_num >=3D nb_sectors2) {
                 ret |=3D BDRV_BLOCK_ZERO;
             }
         }
     }
=20
-    if (local_file && local_file !=3D bs &&
+    if (want_zero && local_file && local_file !=3D bs &&
         (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
         (ret & BDRV_BLOCK_OFFSET_VALID)) {
         int file_pnum;
=20
-        ret2 =3D bdrv_co_get_block_status(local_file, ret >> BDRV_SECTOR_B=
ITS,
+        ret2 =3D bdrv_co_get_block_status(local_file, want_zero,
+                                        ret >> BDRV_SECTOR_BITS,
                                         *pnum, &file_pnum, NULL);
         if (ret2 >=3D 0) {
             /* Ignore errors.  This is just providing extra information, it
@@ -1938,6 +1948,7 @@ early_out:
=20
 static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverStat=
e *bs,
         BlockDriverState *base,
+        bool want_zero,
         int64_t sector_num,
         int nb_sectors,
         int *pnum,
@@ -1949,7 +1960,8 @@ static int64_t coroutine_fn bdrv_co_get_block_status_=
above(BlockDriverState *bs,
=20
     assert(bs !=3D base);
     for (p =3D bs; p !=3D base; p =3D backing_bs(p)) {
-        ret =3D bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum, =
file);
+        ret =3D bdrv_co_get_block_status(p, want_zero, sector_num, nb_sect=
ors,
+                                       pnum, file);
         if (ret < 0) {
             break;
         }
@@ -1979,6 +1991,7 @@ static void coroutine_fn bdrv_get_block_status_above_=
co_entry(void *opaque)
     BdrvCoGetBlockStatusData *data =3D opaque;
=20
     data->ret =3D bdrv_co_get_block_status_above(data->bs, data->base,
+                                               data->want_zero,
                                                data->sector_num,
                                                data->nb_sectors,
                                                data->pnum,
@@ -1991,20 +2004,22 @@ static void coroutine_fn bdrv_get_block_status_abov=
e_co_entry(void *opaque)
  *
  * See bdrv_co_get_block_status_above() for details.
  */
-int64_t bdrv_get_block_status_above(BlockDriverState *bs,
-                                    BlockDriverState *base,
-                                    int64_t sector_num,
-                                    int nb_sectors, int *pnum,
-                                    BlockDriverState **file)
+static int64_t bdrv_common_block_status_above(BlockDriverState *bs,
+                                              BlockDriverState *base,
+                                              bool want_zero,
+                                              int64_t sector_num,
+                                              int nb_sectors, int *pnum,
+                                              BlockDriverState **file)
 {
     Coroutine *co;
     BdrvCoGetBlockStatusData data =3D {
         .bs =3D bs,
         .base =3D base,
-        .file =3D file,
+        .want_zero =3D want_zero,
         .sector_num =3D sector_num,
         .nb_sectors =3D nb_sectors,
         .pnum =3D pnum,
+        .file =3D file,
         .done =3D false,
     };
=20
@@ -2020,6 +2035,16 @@ int64_t bdrv_get_block_status_above(BlockDriverState=
 *bs,
     return data.ret;
 }
=20
+int64_t bdrv_get_block_status_above(BlockDriverState *bs,
+                                    BlockDriverState *base,
+                                    int64_t sector_num,
+                                    int nb_sectors, int *pnum,
+                                    BlockDriverState **file)
+{
+    return bdrv_common_block_status_above(bs, base, true, sector_num,
+                                          nb_sectors, pnum, file);
+}
+
 int64_t bdrv_get_block_status(BlockDriverState *bs,
                               int64_t sector_num,
                               int nb_sectors, int *pnum,
@@ -2032,15 +2057,15 @@ int64_t bdrv_get_block_status(BlockDriverState *bs,
 int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset,
                                    int64_t bytes, int64_t *pnum)
 {
-    int64_t sector_num =3D offset >> BDRV_SECTOR_BITS;
-    int nb_sectors =3D bytes >> BDRV_SECTOR_BITS;
     int64_t ret;
     int psectors;
=20
     assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
     assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE) && bytes < INT_MAX);
-    ret =3D bdrv_get_block_status(bs, sector_num, nb_sectors, &psectors,
-                                NULL);
+    ret =3D bdrv_common_block_status_above(bs, backing_bs(bs), false,
+                                         offset >> BDRV_SECTOR_BITS,
+                                         bytes >> BDRV_SECTOR_BITS, &psect=
ors,
+                                         NULL);
     if (ret < 0) {
         return ret;
     }
--=20
2.13.6