Re: [fuse-devel] [PATCH v2] fuse: add a new fuse init flag to relax restrictions in no cache mode

[Hao Xu <hao.xu@linux.dev>]

Hi Bernd,

On 6/30/23 01:13, Bernd Schubert wrote:
> 
> 
> On 6/29/23 10:17, Hao Xu wrote:
>> From: Hao Xu <howeyxu@tencent.com>
>>
>> FOPEN_DIRECT_IO is usually set by fuse daemon to indicate need of strong
>> coherency, e.g. network filesystems. Thus shared mmap is disabled since
>> it leverages page cache and may write to it, which may cause
>> inconsistence. But FOPEN_DIRECT_IO can be used not for coherency but to
>> reduce memory footprint as well, e.g. reduce guest memory usage with
>> virtiofs. Therefore, add a new fuse init flag FUSE_DIRECT_IO_RELAX to
>> relax restrictions in that mode, currently, it allows shared mmap.
>> One thing to note is to make sure it doesn't break coherency in your
>> use case.
>>
>> Signed-off-by: Hao Xu <howeyxu@tencent.com>
>> ---
>>
>> v1 -> v2:
>>      make the new flag a fuse init one rather than a open flag since it's
>>      not common that different files in a filesystem has different
>>      strategy of shared mmap.
>>
>>   fs/fuse/file.c            | 8 ++++++--
>>   fs/fuse/fuse_i.h          | 3 +++
>>   fs/fuse/inode.c           | 5 ++++-
>>   include/uapi/linux/fuse.h | 1 +
>>   4 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index bc4115288eec..871b66b54322 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -2478,14 +2478,18 @@ static const struct vm_operations_struct 
>> fuse_file_vm_ops = {
>>   static int fuse_file_mmap(struct file *file, struct vm_area_struct 
>> *vma)
>>   {
>>       struct fuse_file *ff = file->private_data;
>> +    struct fuse_conn *fc = ff->fm->fc;
>>       /* DAX mmap is superior to direct_io mmap */
>>       if (FUSE_IS_DAX(file_inode(file)))
>>           return fuse_dax_mmap(file, vma);
>>       if (ff->open_flags & FOPEN_DIRECT_IO) {
>> -        /* Can't provide the coherency needed for MAP_SHARED */
>> -        if (vma->vm_flags & VM_MAYSHARE)
>> +        /* Can't provide the coherency needed for MAP_SHARED
>> +         * if FUSE_DIRECT_IO_RELAX isn't set.
>> +         */
>> +        if (!(ff->open_flags & fc->direct_io_relax) &&
>> +            vma->vm_flags & VM_MAYSHARE)
>>               return -ENODEV;
> 
> I'm confused here, the idea was that open_flags do not need additional 
> flags? Why is this not just
> 

sorry for this, seems I sent a WIP version by accident.., I'll fix it soon.

Thanks,
Hao