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

[Bernd Schubert <bschubert@ddn.com>]

On 3/2/26 19:56, Joanne Koong wrote:
> On Sat, Feb 28, 2026 at 12:14 AM Horst Birthelmer <horst@birthelmer.de> 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@gmail.com> wrote:
>>>>
>>>> On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> 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@birthelmer.com> wrote:
>>>>>>>
>>>>>>> From: Horst Birthelmer <hbirthelmer@ddn.com>
>>>>>>>
>>>>>>> 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@ddn.com>
>>>>>>> ---
>>>>>>>  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).
> 
> 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.

Thanks,
Bernd