From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932731AbaHVSK1 (ORCPT ); Fri, 22 Aug 2014 14:10:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:16090 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932410AbaHVSKZ (ORCPT ); Fri, 22 Aug 2014 14:10:25 -0400 Message-ID: <53F78780.9000206@redhat.com> Date: Fri, 22 Aug 2014 11:10:08 -0700 From: Anand Avati User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Miklos Szeredi , Maxim Patlasov CC: fuse-devel , Anand Avati , Kernel Mailing List Subject: Re: [fuse-devel] [PATCH 6/6] fuse: add mount option to disable synchronous release References: <20140821160304.11005.15166.stgit@localhost.localdomain> <20140821160930.11005.29357.stgit@localhost.localdomain> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 8/22/14, 7:09 AM, Miklos Szeredi wrote: > On Thu, Aug 21, 2014 at 6:09 PM, Maxim Patlasov wrote: >> Synchronous release ensures that kernel fuse reports to userspace exactly >> last fput(). However, nothing guarantees that last fput() will happen in the >> context of close(2). So userspace applications must be prepared to the >> situation when close(2) returned, but processing of FUSE_RELEASE is not >> completed by fuse daemon yet. To make testing such use cases easier, the >> patch introduces DISABLE_SYNC_RELEASE mount option which effectively mask out >> FOPEN_SYNC_RELEASE flag. > > Why not make this an INIT flag? Much easier to add support for in userspace. > > Thanks, > Miklos In a real world scenario you may want to perform synchronous release selectively. As such performance over lots of small files is generally slow because of context switches, and a synchronous release adds an extra switch. In cases like 'grep -r' over lots of text files where consistency don't matter or userspace knows data doesn't get modified, the userspace can chose to not perform synchronous release with some heuristics (like size is small and open flag is O_RDONLY etc.) where as for large read/write files (like VM images?) you would want to perform synchronous release for consistency purposes. A global flag in INIT would make it too crude and impose the same consistency for all files. Thanks Avati > >> >> Signed-off-by: Maxim Patlasov >> --- >> --- >> fs/fuse/file.c | 3 ++- >> fs/fuse/fuse_i.h | 3 +++ >> fs/fuse/inode.c | 8 ++++++++ >> 3 files changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/fs/fuse/file.c b/fs/fuse/file.c >> index b92143a..ad3d357 100644 >> --- a/fs/fuse/file.c >> +++ b/fs/fuse/file.c >> @@ -296,7 +296,8 @@ static void fuse_prepare_release(struct fuse_file *ff, int flags, int opcode) >> >> static bool must_release_synchronously(struct fuse_file *ff) >> { >> - return ff->open_flags & FOPEN_SYNC_RELEASE; >> + return ff->open_flags & FOPEN_SYNC_RELEASE && >> + !(ff->fc->flags & FUSE_DISABLE_SYNC_RELEASE); >> } >> >> void fuse_release_common(struct file *file, int opcode) >> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h >> index e8e47a6..c5e2fca 100644 >> --- a/fs/fuse/fuse_i.h >> +++ b/fs/fuse/fuse_i.h >> @@ -44,6 +44,9 @@ >> doing the mount will be allowed to access the filesystem */ >> #define FUSE_ALLOW_OTHER (1 << 1) >> >> +/** Disable synchronous release */ >> +#define FUSE_DISABLE_SYNC_RELEASE (1 << 2) >> + >> /** Number of page pointers embedded in fuse_req */ >> #define FUSE_REQ_INLINE_PAGES 1 >> >> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c >> index 03246cd..86d47d0 100644 >> --- a/fs/fuse/inode.c >> +++ b/fs/fuse/inode.c >> @@ -463,6 +463,7 @@ enum { >> OPT_ALLOW_OTHER, >> OPT_MAX_READ, >> OPT_BLKSIZE, >> + OPT_DISABLE_SYNC_RELEASE, >> OPT_ERR >> }; >> >> @@ -475,6 +476,7 @@ static const match_table_t tokens = { >> {OPT_ALLOW_OTHER, "allow_other"}, >> {OPT_MAX_READ, "max_read=%u"}, >> {OPT_BLKSIZE, "blksize=%u"}, >> + {OPT_DISABLE_SYNC_RELEASE, "disable_sync_release"}, >> {OPT_ERR, NULL} >> }; >> >> @@ -560,6 +562,10 @@ static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev) >> d->blksize = value; >> break; >> >> + case OPT_DISABLE_SYNC_RELEASE: >> + d->flags |= FUSE_DISABLE_SYNC_RELEASE; >> + break; >> + >> default: >> return 0; >> } >> @@ -583,6 +589,8 @@ static int fuse_show_options(struct seq_file *m, struct dentry *root) >> seq_puts(m, ",default_permissions"); >> if (fc->flags & FUSE_ALLOW_OTHER) >> seq_puts(m, ",allow_other"); >> + if (fc->flags & FUSE_DISABLE_SYNC_RELEASE) >> + seq_puts(m, ",disable_sync_release"); >> if (fc->max_read != ~0) >> seq_printf(m, ",max_read=%u", fc->max_read); >> if (sb->s_bdev && sb->s_blocksize != FUSE_DEFAULT_BLKSIZE) >> > > ------------------------------------------------------------------------------ > Slashdot TV. > Video for Nerds. Stuff that matters. > http://tv.slashdot.org/ > _______________________________________________ > fuse-devel mailing list > fuse-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/fuse-devel >