On Thu, Dec 7, 2023 at 1:11 AM Bernd Schubert
<bernd.schubert@xxxxxxxxxxx> wrote:
>
> Hi Amir,
>
>
> On 12/6/23 10:59, Amir Goldstein wrote:
> > On Wed, Nov 29, 2023 at 6:55 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> >>
> >> On Wed, 29 Nov 2023 at 16:52, Amir Goldstein <amir73il@xxxxxxxxx> 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?
I will try to prepare a patch today to demonstrate.
Thanks,
Amir.
