Re: [PATCH v14 00/12] FUSE passthrough for file io

[Bernd Schubert <bernd.schubert@fastmail.fm>]

On 12/7/23 08:23, Amir Goldstein wrote:
> On Thu, Dec 7, 2023 at 1:11 AM Bernd Schubert
> <bernd.schubert@fastmail.fm> wrote:
>>
>> Hi Amir,
>>
>>
>> On 12/6/23 10:59, Amir Goldstein wrote:
>>> On Wed, Nov 29, 2023 at 6:55 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>>
>>>> On Wed, 29 Nov 2023 at 16:52, Amir Goldstein <amir73il@gmail.com> wrote:
>>>>
>>>>> direct I/O read()/write() is never a problem.
>>>>>
>>>>> The question is whether mmap() on a file opened with FOPEN_DIRECT_IO
>>>>> when the inode is in passthrough mode, also uses fuse_passthrough_mmap()?
>>>>
>>>> I think it should.
>>>>
>>>>> or denied, similar to how mmap with ff->open_flags & FOPEN_DIRECT_IO &&
>>>>> vma->vm_flags & VM_MAYSHARE) && !fc->direct_io_relax
>>>>> is denied?
>>>>
>>>> What would be the use case for FOPEN_DIRECT_IO with passthrough mmap?
>>>>
>>>>> A bit more challenging, because we will need to track unmounts, or at
>>>>> least track
>>>>> "was_cached_mmaped" state per file, but doable.
>>>>
>>>> Tracking unmaps via fuse_vma_close() should not be difficult.
>>>>
>>>
>>> I think that it is.
>>>
>>> fuse_vma_close() does not seem to be balanced with fuse_file_mmap()
>>> because IIUC, maps can be cloned via fork() etc.
>>>
>>> It tried to implement an iocachectr refcount to track cache mmaps,
>>> but it keeps underflowing in fuse_vma_close().
>>>
>>> I would like us to consider a slightly different model.
>>>
>>> We agreed that caching and passthrough mode on the same
>>> inode cannot mix and there is no problem with different modes
>>> per inode on the same filesystem.
>>>
>>> I have a use case for mixing direct_io and passthrough on the
>>> same inode (i.e. inode in passthrough mode).
>>>
>>> I have no use case (yet) for the transition from caching to passthrough
>>> mode on the same inode and direct_io cached mmaps complicate
>>> things quite a bit for this scenario.
>>>
>>> My proposal is to taint a direct_io file with FOPEN_CACHE_MMAP
>>> if it was ever mmaped using page cache.
>>> We will not try to clean this flag in fuse_vma_close(), it stays with
>>> the file until release.
>>>
>>> An FOPEN_CACHE_MMAP file forces an inode into caching mode,
>>> same as a regular caching open.
>>
>> where do you actually want to set that flag? My initial idea for
>> FUSE_I_CACHE_WRITES was to set that in fuse_file_mmap, but I would have
>> needed the i_rwsem lock and that resulted in a lock ordering issue.
>>
> 
> Yes, the idea is to set this flag on the first mmap of a FOPEN_DIRECT_IO file.
> Why does that require an i_rwsem lock?
> 
> Before setting FUSE_I_CACHE_WRITES inode flag, your patch does:
>      wait_event(fi->direct_io_waitq, fi->shared_lock_direct_io_ctr == 0);
> 
> We can do the same in direct_io mmap, before setting the
> FOPEN_CACHE_MMAP file flag and consequently changing inode
> mode to caching. No?


Yeah right, that will work.

Thanks,
Bernd