| Message ID | 1683800241-14488-1-git-send-email-renlei1@chinatelecom.cn (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | xfs: xfs_nfs_get_inode support zero generation | expand |
On Thu, May 11, 2023 at 06:17:21PM +0800, renlei1@chinatelecom.cn wrote: > From: Ren Lei <renlei1@chinatelecom.cn> > > If generation is zero, bypass the verification of generation number > to avoid stale file error. (Be consistent with other fs, such as > ext4, fat, jfs, etc.) What code is affected by the gen==0 handles being rejected? Is there a user program or test case where this is required? --D > Signed-off-by: Ren Lei<renlei1@chinatelecom.cn> > --- > fs/xfs/xfs_export.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c > index 1064c2342876..cbee32c5ad37 100644 > --- a/fs/xfs/xfs_export.c > +++ b/fs/xfs/xfs_export.c > @@ -146,7 +146,7 @@ xfs_nfs_get_inode( > return ERR_PTR(error); > } > > - if (VFS_I(ip)->i_generation != generation) { > + if (generation && VFS_I(ip)->i_generation != generation) { > xfs_irele(ip); > return ERR_PTR(-ESTALE); > } > -- > 2.27.0 >
On Thu, May 11, 2023 at 04:22:06PM -0700, Darrick J. Wong wrote: > On Thu, May 11, 2023 at 06:17:21PM +0800, renlei1@chinatelecom.cn wrote: > > From: Ren Lei <renlei1@chinatelecom.cn> > > > > If generation is zero, bypass the verification of generation number > > to avoid stale file error. (Be consistent with other fs, such as > > ext4, fat, jfs, etc.) > > What code is affected by the gen==0 handles being rejected? Is there a > user program or test case where this is required? A generation number of 0 is perfectly valid in XFS. We've been creating them in XFS filesystems since 1993 and never had a problem with filehandle verification. Indeed, every root inode in every XFS filesystem ever made will have a generation number of 0. Yup, a random XFS filesystem recently made from a current xfsprogs: # xfs_db /dev/vdc xfs_db> sb 0 xfs_db> a rootino xfs_db> p core.gen core.gen = 0 xfs_db> -Dave.
On Fri, May 12, 2023 at 10:53:29AM +0800, renlei1@chinatelecom.cn wrote: > Yup, gen is 0 for the inodes created by libxfs, such as rootino, rbmino, rsumino. > but those inodes will never be freed, and gen will always zero. > so I think bypass the verification if gen==0 is still valid. I disagree. Handles have long encoded inode and generation to prevent users from unintentionally modifying a file when the inode number is recycled as a result of an unlink/create cycle. --D > Regards, > Lei > > From: Dave Chinner > Date: 2023-05-12 10:00 > To: Darrick J. Wong > CC: renlei1; linux-xfs; linux-kernel > Subject: Re: [PATCH] xfs: xfs_nfs_get_inode support zero generation > On Thu, May 11, 2023 at 04:22:06PM -0700, Darrick J. Wong wrote: > > On Thu, May 11, 2023 at 06:17:21PM +0800, renlei1@chinatelecom.cn wrote: > > > From: Ren Lei <renlei1@chinatelecom.cn> > > > > > > If generation is zero, bypass the verification of generation number > > > to avoid stale file error. (Be consistent with other fs, such as > > > ext4, fat, jfs, etc.) > > > > What code is affected by the gen==0 handles being rejected? Is there a > > user program or test case where this is required? > > A generation number of 0 is perfectly valid in XFS. We've been > creating them in XFS filesystems since 1993 and never had a problem > with filehandle verification. Indeed, every root inode in every XFS > filesystem ever made will have a generation number of 0. > > Yup, a random XFS filesystem recently made from a current xfsprogs: > > # xfs_db /dev/vdc > xfs_db> sb 0 > xfs_db> a rootino > xfs_db> p core.gen > core.gen = 0 > xfs_db> > > -Dave. > -- > Dave Chinner > david@fromorbit.com >
On Fri, May 12, 2023 at 10:53:29AM +0800, renlei1@chinatelecom.cn wrote: > Yup, gen is 0 for the inodes created by libxfs, such as rootino, rbmino, rsumino. > but those inodes will never be freed, and gen will always zero. Yes, but why does that even matter for file handle verification? > so I think bypass the verification if gen==0 is still valid. Did you think about what happens when an inode->gen overflows from 0xffffffff to 0 as part of a normal unlink/create cycle? -Dave.
diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c index 1064c2342876..cbee32c5ad37 100644 --- a/fs/xfs/xfs_export.c +++ b/fs/xfs/xfs_export.c @@ -146,7 +146,7 @@ xfs_nfs_get_inode( return ERR_PTR(error); } - if (VFS_I(ip)->i_generation != generation) { + if (generation && VFS_I(ip)->i_generation != generation) { xfs_irele(ip); return ERR_PTR(-ESTALE); }