From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44995) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c9B9X-0001uM-DD for qemu-devel@nongnu.org; Tue, 22 Nov 2016 08:30:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c9B9T-0002wS-Dj for qemu-devel@nongnu.org; Tue, 22 Nov 2016 08:30:27 -0500 Date: Tue, 22 Nov 2016 14:30:12 +0100 From: Kevin Wolf Message-ID: <20161122133012.GE5615@noname.redhat.com> References: <1479413642-22463-1-git-send-email-eblake@redhat.com> <1479413642-22463-4-git-send-email-eblake@redhat.com> <20161122131653.GD5615@noname.redhat.com> <69447b54-f0ea-de74-ea9b-5184c44bddc8@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="aT9PWwzfKXlsBJM1" Content-Disposition: inline In-Reply-To: <69447b54-f0ea-de74-ea9b-5184c44bddc8@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 3/9] block: Let write zeroes fallback work even with small max_transfer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, pbonzini@redhat.com, qemu-stable@nongnu.org, "Denis V . Lunev" , Stefan Hajnoczi , Fam Zheng , Max Reitz --aT9PWwzfKXlsBJM1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 22.11.2016 um 14:22 hat Eric Blake geschrieben: > On 11/22/2016 07:16 AM, Kevin Wolf wrote: > > Am 17.11.2016 um 21:13 hat Eric Blake geschrieben: > >> Commit 443668ca rewrote the write_zeroes logic to guarantee that > >> an unaligned request never crosses a cluster boundary. But > >> in the rewrite, the new code assumed that at most one iteration > >> would be needed to get to an alignment boundary. > >> >=20 > >> @@ -1257,8 +1262,6 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes= (BlockDriverState *bs, > >> > >> if (ret =3D=3D -ENOTSUP) { > >> /* Fall back to bounce buffer if write zeroes is unsuppor= ted */ > >> - int max_transfer =3D MIN_NON_ZERO(bs->bl.max_transfer, > >> - MAX_WRITE_ZEROES_BOUNCE_B= UFFER); > >> BdrvRequestFlags write_flags =3D flags & ~BDRV_REQ_ZERO_W= RITE; > >=20 > > Why do we even still bother with max_transfer in this function when we > > could just call bdrv_aligned_pwritev() and use its fragmentation code? >=20 > Hmm. bdrv_aligned_pwritev() asserts that its arguments are already > aligned, but for the head and tail, they might not be. I agree that for > the bulk of the body, it may help, but it would take more thought on > refactoring if we want to have fragmentation at only one spot. Right, it should be more like bdrv_co_pwritev() then, but something that uses the logic in bdrv_aligned_pwritev(). Using bdrv_co_pwritev() would mean that it's tracked as another request, but I don't think that's a problem. Otherwise we'd have to factor that part out. > > Of course, when bdrv_co_do_pwrite_zeroes() was written, your > > fragmentation code didn't exist yet, but today I think it would make > > more sense to use a single centralised version of it instead of > > reimplementing it here. > >=20 > > This doesn't make your fix less correct, but if we did things this way, > > the fix wouldn't even be needed because a single iteration (in this > > loop) would indeed always be enough. >=20 > Can I request to defer such refactoring to 2.9, while getting this patch > as-is into 2.8? Yes, the refactoring is definitely for 2.9. Kevin --aT9PWwzfKXlsBJM1 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJYNEhkAAoJEH8JsnLIjy/WuhkP/2dzNmUj3A1091kad0IWxEZ8 NPT1x4uCEJ0hBBEFSF7rzPuqcVOtc8ogC+8rpRu0vl9koESqyQGDjg4+gjjrxl3l VlWY7t576QZo/Ukg0VFuLpEIoIi3vmNA/mC6c2qOdgwMRrXU2VO4Nj3URyS7z+RN bANHftfPXw56F0LHns+1FAce0ygRIcHq4B8FIWaqls75EiU3A0C2urusIdWbxawP vf5fUpBCnyw2fvrWV1K/T5jTnA3atN9WlZ1sjlTtVKBxjOJT823i9FXRCn1l0thL li4S1aMLpUbFfHaU3Nh3frZNhbvnpy9m/Oz7qYSzAlmZKR5dGLWLKtWFykSuvLo9 6Fd1h1VWDrnmSN6BwR8BRTy6i3149oCPZmfVefRcqTUbHXDna7wo9G4Vdbn0npzS hk2VNLzTB5jifZYr9JibdbMAzusvVPibAxjS6DIHdWByVAlBy4Oo+/rNUWPSNdP/ YyR5aaCz2tK68Z1nFcudCrBvXk8exbrTlR8MU5BO/JOVBy/MvsZVd3pCCwdWNHnI LJiSrGOsFKdB11KlAhTupGvaRrVQM8iws6Dib2SV0oMI1wTxr27lEpVtutNijmFK I9fH4vMe24G4Tq0x4pAQu7Z9sFKdGkwpQ72syevgN3zZNkIPPEB3NYWtA2ba65V2 tmQ0c5ilW/duuCbybXbL =sI3W -----END PGP SIGNATURE----- --aT9PWwzfKXlsBJM1--