From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Maxim V. Patlasov" Subject: Re: [fuse-devel] [PATCH 0/6] fuse: process direct IO asynchronously Date: Fri, 14 Dec 2012 16:40:26 +0400 Message-ID: <50CB1E3A.3000101@parallels.com> References: <50C9CF95.7090704@parallels.com> <50CA31D8.9020507@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "miklos@szeredi.hu" , Alexander Viro , "fuse-devel@lists.sourceforge.net" , "linux-fsdevel@vger.kernel.org" , , To: Brian Foster Return-path: Received: from relay.parallels.com ([195.214.232.42]:38623 "EHLO relay.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753220Ab2LNMk3 convert rfc822-to-8bit (ORCPT ); Fri, 14 Dec 2012 07:40:29 -0500 In-Reply-To: <50CA31D8.9020507@redhat.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Hi Brian, 12/13/2012 11:51 PM, Brian Foster =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > [cut] > Before getting into that, I made some adjustments to the original pat= ch > to use your new fuse_io_priv structure in place of an iocb, primarily= to > decouple the request submission mechanism from the type of the user > request. The highlighted differences are: > > - Updated fuse_io_priv with an async field and file pointer to preser= ve > the current style of interface (i.e., use this instead of iocb). > - Trigger the type of request submission based on the async field. > - Reintroduce the wait in fuse_direct_IO(), but I also pulled up the > fuse_write_update_size() call out of __fuse_direct_write() to make th= e > separate paths more consistent. Sounds reasonable. > This could probably use more cleanups (i.e., it morphs your fuse_io_p= riv > into more of a generic I/O descriptor), but illustrates the idea. The patch you provided looks fine. Do you have any particular (further)= =20 cleanups in mind? I'm going to resend initial patch-set with your last=20 patch applied. If you provide more ideas about improving it, I'll be=20 glad to consider and integrate them as well. > [cut] >> The problem is that FUSE tends to be the only user of this 'feature'= =2E >> I'm considering one-line change of aio_complete(): >> >> - if (is_sync_kiocb(iocb)) { >> + if (!iocb->ki_ctx) { >> >> Though, not sure whether Miklos, Al Viro, Benjamin and others will >> accept it... Let's try if you're OK about this approach. At least th= is >> will cover all fuse dio use-cases (including libaio extending file) = and >> makes fuse code significantly cleaner. >> > Interesting idea. It seems like it could work to me but I'm not very > familiar with that code and it might not be worth cluttering that > interface for this particular case. Yes, exactly. Moreover, after looking at aio.h closer: > struct kioctx *ki_ctx; /* may be NULL for sync ops */ I realized that that would be rather hack: sync ops doesn't use ki_ctx=20 for now, but this can change in the future. > I brought this up briefly with Jeff > Moyer and he pointed me at this set (I just saw zab's mail pointing t= his > out as well :): > > https://lkml.org/lkml/2012/10/22/321 > > ... which looks like it could help. Perhaps we could use that new typ= e > of kiocb with a custom callback to wake a blocking thread. That said,= I > wonder if we could do that anyways; i.e., define our own > wait_on_fuse_async() type function that that waits on the number of > in-flight requests in the fuse_priv_io. I guess we'd have to complica= te > the refcounting with that type of approach, though. I've considered using that new API (kernel aio) but refrained from it=20 deliberately. The reasons were similar to ones you listed above: why=20 should we invent another synchronization mechanism if=20 wait_on_sync_kiocb() does exactly what we need? Now, after more=20 thinking, I tend to think that using that API is doable, but the way ho= w=20 we could do it is too clumsy. Please let me know if you need more detai= ls. Thanks, Maxim -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html