From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55505) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsAe5-0004VK-29 for qemu-devel@nongnu.org; Wed, 04 Jun 2014 08:50:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WsAdv-0002Uc-Ad for qemu-devel@nongnu.org; Wed, 04 Jun 2014 08:50:21 -0400 Received: from mail-we0-x22b.google.com ([2a00:1450:400c:c03::22b]:47478) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsAdv-0002UT-1q for qemu-devel@nongnu.org; Wed, 04 Jun 2014 08:50:11 -0400 Received: by mail-we0-f171.google.com with SMTP id w62so8536443wes.30 for ; Wed, 04 Jun 2014 05:50:09 -0700 (PDT) Date: Wed, 4 Jun 2014 14:50:05 +0200 From: Stefan Hajnoczi Message-ID: <20140604125005.GG11073@stefanha-thinkpad.redhat.com> References: <1401561792-13410-1-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1401561792-13410-1-git-send-email-mreitz@redhat.com> Subject: Re: [Qemu-devel] [RFC 0/5] nbd: Adapt for dataplane List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , Paolo Bonzini , Fam Zheng , qemu-devel@nongnu.org, Stefan Hajnoczi On Sat, May 31, 2014 at 08:43:07PM +0200, Max Reitz wrote: > For the NBD server to work with dataplane, it needs to correctly access > the exported BDS. It makes the most sense to run both in the same > AioContext, therefore this series implements methods for tracking a > BDS's AioContext and makes NBD make use of this for keeping the clients > connected to that BDS in the same AioContext. > > The reason this is an RFC and not a PATCH is my inexperience with AIO, > coroutines and the like. Also, I'm not sure about what to do about the > coroutines. The NBD server has up to two coroutines per client: One for > receiving and one for sending. Theoretically, both have to be > "transferred" to the new AioContext if it is changed; however, as far as > I see it, coroutines are not really bound to an AioContext, they are > simply run in the AioContext entering them. Therefore, I think a > transfer is unnecessary. All coroutines are entered from nbd_read() and > nbd_restart_write(), both of which are AIO routines registered via > aio_set_fd_handler2(). > > As bs_aio_detach() unregisters all of these routines, the coroutines can > no longer be entered, but only after bs_aio_attach() is called again. > Then, when they are called, they will enter the coroutines in the new > AioContext. Therefore, I think an explicit transfer unnecessary. > > However, if bs_aio_detach() is called from a different thread than the > old AioContext is running in, we may still have coroutines running for > which we should wait before returning from bs_aio_detach(). > > But because of my inexperience with coroutines, I'm not sure. I now have > these patches nearly unchanged here for about a week and I'm looking for > ways of testing them, but so far I could only test whether the old use > cases work, but not whether they will work for what they are intended to > do: With BDS changing their AioContext. > > So, because I'm not sure what else to do and because I don't know how to > test multiple AIO threads (how do I move a BDS into another iothread?) > I'm just sending this out as an RFC. > > > Max Reitz (5): > nbd: Correct name comparison for export_set_name() > aio: Add io_read_poll() callback > nbd: Use aio_set_fd_handler2() > block: Add AIO followers > nbd: Follow the BDS' AIO context > > aio-posix.c | 26 ++++++++++++++++----- > block.c | 55 +++++++++++++++++++++++++++++++++++++++++++ > include/block/aio.h | 12 ++++++++++ > include/block/block_int.h | 40 ++++++++++++++++++++++++++++++++ > include/qemu/main-loop.h | 1 - > nbd.c | 59 ++++++++++++++++++++++++++++++++++++++--------- > 6 files changed, 175 insertions(+), 18 deletions(-) Thanks for the RFC. It looks like the concept will work correctly. I left comments, a lot of them are just ideas and you don't have to implement them if you don't want to. Stefan