On Tue, Aug 13, 2024 at 2:44 PM Bernd Schubert <bernd.schubert@xxxxxxxxxxx> wrote: > > On 8/13/24 23:21, Joanne Koong wrote: > > Add FOPEN_FETCH_ATTR flag to indicate that attributes should be > > fetched from the server after an open. > > > > For fuse servers that are backed by network filesystems, this is > > needed to ensure that file attributes are up to date between > > consecutive open calls. > > > > For example, if there is a file that is opened on two fuse mounts, > > in the following scenario: > > > > on mount A, open file.txt w/ O_APPEND, write "hi", close file > > on mount B, open file.txt w/ O_APPEND, write "world", close file > > on mount A, open file.txt w/ O_APPEND, write "123", close file > > > > when the file is reopened on mount A, the file inode contains the old > > size and the last append will overwrite the data that was written when > > the file was opened/written on mount B. > > > > (This corruption can be reproduced on the example libfuse passthrough_hp > > server with writeback caching disabled and nopassthrough) > > > > Having this flag as an option enables parity with NFS's close-to-open > > consistency. > > > > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx> > > --- > > fs/fuse/file.c | 7 ++++++- > > include/uapi/linux/fuse.h | 7 ++++++- > > 2 files changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > > index f39456c65ed7..437487ce413d 100644 > > --- a/fs/fuse/file.c > > +++ b/fs/fuse/file.c > > @@ -264,7 +264,12 @@ static int fuse_open(struct inode *inode, struct file *file) > > err = fuse_do_open(fm, get_node_id(inode), file, false); > > if (!err) { > > ff = file->private_data; > > - err = fuse_finish_open(inode, file); > > + if (ff->open_flags & FOPEN_FETCH_ATTR) { > > + fuse_invalidate_attr(inode); > > + err = fuse_update_attributes(inode, file, STATX_BASIC_STATS); > > + } > > + if (!err) > > + err = fuse_finish_open(inode, file); > > if (err) > > fuse_sync_release(fi, ff, file->f_flags); > > else if (is_truncate) > > I didn't come to it yet, but I actually wanted to update Dharmendras/my > atomic open patches - giving up all the vfs changes (for now) and then > always use atomic open if available, for FUSE_OPEN and FUSE_CREATE. And > then update attributes through that. > Would that be an alternative for you? Would basically require to add an > atomic_open method into your file system. > > Definitely more complex than your solution, but avoids a another > kernel/userspace transition. Hi Bernd, Unfortunately I don't think this is an alternative for my use case. I haven't looked closely at the implementation details of your atomic open patchset yet but if I'm understanding the gist of it correctly, it bundles the lookup with the open into 1 request, where the attributes can be passed from server -> kernel through the reply to that request. I think in the case I'm working on, the file open call does not require a lookup so it can't take advantage of your feature. I just tested it on libfuse on the passthrough_hp server (with no writeback caching and nopassthrough) on the example in the commit message and I'm not seeing any lookup request being sent for that last open call (for writing "123"). Thanks, Joanne > > Thanks, > Bernd