public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Luis Henriques <luis@igalia.com>
To: Jingbo Xu <jefflexu@linux.alibaba.com>
Cc: Joanne Koong <joannelkoong@gmail.com>,
	 Miklos Szeredi <miklos@szeredi.hu>,
	 Amir Goldstein <amir73il@gmail.com>,
	 Bernd Schubert <bschubert@ddn.com>,
	 Bernd Schubert <bernd@bsbernd.com>,
	 "Darrick J. Wong" <djwong@kernel.org>,
	 Horst Birthelmer <hbirthelmer@ddn.com>,
	 Kevin Chen <kchen@ddn.com>,
	 linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Matt Harvey <mharvey@jumptrading.com>,
	kernel-dev@igalia.com
Subject: Re: [RFC PATCH v3 6/8] fuse: implementation of lookup_handle+statx compound operation
Date: Thu, 09 Apr 2026 12:10:05 +0100	[thread overview]
Message-ID: <87ika0z8xe.fsf@igalia.com> (raw)
In-Reply-To: <5c13b5ae-ca74-4fd8-97ec-39f24ccfd5c2@linux.alibaba.com> (Jingbo Xu's message of "Thu, 9 Apr 2026 10:27:35 +0800")

On Thu, Apr 09 2026, Jingbo Xu wrote:

> Hi Luis,
>
> On 4/8/26 6:16 PM, Luis Henriques wrote:
>> On Tue, Apr 07 2026, Joanne Koong wrote:
>> 
>>> On Tue, Apr 7, 2026 at 2:20 PM Luis Henriques <luis@igalia.com> wrote:
>>>>
>>>> Hi Joanne,
>>>>
>>>> On Tue, Apr 07 2026, Joanne Koong wrote:
>>>>
>>>>> On Wed, Feb 25, 2026 at 3:25 AM Luis Henriques <luis@igalia.com> wrote:
>>>>>>
>>>>>> The implementation of lookup_handle+statx compound operation extends the
>>>>>> lookup operation so that a file handle is be passed into the kernel.  It
>>>>>> also needs to include an extra inarg, so that the parent directory file
>>>>>> handle can be sent to user-space.  This extra inarg is added as an extension
>>>>>> header to the request.
>>>>>>
>>>>>> By having a separate statx including in a compound operation allows the
>>>>>> attr to be dropped from the lookup_handle request, simplifying the
>>>>>> traditional FUSE lookup operation.
>>>>>>
>>>>>> Signed-off-by: Luis Henriques <luis@igalia.com>
>>>>>> ---
>>>>>>  fs/fuse/dir.c             | 294 +++++++++++++++++++++++++++++++++++---
>>>>>>  fs/fuse/fuse_i.h          |  23 ++-
>>>>>>  fs/fuse/inode.c           |  48 +++++--
>>>>>>  fs/fuse/readdir.c         |   2 +-
>>>>>>  include/uapi/linux/fuse.h |  23 ++-
>>>>>>  5 files changed, 355 insertions(+), 35 deletions(-)
>>>>>>
>>>>>> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
>>>>>> index 113583c4efb6..89e6176abe25 100644
>>>>>> --- a/include/uapi/linux/fuse.h
>>>>>> +++ b/include/uapi/linux/fuse.h
>>>>>>
>>>>>>  enum fuse_opcode {
>>>>>> @@ -671,6 +676,8 @@ enum fuse_opcode {
>>>>>>          */
>>>>>>         FUSE_COMPOUND           = 54,
>>>>>>
>>>>>> +       FUSE_LOOKUP_HANDLE      = 55,
>>>>>> +
>>>>>>         /* CUSE specific operations */
>>>>>>         CUSE_INIT               = 4096,
>>>>>>
>>>>>> @@ -707,6 +714,20 @@ struct fuse_entry_out {
>>>>>>         struct fuse_attr attr;
>>>>>>  };
>>>>>>
>>>>>> +struct fuse_entry2_out {
>>>>>> +       uint64_t        nodeid;
>>>>>> +       uint64_t        generation;
>>>>>> +       uint64_t        entry_valid;
>>>>>> +       uint32_t        entry_valid_nsec;
>>>>>> +       uint32_t        flags;
>>>>>> +       uint64_t        spare;
>>>>>> +};
>>>>>
>>>>> Hi Luis,
>>>>>
>>>>> Could you explain why we need a new struct fuse_entry2_out instead of
>>>>> reusing struct fuse_entry_out? From what i see, the only differences
>>>>> between them are that fuse_entry2_out drops attr_valid,
>>>>> attr_valid_nsec, and struct fuse_attr. Is this done so that it saves
>>>>> the ~100 bytes per lookup? Would it be cleaner from an abi perspective
>>>>> to just reuse fuse_entry_out and ignore the attr fields if they're not
>>>>> necessary? The reason I'm asking is because I'm looking at how you're
>>>>> doing the lookup request reply to see if the fuse passthrough stuff
>>>>> for metadata/directory operations can be combined with it. But I'm not
>>>>> fully understanding why fuse_entry2_out is needed here.
>>>>>
>>>>> I'm also a bit confused by why the compound with statx is needed here,
>>>>> could you explain this part? I see the call to fuse_statx_to_attr()
>>>>> after do_lookup_handle_statx(),  but fuse_statx_to_attr() converts the
>>>>> statx reply right back to a struct fuse_attr for inode setup, so if
>>>>> FUSE_LOOKUP_HANDLE returned struct fuse_entry_out instead of struct
>>>>> fuse_entry2_out, doesn't this solve the problem of needing to compound
>>>>> it with a statx or getattr request? I also noticed that the statx part
>>>>> uses FUSE_ROOT_ID as a workaround for the node id because the actual
>>>>> nodeid isn't known yet, this seems like another sign that the
>>>>> attributes stuff should just be part of the lookup response itself
>>>>> rather than a separate operation?
>>>>
>>>> First of all, thanks a lot for looking into this patchset.  Much
>>>> appreciated!
>>>>
>>>> The main reason for swapping the usage of attr by statx is that statx
>>>> includes some attributes that attr does not (e.g. btime).  And since I was
>>>> adding a new FUSE operation, it would be a good time for using statx
>>>> instead.  (Moreover, as new attributes may be added to statx in the
>>>> future, the benefits of using statx could eventually be even greater.)
>>>>
>>>> This was suggested by Miklos here[0], before converting the whole thing to
>>>> use compound commands.  So, I was going to use fuse_statx in the _out args
>>>> for lookup_handle.  However, because the interface was getting a bit
>>>> complex with extra args (and ext headers!), Miklos ended up suggesting[1]
>>>> to remove attr completely from the lookup_handle operation, and use
>>>> compounds instead to have the full functionality.
>>>
>>> Thank you for the context and links, Luis!
>>>
>>> Using fuse_statx over fuse_getattr makes sense to me for the new
>>> FUSE_LOOKUP_HANDLE op but the part I am confused about is why it needs
>>> to be compounded. If we are adding a new struct (struct
>>> fuse_entry2_out) to the abi, why is it not simpler to just make the
>>> new struct something like:
>>>
>>>   struct fuse_entry_handle_out {
>>>       uint64_t    nodeid;
>>>       uint64_t    generation;
>>>       uint64_t    entry_valid;
>>>       uint64_t    attr_valid;
>>>       uint32_t    entry_valid_nsec;
>>>       uint32_t    attr_valid_nsec;
>>>       struct fuse_statx stat;
>>>   };
>>>
>>> and avoid the compound stuff altogether? Is it because that would make
>>> the struct fuse_entry_handle_out too big to be stack-allocated and
>>> would thus have to be heap allocated? But if I'm recalling correctly,
>>> the compound requests path requires heap allocations as well. Although
>>> at that point if we're gonna have to do the heap allocation, then we
>>> might as well just also embed the struct fuse_file_handle inside
>>> struct fuse_entry_handle_out?
>> 
>> I'm open to drop the usage of compounds for this, of course.  In fact,
>> during the v2 discussions I suggested here[0] the usage of a similar
>> struct fuse_entry_handle_out.  Using compound commands was a way to try to
>> simplify the interface.  But maybe at that point I was too quick at
>> jumping into the compound commands suggestion.  It may make sense to
>> reevaluate this decision if you think it simplifies things, specially for
>> passthrough.
>> 
>> [0] https://lore.kernel.org/all/87zf6nov6c.fsf@wotan.olymp
>> 
>
> Many thanks for your work on this.
>
> Just FYI We are also interested in the kernel-maintained filehandle,
> which can dramatically help reduce the memory footprint on the FUSE
> server side.

Awesome!

> It just seems that the overall design is still not clear right now.  I'd
> like to and would be happy if I could help any.

Thanks you, Jingbo.  I guess that for now we need to revalidate the usage
of compounds or going back to my previous (v2) approach.  If we end up
deciding to drop compound commands, I can try to get a new version of the
patchset before LSFMM so that we can eventually discuss more details
there.

Cheers,
-- 
Luís

  reply	other threads:[~2026-04-09 11:10 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-25 11:24 [RFC PATCH v3 0/8] fuse: LOOKUP_HANDLE operation Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 1/8] fuse: simplify fuse_lookup_name() interface Luis Henriques
2026-02-27 15:46   ` Miklos Szeredi
2026-02-28 14:42     ` Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 2/8] fuse: export extend_arg() and factor out fuse_ext_size() Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 3/8] fuse: store index of the variable length argument Luis Henriques
2026-02-27 15:41   ` Miklos Szeredi
2026-02-28 14:50     ` Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 4/8] fuse: drop unnecessary argument from fuse_lookup_init() Luis Henriques
2026-02-27 15:57   ` Miklos Szeredi
2026-02-25 11:24 ` [RFC PATCH v3 5/8] fuse: extract helper functions from fuse_do_statx() Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 6/8] fuse: implementation of lookup_handle+statx compound operation Luis Henriques
2026-02-25 18:06   ` Amir Goldstein
2026-02-26  9:54     ` Luis Henriques
2026-02-26 10:08       ` Amir Goldstein
2026-02-26 10:29         ` Miklos Szeredi
2026-02-26 15:06           ` Luis Henriques
2026-02-26 15:44             ` Miklos Szeredi
2026-02-26 16:17               ` Luis Henriques
2026-02-26 10:33         ` Luis Henriques
2026-04-07 17:43   ` Joanne Koong
2026-04-07 21:20     ` Luis Henriques
2026-04-07 23:06       ` Joanne Koong
2026-04-07 23:24         ` Joanne Koong
2026-04-07 23:38           ` Joanne Koong
2026-04-08 10:22           ` Luis Henriques
2026-04-08 15:15             ` Joanne Koong
2026-04-08 10:16         ` Luis Henriques
2026-04-08 15:05           ` Joanne Koong
2026-04-09 10:46             ` Luis Henriques
2026-04-09  2:27           ` Jingbo Xu
2026-04-09 11:10             ` Luis Henriques [this message]
2026-04-09 14:03             ` Amir Goldstein
2026-02-25 11:24 ` [RFC PATCH v3 7/8] fuse: export fuse_open_args_fill() helper function Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 8/8] fuse: implementation of mkobj_handle+statx+open compound operation Luis Henriques
2026-02-25 15:08   ` Horst Birthelmer
2026-02-25 17:26     ` Luis Henriques
2026-02-25 15:14 ` [RFC PATCH v3 0/8] fuse: LOOKUP_HANDLE operation Horst Birthelmer
2026-02-25 17:06   ` Luis Henriques

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ika0z8xe.fsf@igalia.com \
    --to=luis@igalia.com \
    --cc=amir73il@gmail.com \
    --cc=bernd@bsbernd.com \
    --cc=bschubert@ddn.com \
    --cc=djwong@kernel.org \
    --cc=hbirthelmer@ddn.com \
    --cc=jefflexu@linux.alibaba.com \
    --cc=joannelkoong@gmail.com \
    --cc=kchen@ddn.com \
    --cc=kernel-dev@igalia.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mharvey@jumptrading.com \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox