linux-kernel - Re: [fuse-devel] [PATCH 6/6] fuse: add mount option to disable synchronous release

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <53F78780.9000206@redhat.com>
Date:	Fri, 22 Aug 2014 11:10:08 -0700
From:	Anand Avati <avati@...hat.com>
To:	Miklos Szeredi <miklos@...redi.hu>,
	Maxim Patlasov <MPatlasov@...allels.com>
CC:	fuse-devel <fuse-devel@...ts.sourceforge.net>,
	Anand Avati <avati@...ster.org>,
	Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [fuse-devel] [PATCH 6/6] fuse: add mount option to disable synchronous
 release

On 8/22/14, 7:09 AM, Miklos Szeredi wrote:
> On Thu, Aug 21, 2014 at 6:09 PM, Maxim Patlasov <MPatlasov@...allels.com> 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 <mpatlasov@...allels.com>
>> ---
>> ---
>>   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@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/fuse-devel
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/