From: Luis Henriques <luis@igalia.com>
To: Joanne Koong <joannelkoong@gmail.com>
Cc: 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: Wed, 08 Apr 2026 11:22:22 +0100 [thread overview]
Message-ID: <87bjftlpk1.fsf@igalia.com> (raw)
In-Reply-To: <CAJnrk1b9CkQ0i6FdFvxcKbhS7K4TG5yQKbnEXV=Hc+ry593KoQ@mail.gmail.com> (Joanne Koong's message of "Tue, 7 Apr 2026 16:24:50 -0700")
On Tue, Apr 07 2026, Joanne Koong wrote:
> On Tue, Apr 7, 2026 at 4:06 PM Joanne Koong <joannelkoong@gmail.com> 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 {
>
> Thinking more about how passthrough will use this... I'm thinking
> something like this would be ideal (though please correct me if I'm
> completely off track here, Amir):
>
> 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;
> int32_t backing_id;
> uint32_t flags;
> struct fuse_statx statx;
> };
>
> where if the inode is not passed through, the kernel uses the stax
> field for the attributes but if the inode is passed through and the
> server sets a bitmask specifying that attributes should be derived
> from the backing file, then the statx field will be ignored by the
> server and kernel).
If we end up deciding to revert back to not using compound commands I'll
have to take a closer look into this, as it's not clear what the above
means. For example, when you say:
"... if the inode is not passed through"
does this "inode" refer to the backing_id? I guess this is some index to
backing ids being used by FUSE, not exactly an inode, but I'm not very
familiar with the code.
Cheers,
--
Luís
next prev parent reply other threads:[~2026-04-08 10:22 UTC|newest]
Thread overview: 35+ 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 [this message]
2026-04-08 15:15 ` Joanne Koong
2026-04-08 10:16 ` Luis Henriques
2026-04-08 15:05 ` Joanne Koong
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=87bjftlpk1.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=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