Re: [PATCH 10/10] xfs: add support for non-blocking fh_to_dentry()

[Amir Goldstein <amir73il@gmail.com>]

On Wed, Sep 10, 2025 at 11:48 PM Thomas Bertschinger
<tahbertschinger@gmail.com> wrote:
>
> This is to support using open_by_handle_at(2) via io_uring. It is useful
> for io_uring to request that opening a file via handle be completed
> using only cached data, or fail with -EAGAIN if that is not possible.
>
> The signature of xfs_nfs_get_inode() is extended with a new flags
> argument that allows callers to specify XFS_IGET_INCORE.
>
> That flag is set when the VFS passes the FILEID_CACHED flag via the
> fileid_type argument.
>
> Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>

I'll let xfs developers review this, but its looks pretty straightforward,
so on my part you may add:

Acked-by: Amir Goldstein <amir73il@gmail.com>

One small nit below

> ---
>  fs/xfs/xfs_export.c | 32 ++++++++++++++++++++++++++------
>  fs/xfs/xfs_export.h |  3 ++-
>  fs/xfs/xfs_handle.c |  2 +-
>  3 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
> index 201489d3de08..ca2a9ed0eb16 100644
> --- a/fs/xfs/xfs_export.c
> +++ b/fs/xfs/xfs_export.c
> @@ -106,7 +106,8 @@ struct inode *
>  xfs_nfs_get_inode(
>         struct super_block      *sb,
>         u64                     ino,
> -       u32                     generation)
> +       u32                     generation,
> +       uint                    flags)
>  {
>         xfs_mount_t             *mp = XFS_M(sb);
>         xfs_inode_t             *ip;
> @@ -123,7 +124,9 @@ xfs_nfs_get_inode(
>          * fine and not an indication of a corrupted filesystem as clients can
>          * send invalid file handles and we have to handle it gracefully..
>          */
> -       error = xfs_iget(mp, NULL, ino, XFS_IGET_UNTRUSTED, 0, &ip);
> +       flags |= XFS_IGET_UNTRUSTED;
> +
> +       error = xfs_iget(mp, NULL, ino, flags, 0, &ip);
>         if (error) {
>
>                 /*
> @@ -140,6 +143,10 @@ xfs_nfs_get_inode(
>                 case -EFSCORRUPTED:
>                         error = -ESTALE;
>                         break;
> +               case -ENODATA:
> +                       if (flags & XFS_IGET_INCORE)
> +                               error = -EAGAIN;
> +                       break;
>                 default:
>                         break;
>                 }
> @@ -174,6 +181,12 @@ xfs_fs_fh_to_dentry(struct super_block *sb, struct fid *fid,
>  {
>         struct xfs_fid64        *fid64 = (struct xfs_fid64 *)fid;
>         struct inode            *inode = NULL;
> +       uint                    flags = 0;
> +
> +       if (fileid_type & FILEID_CACHED)
> +               flags = XFS_IGET_INCORE;
> +
> +       fileid_type = FILEID_TYPE(fileid_type);

That is a smelly practice.
It's better to rename the function argument to fileid_flags or fileid_type_flags
and use a local fileid_type var for fileid_type = FILEID_TYPE(fileid_flags)

Thanks,
Amir.

>
>         if (fh_len < xfs_fileid_length(fileid_type))
>                 return NULL;
> @@ -181,11 +194,11 @@ xfs_fs_fh_to_dentry(struct super_block *sb, struct fid *fid,
>         switch (fileid_type) {
>         case FILEID_INO32_GEN_PARENT:
>         case FILEID_INO32_GEN:
> -               inode = xfs_nfs_get_inode(sb, fid->i32.ino, fid->i32.gen);
> +               inode = xfs_nfs_get_inode(sb, fid->i32.ino, fid->i32.gen, flags);
>                 break;
>         case FILEID_INO32_GEN_PARENT | XFS_FILEID_TYPE_64FLAG:
>         case FILEID_INO32_GEN | XFS_FILEID_TYPE_64FLAG:
> -               inode = xfs_nfs_get_inode(sb, fid64->ino, fid64->gen);
> +               inode = xfs_nfs_get_inode(sb, fid64->ino, fid64->gen, flags);
>                 break;
>         }
>
> @@ -198,6 +211,12 @@ xfs_fs_fh_to_parent(struct super_block *sb, struct fid *fid,
>  {
>         struct xfs_fid64        *fid64 = (struct xfs_fid64 *)fid;
>         struct inode            *inode = NULL;
> +       uint                    flags = 0;
> +
> +       if (fileid_type & FILEID_CACHED)
> +               flags = XFS_IGET_INCORE;
> +
> +       fileid_type = FILEID_TYPE(fileid_type);
>
>         if (fh_len < xfs_fileid_length(fileid_type))
>                 return NULL;
> @@ -205,11 +224,11 @@ xfs_fs_fh_to_parent(struct super_block *sb, struct fid *fid,
>         switch (fileid_type) {
>         case FILEID_INO32_GEN_PARENT:
>                 inode = xfs_nfs_get_inode(sb, fid->i32.parent_ino,
> -                                             fid->i32.parent_gen);
> +                                             fid->i32.parent_gen, flags);
>                 break;
>         case FILEID_INO32_GEN_PARENT | XFS_FILEID_TYPE_64FLAG:
>                 inode = xfs_nfs_get_inode(sb, fid64->parent_ino,
> -                                             fid64->parent_gen);
> +                                             fid64->parent_gen, flags);
>                 break;
>         }
>
> @@ -248,4 +267,5 @@ const struct export_operations xfs_export_operations = {
>         .map_blocks             = xfs_fs_map_blocks,
>         .commit_blocks          = xfs_fs_commit_blocks,
>  #endif
> +       .flags                  = EXPORT_OP_NONBLOCK,
>  };
> diff --git a/fs/xfs/xfs_export.h b/fs/xfs/xfs_export.h
> index 3cd85e8901a5..9addfcd5b1e1 100644
> --- a/fs/xfs/xfs_export.h
> +++ b/fs/xfs/xfs_export.h
> @@ -57,6 +57,7 @@ struct xfs_fid64 {
>  /* This flag goes on the wire.  Don't play with it. */
>  #define XFS_FILEID_TYPE_64FLAG 0x80    /* NFS fileid has 64bit inodes */
>
> -struct inode *xfs_nfs_get_inode(struct super_block *sb, u64 ino, u32 gen);
> +struct inode *xfs_nfs_get_inode(struct super_block *sb, u64 ino, u32 gen,
> +                               uint flags);
>
>  #endif /* __XFS_EXPORT_H__ */
> diff --git a/fs/xfs/xfs_handle.c b/fs/xfs/xfs_handle.c
> index f19fce557354..7d877ff504d6 100644
> --- a/fs/xfs/xfs_handle.c
> +++ b/fs/xfs/xfs_handle.c
> @@ -193,7 +193,7 @@ xfs_khandle_to_inode(
>                 return ERR_PTR(-EINVAL);
>
>         inode = xfs_nfs_get_inode(mp->m_super, handle->ha_fid.fid_ino,
> -                       handle->ha_fid.fid_gen);
> +                       handle->ha_fid.fid_gen, 0);
>         if (IS_ERR(inode))
>                 return ERR_CAST(inode);
>
> --
> 2.51.0
>
>