On Tue, Aug 20, 2024 at 2:18 PM Bernd Schubert <bschubert@xxxxxxx> wrote: > > This is to update attributes on open to achieve close-to-open > coherency even if an inode has a attribute cache timeout. > > Signed-off-by: Bernd Schubert <bschubert@xxxxxxx> > > --- > libfuse patch: > https://github.com/libfuse/libfuse/pull/1020 > (FUSE_OPENDIR_GETATTR still missing at time of writing) > > Note: This does not make use of existing atomic-open patches > as these are more complex than two new opcodes for open-getattr. > > Note2: This is an alternative to Joannes patch that adds > FOPEN_FETCH_ATTR, which would need to kernel/userspace transitions > https://lore.kernel.org/all/20240813212149.1909627-1-joannelkoong@xxxxxxxxx/ > > Question for reviewers: > - Should this better use statx fields? Probably not needed for > coherency? > - Should this introduce a new struct that contains > struct fuse_open_out and struct fuse_attr_out, with > additional padding between them to avoid incompat issues > if either struct should be extended? > --- > fs/fuse/file.c | 94 ++++++++++++++++++++++++++++++++++++++- > fs/fuse/fuse_i.h | 7 +++ > fs/fuse/ioctl.c | 2 +- > include/uapi/linux/fuse.h | 5 +++ > 4 files changed, 105 insertions(+), 3 deletions(-) > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index f39456c65ed7..d470e6a2b3d4 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -51,6 +51,78 @@ static int fuse_send_open(struct fuse_mount *fm, u64 nodeid, > return fuse_simple_request(fm, &args); > } > > +/* > + * Open the file and update inode attributes > + */ > +static int fuse_file_open_getattr(struct fuse_mount *fm, u64 nodeid, > + struct inode *inode, unsigned int open_flags, > + int opcode, > + struct fuse_open_out *open_outargp) > +{ > + struct fuse_conn *fc = fm->fc; > + u64 attr_version = fuse_get_attr_version(fc); > + struct fuse_open_in inarg; > + struct fuse_attr_out attr_outarg; > + FUSE_ARGS(args); > + int err; > + > + /* convert the opcode from plain open to open-with-getattr */ > + if (opcode == FUSE_OPEN) { > + if (fc->no_open_getattr) > + return -ENOSYS; > + opcode = FUSE_OPEN_GETATTR; > + } else { > + if (fc->no_opendir_getattr) > + return -ENOSYS; > + opcode = FUSE_OPENDIR_GETATTR; > + } > + > + memset(&inarg, 0, sizeof(inarg)); > + inarg.flags = open_flags & ~(O_CREAT | O_EXCL | O_NOCTTY); > + if (!fm->fc->atomic_o_trunc) > + inarg.flags &= ~O_TRUNC; > + > + if (fm->fc->handle_killpriv_v2 && > + (inarg.flags & O_TRUNC) && !capable(CAP_FSETID)) { > + inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID; > + } > + > + args.opcode = opcode; > + args.nodeid = nodeid; > + args.in_numargs = 1; > + args.in_args[0].size = sizeof(inarg); > + args.in_args[0].value = &inarg; > + args.out_numargs = 2; > + args.out_args[0].size = sizeof(*open_outargp); > + args.out_args[0].value = open_outargp; > + args.out_args[1].size = sizeof(attr_outarg); > + args.out_args[1].value = &attr_outarg; > + > + err = fuse_simple_request(fm, &args); > + if (err) { > + if (err == -ENOSYS) { > + if (opcode == FUSE_OPEN) > + fc->no_open_getattr = 1; > + else > + fc->no_opendir_getattr = 1; > + } > + return err; > + } > + > + err = -EIO; > + if (fuse_invalid_attr(&attr_outarg.attr) || > + inode_wrong_type(inode, attr_outarg.attr.mode)) { > + fuse_make_bad(inode); > + return err; > + } > + > + fuse_change_attributes(inode, &attr_outarg.attr, NULL, > + ATTR_TIMEOUT(&attr_outarg), attr_version); > + Hi Bernrd, For my use case, it'd be preferred if this could be gated by an FOPEN flag that the server can set in the reply if it doesn't want to opt into the refreshed attributes (eg if this flag is set, then the getattr values in the reply are blank and the kernel should skip fuse_change_attributes()). For the use case I have, the attributes only need to be fetched and changed for O_APPENDs where it'd be ideal if we could skip this overhead for non O_APPENDs. This flag can be added separately after your patch lands, but just wanted to note this now as a heads-up. I'm also happy to add this in later if it's better to do this later than as part of this patch. Thanks, Joanne
