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

[Horst Birthelmer]

On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
> 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.
> 
Don't take this as an objection. I'm looking for arguments, since my defense
was always the line I used above (if the fuse server implements the compound,
it's one call).

What made you change your mind from avoiding the data corruption to worry 
about the number of stat calls done? Since you were the one reporting the
issue and even providing a fix.

ATM I would rather have data consistency than fewer requests during open.

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