[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87wm2lp3oq.fsf@wotan.olymp>
Date: Wed, 17 Dec 2025 14:45:41 +0000
From: Luis Henriques <luis@...lia.com>
To: Amir Goldstein <amir73il@...il.com>
Cc: Bernd Schubert <bschubert@....com>, Miklos Szeredi <miklos@...redi.hu>,
"Darrick J. Wong" <djwong@...nel.org>, Kevin Chen <kchen@....com>,
Horst Birthelmer <hbirthelmer@....com>, "linux-fsdevel@...r.kernel.org"
<linux-fsdevel@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, Matt Harvey <mharvey@...ptrading.com>,
"kernel-dev@...lia.com" <kernel-dev@...lia.com>
Subject: Re: [RFC PATCH v2 4/6] fuse: implementation of the
FUSE_LOOKUP_HANDLE operation
On Wed, Dec 17 2025, Amir Goldstein wrote:
> On Tue, Dec 16, 2025 at 12:48 PM Luis Henriques <luis@...lia.com> wrote:
>>
>> On Mon, Dec 15 2025, Bernd Schubert wrote:
>>
>> > On 12/12/25 19:12, Luis Henriques wrote:
>> >> The implementation of LOOKUP_HANDLE modifies the LOOKUP operation to include
>> >> an extra inarg: the file handle for the parent directory (if it is
>> >> available). Also, because fuse_entry_out now has a extra variable size
>> >> struct (the actual handle), it also sets the out_argvar flag to true.
>> >>
>> >> Most of the other modifications in this patch are a fallout from these
>> >> changes: because fuse_entry_out has been modified to include a variable size
>> >> struct, every operation that receives such a parameter have to take this
>> >> into account:
>> >>
>> >> CREATE, LINK, LOOKUP, MKDIR, MKNOD, READDIRPLUS, SYMLINK, TMPFILE
>> >>
>> >> Signed-off-by: Luis Henriques <luis@...lia.com>
>> >> ---
>> >> fs/fuse/dev.c | 16 +++++++
>> >> fs/fuse/dir.c | 87 ++++++++++++++++++++++++++++++---------
>> >> fs/fuse/fuse_i.h | 34 +++++++++++++--
>> >> fs/fuse/inode.c | 69 +++++++++++++++++++++++++++----
>> >> fs/fuse/readdir.c | 10 ++---
>> >> include/uapi/linux/fuse.h | 8 ++++
>> >> 6 files changed, 189 insertions(+), 35 deletions(-)
>> >>
>> >> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>> >> index 629e8a043079..fc6acf45ae27 100644
>> >> --- a/fs/fuse/dev.c
>> >> +++ b/fs/fuse/dev.c
>> >> @@ -606,6 +606,22 @@ static void fuse_adjust_compat(struct fuse_conn *fc, struct fuse_args *args)
>> >> if (fc->minor < 4 && args->opcode == FUSE_STATFS)
>> >> args->out_args[0].size = FUSE_COMPAT_STATFS_SIZE;
>> >>
>> >> + if (fc->minor < 45) {
>> >
>> > Could we use fc->lookup_handle here? Numbers are hard with backports
>>
>> To be honest, I'm not sure this code is correct. I just followed the
>> pattern. I'll need to dedicate some more time looking into this,
>> specially because the READDIRPLUS op handling is still TBD.
>>
>> <snip>
>>
>> >> @@ -505,6 +535,30 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
>> >> if (!inode)
>> >> return NULL;
>> >>
>> >> + fi = get_fuse_inode(inode);
>> >> + if (fc->lookup_handle) {
>> >> + if ((fh == NULL) && (nodeid != FUSE_ROOT_ID)) {
>> >> + pr_err("NULL file handle for nodeid %llu\n", nodeid);
>> >> + iput(inode);
>> >> + return NULL;
>> >
>> > Hmm, so there are conditions like "if (fi && fi->fh) {" in lookup and I
>> > was thinking "nice, fuse-server can decide to skip the fh for some
>> > inodes like FUSE_ROOT_ID. But now it gets forbidden here. In combination
>> > with the other comment in fuse_inode_handle_alloc(), could be allocate
>> > here to the needed size and allow fuse-server to not send the handle
>> > for some files?
>>
>> I'm not sure the code is consistent with this regard, but here I'm doing
>> exactly that: allowing the fh to be NULL only for FUSE_ROOT_ID. Or did I
>> misunderstood your comment?
>>
>
> root inode is a special case.
> The NFS server also does not encode the file handle for export root as
> far as a I know
> it just sends the special file handle type FILEID_ROOT to describe the
> root inode
> without any blob unique, so FUSE can do the same.
OK, that makes sense.
> There is not much point in "looking up" the root inode neither by nodeid
> nor by handle. unless is for making the code more generic.
>
> I am not sure if FUSE server restart is supposed to revalidate the
> root inode by file handle. That's kind of an administrative question about
> the feature. My feeling is that it is not needed.
Thanks, Amir. Looks like there's a lot in these v2 review comments that
I'll need to go through. I'll try to put everything together and see what
I can cook for v3.
Cheers,
--
Luís
Powered by blists - more mailing lists