From nobody Wed Apr 16 03:24:27 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1531238751271902.677840882951; Tue, 10 Jul 2018 09:05:51 -0700 (PDT) Received: from localhost ([::1]:48588 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fcv9C-0007KO-3w for importer@patchew.org; Tue, 10 Jul 2018 12:05:50 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44234) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fcunV-00053o-UF for qemu-devel@nongnu.org; Tue, 10 Jul 2018 11:43:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fcunU-0001be-ML for qemu-devel@nongnu.org; Tue, 10 Jul 2018 11:43:25 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:34316 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fcunS-0001Zb-1B; Tue, 10 Jul 2018 11:43:22 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8E781402315B; Tue, 10 Jul 2018 15:43:21 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-156.ams2.redhat.com [10.36.116.156]) by smtp.corp.redhat.com (Postfix) with ESMTP id BCA542026D6B; Tue, 10 Jul 2018 15:43:20 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 10 Jul 2018 17:42:45 +0200 Message-Id: <20180710154304.18304-6-kwolf@redhat.com> In-Reply-To: <20180710154304.18304-1-kwolf@redhat.com> References: <20180710154304.18304-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Tue, 10 Jul 2018 15:43:21 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Tue, 10 Jul 2018 15:43:21 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 05/24] block/io: fix copy_range X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, peter.maydell@linaro.org, qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" 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: Vladimir Sementsov-Ogievskiy Here two things are fixed: 1. Architecture On each recursion step, we go to the child of src or dst, only for one of them. So, it's wrong to create tracked requests for both on each step. It leads to tracked requests duplication. 2. Wait for serializing requests on write path independently of BDRV_REQ_NO_SERIALISING Before commit 9ded4a01149 "backup: Use copy offloading", BDRV_REQ_NO_SERIALISING was used for only one case: read in copy-on-write operation during backup. Also, the flag was handled only on read path (in bdrv_co_preadv and bdrv_aligned_preadv). After 9ded4a01149, flag is used for not waiting serializing operations on backup target (in same case of copy-on-write operation). This behavior change is unsubstantiated and potentially dangerous, let's drop it and add additional asserts and documentation. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- include/block/block.h | 12 ++++++++++++ block/io.c | 42 +++++++++++++++++++++++++++--------------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 706ef009ad..f7ddff45b6 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -50,6 +50,18 @@ typedef enum { * opened with BDRV_O_UNMAP. */ BDRV_REQ_MAY_UNMAP =3D 0x4, + + /* + * The BDRV_REQ_NO_SERIALISING flag is only valid for reads and means = that + * we don't want wait_serialising_requests() during the read operation. + * + * This flag is used for backup copy-on-write operations, when we need= to + * read old data before write (write notifier triggered). It is okay s= ince + * we already waited for other serializing requests in the initiating = write + * (see bdrv_aligned_pwritev), and it is necessary if the initiating w= rite + * is already serializing (without the flag, the read would deadlock + * waiting for the serialising write to complete). + */ BDRV_REQ_NO_SERIALISING =3D 0x8, BDRV_REQ_FUA =3D 0x10, BDRV_REQ_WRITE_COMPRESSED =3D 0x20, diff --git a/block/io.c b/block/io.c index 4c0831149c..3a321d69d3 100644 --- a/block/io.c +++ b/block/io.c @@ -1592,6 +1592,8 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChil= d *child, max_transfer =3D QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_transfer, INT= _MAX), align); =20 + /* BDRV_REQ_NO_SERIALISING is only for read operation */ + assert(!(flags & BDRV_REQ_NO_SERIALISING)); waited =3D wait_serialising_requests(req); assert(!waited || !req->serialising); assert(req->overlap_offset <=3D offset); @@ -2916,7 +2918,7 @@ static int coroutine_fn bdrv_co_copy_range_internal(B= drvChild *src, BdrvRequestFlags flags, bool recurse_src) { - BdrvTrackedRequest src_req, dst_req; + BdrvTrackedRequest req; int ret; =20 if (!dst || !dst->bs) { @@ -2943,32 +2945,42 @@ static int coroutine_fn bdrv_co_copy_range_internal= (BdrvChild *src, || src->bs->encrypted || dst->bs->encrypted) { return -ENOTSUP; } - bdrv_inc_in_flight(src->bs); - bdrv_inc_in_flight(dst->bs); - tracked_request_begin(&src_req, src->bs, src_offset, - bytes, BDRV_TRACKED_READ); - tracked_request_begin(&dst_req, dst->bs, dst_offset, - bytes, BDRV_TRACKED_WRITE); =20 - if (!(flags & BDRV_REQ_NO_SERIALISING)) { - wait_serialising_requests(&src_req); - wait_serialising_requests(&dst_req); - } if (recurse_src) { + bdrv_inc_in_flight(src->bs); + tracked_request_begin(&req, src->bs, src_offset, bytes, + BDRV_TRACKED_READ); + + if (!(flags & BDRV_REQ_NO_SERIALISING)) { + wait_serialising_requests(&req); + } + ret =3D src->bs->drv->bdrv_co_copy_range_from(src->bs, src, src_offset, dst, dst_offset, bytes, flags); + + tracked_request_end(&req); + bdrv_dec_in_flight(src->bs); } else { + bdrv_inc_in_flight(dst->bs); + tracked_request_begin(&req, dst->bs, dst_offset, bytes, + BDRV_TRACKED_WRITE); + + /* BDRV_REQ_NO_SERIALISING is only for read operation, + * so we ignore it in flags. + */ + wait_serialising_requests(&req); + ret =3D dst->bs->drv->bdrv_co_copy_range_to(dst->bs, src, src_offset, dst, dst_offset, bytes, flags); + + tracked_request_end(&req); + bdrv_dec_in_flight(dst->bs); } - tracked_request_end(&src_req); - tracked_request_end(&dst_req); - bdrv_dec_in_flight(src->bs); - bdrv_dec_in_flight(dst->bs); + return ret; } =20 --=20 2.13.6