From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 36276C77B75 for ; Wed, 3 May 2023 08:09:49 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pu7Xs-0007FL-5X; Wed, 03 May 2023 04:09:04 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pu7Xo-00079N-4K for qemu-devel@nongnu.org; Wed, 03 May 2023 04:09:00 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pu7Xm-0007aq-Bh for qemu-devel@nongnu.org; Wed, 03 May 2023 04:08:59 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1683101337; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=GdsrTvtpKiwphnVpAamWYth1JM74zuqKQh3Am2keHQg=; b=DrcBo0DMnCv/KYxjqbWPFEyuBQ5x0xjmLkCGM4iu9AWTD7bMDcq3Wb9YyNkcujEtAfsvw/ j+MCnqqGLEWWFB1X3FiTInfVcS+E9cAVWuKkQJdECig6CHgi8sOGjV7gIO0ZE6SJgaxxPl EOqzB54vB61LF8uKV5o2rajrMYOoT1w= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-36-vLcfhCiUOwuGWkdaAmksmg-1; Wed, 03 May 2023 04:08:51 -0400 X-MC-Unique: vLcfhCiUOwuGWkdaAmksmg-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C56C810504AC; Wed, 3 May 2023 08:08:50 +0000 (UTC) Received: from redhat.com (dhcp-192-205.str.redhat.com [10.33.192.205]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4DD9F492B00; Wed, 3 May 2023 08:08:47 +0000 (UTC) Date: Wed, 3 May 2023 10:08:46 +0200 From: Kevin Wolf To: Stefan Hajnoczi Cc: qemu-devel@nongnu.org, Daniel =?iso-8859-1?Q?P=2E_Berrang=E9?= , Juan Quintela , Julia Suvorova , xen-devel@lists.xenproject.org, eesposit@redhat.com, Richard Henderson , Fam Zheng , "Michael S. Tsirkin" , Coiby Xu , David Woodhouse , Marcel Apfelbaum , Peter Lieven , Paul Durrant , "Richard W.M. Jones" , qemu-block@nongnu.org, Stefano Garzarella , Anthony Perard , Stefan Weil , Xie Yongji , Paolo Bonzini , Aarushi Mehta , Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= , Eduardo Habkost , Stefano Stabellini , Hanna Reitz , Ronnie Sahlberg Subject: Re: [PATCH v4 07/20] block/export: stop using is_external in vhost-user-blk server Message-ID: References: <20230425172716.1033562-1-stefanha@redhat.com> <20230425172716.1033562-8-stefanha@redhat.com> <20230502200645.GE535070@fedora> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="8uIPJqxz2LaoW5Jz" Content-Disposition: inline In-Reply-To: <20230502200645.GE535070@fedora> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 Received-SPF: pass client-ip=170.10.133.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -22 X-Spam_score: -2.3 X-Spam_bar: -- X-Spam_report: (-2.3 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.171, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org --8uIPJqxz2LaoW5Jz Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 02.05.2023 um 22:06 hat Stefan Hajnoczi geschrieben: > On Tue, May 02, 2023 at 06:04:24PM +0200, Kevin Wolf wrote: > > Am 25.04.2023 um 19:27 hat Stefan Hajnoczi geschrieben: > > > vhost-user activity must be suspended during bdrv_drained_begin/end(). > > > This prevents new requests from interfering with whatever is happening > > > in the drained section. > > >=20 > > > Previously this was done using aio_set_fd_handler()'s is_external > > > argument. In a multi-queue block layer world the aio_disable_external= () > > > API cannot be used since multiple AioContext may be processing I/O, n= ot > > > just one. > > >=20 > > > Switch to BlockDevOps->drained_begin/end() callbacks. > > >=20 > > > Signed-off-by: Stefan Hajnoczi > > > --- > > > block/export/vhost-user-blk-server.c | 43 ++++++++++++++------------= -- > > > util/vhost-user-server.c | 10 +++---- > > > 2 files changed, 26 insertions(+), 27 deletions(-) > > >=20 > > > diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhos= t-user-blk-server.c > > > index 092b86aae4..d20f69cd74 100644 > > > --- a/block/export/vhost-user-blk-server.c > > > +++ b/block/export/vhost-user-blk-server.c > > > @@ -208,22 +208,6 @@ static const VuDevIface vu_blk_iface =3D { > > > .process_msg =3D vu_blk_process_msg, > > > }; > > > =20 > > > -static void blk_aio_attached(AioContext *ctx, void *opaque) > > > -{ > > > - VuBlkExport *vexp =3D opaque; > > > - > > > - vexp->export.ctx =3D ctx; > > > - vhost_user_server_attach_aio_context(&vexp->vu_server, ctx); > > > -} > > > - > > > -static void blk_aio_detach(void *opaque) > > > -{ > > > - VuBlkExport *vexp =3D opaque; > > > - > > > - vhost_user_server_detach_aio_context(&vexp->vu_server); > > > - vexp->export.ctx =3D NULL; > > > -} > >=20 > > So for changing the AioContext, we now rely on the fact that the node to > > be changed is always drained, so the drain callbacks implicitly cover > > this case, too? >=20 > Yes. Ok. This surprised me a bit at first, but I think it's fine. We just need to remember it if we ever decide that once we have multiqueue, we can actually change the default AioContext without draining the node. But maybe at that point, we have to do more fundamental changes anyway. > > > static void > > > vu_blk_initialize_config(BlockDriverState *bs, > > > struct virtio_blk_config *config, > > > @@ -272,6 +256,25 @@ static void vu_blk_exp_resize(void *opaque) > > > vu_config_change_msg(&vexp->vu_server.vu_dev); > > > } > > > =20 > > > +/* Called with vexp->export.ctx acquired */ > > > +static void vu_blk_drained_begin(void *opaque) > > > +{ > > > + VuBlkExport *vexp =3D opaque; > > > + > > > + vhost_user_server_detach_aio_context(&vexp->vu_server); > > > +} > >=20 > > Compared to the old code, we're losing the vexp->export.ctx =3D NULL. T= his > > is correct at this point because after drained_begin we still keep > > processing requests until we arrive at a quiescent state. > >=20 > > However, if we detach the AioContext because we're deleting the > > iothread, won't we end up with a dangling pointer in vexp->export.ctx? > > Or can we be certain that nothing interesting happens before drained_end > > updates it with a new valid pointer again? >=20 > If you want I can add the detach() callback back again and set ctx to > NULL there? I haven't thought enough about it to say if it's a problem. If you have and are confident that it's correct the way it is, I'm happy with it. But bringing the callback back is the minimal change compared to the old state. It's just unnecessary code if we don't actually need it. Kevin --8uIPJqxz2LaoW5Jz Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmRSFo4ACgkQfwmycsiP L9bGwQ/9HZIvpsXG7HrllRtRSZut7ucmoWbVO6oQFsVMI4RFuShXz3jZHU1roMAs tCqrZRC6M8Y+JA/pROYtwBTcrvOE/FUc6d/KXFXblg/NGTFDUGwmPWa3t8kZPul/ wfOG8F/BRVLtOhY1DtJhAqO1cXkda/bZxqFDrCgyqtfLpXh+kuVQSOCHbontOMYu vNfhY3FYP0FH44fRnPsUJQVsx0pmZ07waR5lc2ukINv8d0uSN2L7U8Vw8JrNaWAK xGJmpznwr+eZla+/lsWQCQhWysYQIuZGuk/WFeXbitwLEnw7CiJc7qpYf55v4v8Z AOLuMBQVN7QGGrKmpbwuzoD09MOWsIk7dd6X30DxiF7eaE5kO/4jkIeVYPtiaXg2 WUJpZYL1bRX04Tg2snXA4fw06fWMOO1LdAhrDHbU8P+WmxgjDpSO6zFGySFEjukb NHi9p14nSXwrhFUD5wOREzVRR828cmgltcrVypCRPXwISwqxfHuAvqRdrG4/+pIh 5u/VTqxrKerXIPxMgiU9yqJAwaRJkyK/+yFmg9vYUiBcR4sGw5HQIUzeEif5V+pd ckV5NxGX/Wbuyo5RpiL1YnzwB4LthdVaRYLQBuR3RaKyY7G6VSUP9ARQjBg6hEr/ fDnW23q/JH7Gr7CIdAKWz/D9Q4aL+rw4z9CbfBFkq4oG7naqDIU= =3jaa -----END PGP SIGNATURE----- --8uIPJqxz2LaoW5Jz--