Re: [RFC PATCH v2 6/6] fuse: implementation of export_operations with FUSE_LOOKUP_HANDLE

[Luis Henriques <luis@igalia.com>]

On Tue, Dec 16 2025, Amir Goldstein wrote:

> On Fri, Dec 12, 2025 at 7:13 PM Luis Henriques <luis@igalia.com> wrote:
>>
>> This patch allows the NFS handle to use the new file handle provided by the
>> LOOKUP_HANDLE operation.  It still allows the usage of nodeid+generation as
>> an handle if this operation is not supported by the FUSE server or if no
>> handle is available for a specific inode.  I.e. it can mix both file handle
>> types FILEID_INO64_GEN{_PARENT} and FILEID_FUSE_WITH{OUT}_PARENT.
>
> Why the mix? I dont get it.

Well, my thought was that if we have a nodeid for which we don't have a
file handle (FUSE_NODE_ID is the only example I can think of), we still
need to encode it.  And the idea was to use the old FILEID_INO64_GEN in
those cases, even if all other nodeids use the new FILEID.  But I guess we
can simply use the nodeid+gen file handle with the new FILEID anyway.

>>
>> Signed-off-by: Luis Henriques <luis@igalia.com>
>> ---
>>  fs/fuse/export.c         | 162 ++++++++++++++++++++++++++++++++++++---
>>  include/linux/exportfs.h |   7 ++
>>  2 files changed, 160 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/fuse/export.c b/fs/fuse/export.c
>> index 4a9c95fe578e..b40d146a32f2 100644
>> --- a/fs/fuse/export.c
>> +++ b/fs/fuse/export.c
>> @@ -3,6 +3,7 @@
>>   * FUSE NFS export support.
>>   *
>>   * Copyright (C) 2001-2008  Miklos Szeredi <miklos@szeredi.hu>
>> + * Copyright (C) 2025 Jump Trading LLC, author: Luis Henriques <luis@igalia.com>
>>   */
>>
>>  #include "fuse_i.h"
>> @@ -10,7 +11,8 @@
>>
>>  struct fuse_inode_handle {
>>         u64 nodeid;
>> -       u32 generation;
>> +       u32 generation; /* XXX change to u64, and use fid->i64.ino in encode/decode? */
>> +       struct fuse_file_handle fh;
>
> If anything this should be a union
> or maybe I don't understand what you were trying to accomplish.
> Please try to explain the design better in the commit message.

As Miklos also suggested using either nodeid+gen or the new fh, I guess it
makes sense to use a union here.

>>  };
>>
>>  static struct dentry *fuse_get_dentry(struct super_block *sb,
>> @@ -67,8 +69,8 @@ static struct dentry *fuse_get_dentry(struct super_block *sb,
>>         return ERR_PTR(err);
>>  }
>>
>> -static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
>> -                          struct inode *parent)
>> +static int fuse_encode_gen_fh(struct inode *inode, u32 *fh, int *max_len,
>> +                             struct inode *parent)
>>  {
>>         int len = parent ? 6 : 3;
>>         u64 nodeid;
>> @@ -96,38 +98,180 @@ static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
>>         }
>>
>>         *max_len = len;
>> +
>>         return parent ? FILEID_INO64_GEN_PARENT : FILEID_INO64_GEN;
>>  }
>>
>> -static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
>> -               struct fid *fid, int fh_len, int fh_type)
>> +static int fuse_encode_fuse_fh(struct inode *inode, u32 *fh, int *max_len,
>> +                              struct inode *parent)
>> +{
>> +       struct fuse_inode *fi = get_fuse_inode(inode);
>> +       struct fuse_inode *fip = NULL;
>> +       struct fuse_inode_handle *handle = (void *)fh;
>> +       int type = FILEID_FUSE_WITHOUT_PARENT;
>> +       int len, lenp = 0;
>> +       int buflen = *max_len << 2; /* max_len: number of words */
>> +
>> +       len = sizeof(struct fuse_inode_handle) + fi->fh->size;
>> +       if (parent) {
>> +               fip = get_fuse_inode(parent);
>> +               if (fip->fh && fip->fh->size) {
>> +                       lenp = sizeof(struct fuse_inode_handle) +
>> +                               fip->fh->size;
>> +                       type = FILEID_FUSE_WITH_PARENT;
>> +               }
>> +       }
>> +
>> +       if (buflen < (len + lenp)) {
>> +               *max_len = (len + lenp) >> 2;
>> +               return  FILEID_INVALID;
>> +       }
>> +
>> +       handle[0].nodeid = fi->nodeid;
>> +       handle[0].generation = inode->i_generation;
>> +       memcpy(&handle[0].fh, fi->fh, len);
>> +       if (lenp) {
>> +               handle[1].nodeid = fip->nodeid;
>> +               handle[1].generation = parent->i_generation;
>> +               memcpy(&handle[1].fh, fip->fh, lenp);
>> +       }
>> +
>> +       *max_len = (len + lenp) >> 2;
>> +
>> +       return type;
>> +}
>> +
>> +static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
>> +                          struct inode *parent)
>> +{
>> +       struct fuse_conn *fc = get_fuse_conn(inode);
>> +       struct fuse_inode *fi = get_fuse_inode(inode);
>> +
>> +       if (fc->lookup_handle && fi->fh && fi->fh->size)
>> +               return fuse_encode_fuse_fh(inode, fh, max_len, parent);
>> +
>> +       return fuse_encode_gen_fh(inode, fh, max_len, parent);
>> +}
>> +
>> +static struct dentry *fuse_fh_gen_to_dentry(struct super_block *sb,
>> +                                           struct fid *fid, int fh_len)
>>  {
>>         struct fuse_inode_handle handle;
>>
>> -       if ((fh_type != FILEID_INO64_GEN &&
>> -            fh_type != FILEID_INO64_GEN_PARENT) || fh_len < 3)
>> +       if (fh_len < 3)
>
> I dont understand why this was changed.

The reason is because fuse_fh_to_dentry() is the only caller of this
function.  Since we already know the type is correct (fuse_fh_to_dentry()
checks it), there's not point checking it again here.

>>                 return NULL;
>>
>>         handle.nodeid = (u64) fid->raw[0] << 32;
>>         handle.nodeid |= (u64) fid->raw[1];
>>         handle.generation = fid->raw[2];
>> +
>>         return fuse_get_dentry(sb, &handle);
>>  }
>>
>> -static struct dentry *fuse_fh_to_parent(struct super_block *sb,
>> +static struct dentry *fuse_fh_fuse_to_dentry(struct super_block *sb,
>> +                                            struct fid *fid, int fh_len)
>> +{
>> +       struct fuse_inode_handle *handle;
>> +       struct dentry *dentry;
>> +       int len = sizeof(struct fuse_file_handle);
>> +
>> +       handle = (void *)fid;
>> +       len += handle->fh.size;
>> +
>> +       if ((fh_len << 2) < len)
>> +               return NULL;
>> +
>> +       handle = kzalloc(len, GFP_KERNEL);
>> +       if (!handle)
>> +               return NULL;
>> +
>> +       memcpy(handle, fid, len);
>> +
>> +       dentry = fuse_get_dentry(sb, handle);
>> +       kfree(handle);
>> +
>> +       return dentry;
>> +}
>> +
>> +static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
>>                 struct fid *fid, int fh_len, int fh_type)
>> +{
>> +       switch (fh_type) {
>> +       case FILEID_INO64_GEN:
>> +       case FILEID_INO64_GEN_PARENT:
>> +               return fuse_fh_gen_to_dentry(sb, fid, fh_len);
>> +       case FILEID_FUSE_WITHOUT_PARENT:
>> +       case FILEID_FUSE_WITH_PARENT:
>> +               return fuse_fh_fuse_to_dentry(sb, fid, fh_len);
>> +       }
>> +
>> +       return NULL;
>> +
>> +}
>> +
>> +static struct dentry *fuse_fh_gen_to_parent(struct super_block *sb,
>> +                                           struct fid *fid, int fh_len)
>>  {
>>         struct fuse_inode_handle parent;
>>
>> -       if (fh_type != FILEID_INO64_GEN_PARENT || fh_len < 6)
>> +       if (fh_len < 6)
>>                 return NULL;
>>
>>         parent.nodeid = (u64) fid->raw[3] << 32;
>>         parent.nodeid |= (u64) fid->raw[4];
>>         parent.generation = fid->raw[5];
>> +
>>         return fuse_get_dentry(sb, &parent);
>>  }
>>
>> +static struct dentry *fuse_fh_fuse_to_parent(struct super_block *sb,
>> +                                            struct fid *fid, int fh_len)
>> +{
>> +       struct fuse_inode_handle *handle;
>> +       struct dentry *dentry;
>> +       int total_len;
>> +       int len;
>> +
>> +       handle = (void *)fid;
>> +       total_len = len = sizeof(struct fuse_inode_handle) + handle->fh.size;
>> +
>> +       if ((fh_len << 2) < total_len)
>> +               return NULL;
>> +
>> +       handle = (void *)(fid + len);
>> +       len = sizeof(struct fuse_file_handle) + handle->fh.size;
>> +       total_len += len;
>> +
>> +       if ((fh_len << 2) < total_len)
>> +               return NULL;
>> +
>> +       handle = kzalloc(len, GFP_KERNEL);
>> +       if (!handle)
>> +               return NULL;
>> +
>> +       memcpy(handle, fid, len);
>> +
>> +       dentry = fuse_get_dentry(sb, handle);
>> +       kfree(handle);
>> +
>> +       return dentry;
>> +}
>> +
>> +static struct dentry *fuse_fh_to_parent(struct super_block *sb,
>> +               struct fid *fid, int fh_len, int fh_type)
>> +{
>> +       switch (fh_type) {
>> +       case FILEID_INO64_GEN:
>> +       case FILEID_INO64_GEN_PARENT:
>> +               return fuse_fh_gen_to_parent(sb, fid, fh_len);
>> +       case FILEID_FUSE_WITHOUT_PARENT:
>> +       case FILEID_FUSE_WITH_PARENT:
>> +               return fuse_fh_fuse_to_parent(sb, fid, fh_len);
>> +       }
>> +
>> +       return NULL;
>> +}
>> +
>>  static struct dentry *fuse_get_parent(struct dentry *child)
>>  {
>>         struct inode *child_inode = d_inode(child);
>> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
>> index f0cf2714ec52..db783f6b28bc 100644
>> --- a/include/linux/exportfs.h
>> +++ b/include/linux/exportfs.h
>> @@ -110,6 +110,13 @@ enum fid_type {
>>          */
>>         FILEID_INO64_GEN_PARENT = 0x82,
>>
>> +       /*
>> +        * 64 bit nodeid number, 32 bit generation number,
>> +        * variable length handle.
>> +        */
>> +       FILEID_FUSE_WITHOUT_PARENT = 0x91,
>> +       FILEID_FUSE_WITH_PARENT = 0x92,
>> +
>
>
> Do we even need a handle with inode+gen+server specific handle?
> I didn't think we would. If we do, please explain why.

I *think* I've addressed this my other comments above.  Basically, I agree
that we don't need it.  FUSE_ROOT_ID would be the only exception, but
there's no need for this nodeid+gen+fh just because of that.  (Or maybe I
misunderstood this comment?)

(Oh, and thanks a lot everyone for the comments!  Much appreciated.)

Cheers,
-- 
Luís