On Sun, 24 Oct 2021 at 17:30, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Sun, Oct 24, 2021 at 6:17 PM Shachar Sharon <synarete@xxxxxxxxx> wrote: > > > > On Sun, Oct 24, 2021 at 05:21:55PM +0300, Amir Goldstein wrote: > > >On Sun, Oct 24, 2021 at 4:55 PM Shachar Sharon <synarete@xxxxxxxxx> wrote: > > >> > > >> On Sun, Oct 24, 2021 at 04:26:07PM +0300, Amir Goldstein wrote: > > >> >Add flag returned by OPENDIR request to avoid flushing data cache > > >> >on close. > > >> > > > >> I believe this holds not only for FUSE_OPENDIR but also to FUSE_OPEN and > > >> FUSE_CREATE (see 'struct fuse_open_out'). > > >> > > > > > >Oops that was a copy&paste typo. > > >Of course this is only relevant for FUSE_OPEN and FUSE_CREATE. Fixed up. > > >> > fs/fuse/file.c | 3 +++ > > >> > include/uapi/linux/fuse.h | 3 +++ > > >> > 2 files changed, 6 insertions(+) > > >> > > > >> >diff --git a/fs/fuse/file.c b/fs/fuse/file.c > > >> >index 11404f8c21c7..6f502a76f9ac 100644 > > >> >--- a/fs/fuse/file.c > > >> >+++ b/fs/fuse/file.c > > >> >@@ -483,6 +483,9 @@ static int fuse_flush(struct file *file, fl_owner_t id) > > >> > if (fuse_is_bad(inode)) > > >> > return -EIO; > > >> > > > >> >+ if (ff->open_flags & FOPEN_NOFLUSH) This needs to check for !fc->writeback_cache. Fixed up. Without this dirty pages could persist after the file is finally closed, which is undesirable for several reasons. One is that a writably open file is no longer guaranteed to exist (needed for filling fuse_write_in::fh). Another is that writing out pages from memory reclaim is deadlock prone. So even if we could omit the fh field we'd need to make sure that reclaim doesn't wait for fuse writeback. So while having dirty pages beyond last close(2) could be really useful, it's not something we can currently do. Note: while mainline currently does flush pages from ->release(), that's wrong, and removed in fuse.git#for_next. > > >> >--- a/include/uapi/linux/fuse.h > > >> >+++ b/include/uapi/linux/fuse.h > > >> >@@ -184,6 +184,7 @@ > > >> > * > > >> > * 7.34 > > >> > * - add FUSE_SYNCFS > > >> Most likely you want to bump to 7.35; 7.34 is already out in the wild > > >> (e.g., on my Fedora33 workstation) > > >> > > > > > >Possibly. I wasn't sure what was the rationale behind when the version > > >should be bumped. > > >One argument against bumping the version is that there is not much > > >harm is passing this flag to an old kernel - it just ignored the flag > > >and sends the flush requests anyway. > > Miklos, do I need to bump the protocol version? We've historically done that. But the last minor version that is actually checked by the kernel or libfuse was 23 (the fuse_init_out expansion), so nothing bad would happen if for some reason the bump was forgotten. I've fixed this up too and pushed out to #for-next. Thanks, Miklos
