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.
We could allow server to set FOPEN_CACHE_MMAP along with
FOPEN_DIRECT_IO to preemptively deny future passthrough open,
but not sure this is important.
If we wanted to, we could let this flag combination have the same
meaning as direct_io_allow_mmap, but per file/inode.
In relation to the FOPEN_PARALLEL_DIRECT_WRITES vs.
FUSE_DIRECT_IO_ALLOW_MMAP discussion, Bernd has suggested
a per inode FUSE_I_CACHE_WRITES, state that tracks if caching writes
were ever done on inode to allow parallel dio on the rest of the inodes
in the filesystem.
FUSE_I_CACHE_WRITES is a sub-state of caching mode inode state.
I think maybe caching mode would be enough for both use cases -
preventing parallel dio and preventing passthrough open.
The result would be that parallel dio would be performed on inodes that
are not currently open in caching mode and have not been mmaped
at all (regardless of writes to page cache) using any of the currently
open direct_io files.
As long as the applications that use mmap write (e.g. compiler)
do not usually work on the same files as the applications that do
parallel dio writes (e.g. db) and as long as files that are typically mmaped
privately (exe and libs) don't need parallel dio writes,
I think that FUSE_I_CACHE_WRITES state will not be needed.
But maybe I am missing some cases. In any case, there is nothing
preventing FUSE_I_CACHE_WRITES to exist along side caching mode
if needed.
From the description is sounds like the caching mode inode state should
be enough for FOPEN_PARALLEL_DIRECT_WRITES/FUSE_DIRECT_IO_ALLOW_MMAP.
I'm just not sure where the flag will be set (as above).
I guess at some point also use cases might come up that need MMAP +
parallel DIO, at different times or different offsets. Though, before
making the code even more complex I would first like to see a real world
use case for that. Next part for me is to rebase the dio consolidation
branch and to get a shared lock for O_DIRECT (without the need for
FOPEN_DIRECT_IO).
Thanks,
Bernd
