Re: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr

[Joanne Koong]

On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
>
> On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@xxxxxxxxxxxxx> wrote:
> >
> > On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > > On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@xxxxxxxxxxxxxx> wrote:
> > > >
> > > > From: Horst Birthelmer <hbirthelmer@xxxxxxx>
> > > >
> > > > The discussion about compound commands in fuse was
> > > > started over an argument to add a new operation that
> > > > will open a file and return its attributes in the same operation.
> > > >
> > > > Here is a demonstration of that use case with compound commands.
> > > >
> > > > Signed-off-by: Horst Birthelmer <hbirthelmer@xxxxxxx>
> > > > ---
> > > >  fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > > >  fs/fuse/fuse_i.h |   4 +-
> > > >  fs/fuse/ioctl.c  |   2 +-
> > > >  3 files changed, 99 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > > > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > > > --- a/fs/fuse/file.c
> > > > +++ b/fs/fuse/file.c
> > > >  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> > > > -                                unsigned int open_flags, bool isdir)
> > > > +                               struct inode *inode,
> > >
> > > As I understand it, now every open() is a opengetattr() (except for
> > > the ioctl path) but is this the desired behavior? for example if there
> > > was a previous FUSE_LOOKUP that was just done, doesn't this mean
> > > there's no getattr that's needed since the lookup refreshed the attrs?
> > > or if the server has reasonable entry_valid and attr_valid timeouts,
> > > multiple opens() of the same file would only need to send FUSE_OPEN
> > > and not the FUSE_GETATTR, no?
> >
> > So your concern is, that we send too many requests?
> > If the fuse server implwments the compound that is not the case.
> >
>
> My concern is that we're adding unnecessary overhead for every open
> when in most cases, the attributes are already uptodate. I don't think
> we can assume that the server always has attributes locally cached, so
> imo the extra getattr is nontrivial (eg might require having to
> stat()).

Looking at where the attribute valid time gets set... it looks like
this gets stored in fi->i_time (as per
fuse_change_attributes_common()), so maybe it's better to only send
the compound open+getattr if time_before64(fi->i_time,
get_jiffies_64()) is true, otherwise only the open is needed. This
doesn't solve the O_APPEND data corruption bug seen in [1] but imo
this would be a more preferable way of doing it.

Thanks,
Joanne

[1] https://lore.kernel.org/linux-fsdevel/20240813212149.1909627-1-joannelkoong@xxxxxxxxx/

>
> Thanks,
> Joanne