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

[Darrick J. Wong]

On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> 
> 
> On 3/2/26 19:56, Joanne Koong wrote:
> > On Sat, Feb 28, 2026 at 12:14 AM Horst Birthelmer <horst@xxxxxxxxxxxxx> wrote:
> >>
> >> 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.

/me notes that NFS can corrupt O_APPEND writes if you're not careful to
synchronize writers at the application level...

> >> 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).
> > 
> > The overhead for the server to fetch the attributes may be nontrivial
> > (eg may require stat()). I really don't think we can assume the data
> > is locally cached somewhere. Why always compound the getattr to the
> > open instead of only compounding the getattr when the attributes are
> > actually invalid?
> > 
> > But maybe I'm wrong here and this is the preferable way of doing it.
> > Miklos, could you provide your input on this?
> 
> Personally I would see it as change of behavior if out of the sudden
> open is followed by getattr. In my opinion fuse server needs to make a
> decision that it wants that. Let's take my favorite sshfs example with a
> 1s latency - it be very noticeable if open would get slowed down by
> factor 2.

I wonder, since O_APPEND writes supposedly reposition the file position
to i_size before every write, can we enlarge the write reply so that the
fuse server could tell the client what i_size is supposed to be after
every write?  Or perhaps add a notification so a network filesystem
could try to keep the kernel uptodate after another node appends to a
file?

Just my unqualified opinion ;)

--D

> Thanks,
> Bernd
> 
> 
>