Re: [RFC PATCH v1 1/7] fs: Add inode_get_ino() and implement get_ino() for NFS

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2024-10-11 at 17:30 +0200, Mickaël Salaün wrote:
> On Fri, Oct 11, 2024 at 07:39:33AM -0700, Christoph Hellwig wrote:
> > On Fri, Oct 11, 2024 at 03:52:42PM +0200, Mickaël Salaün wrote:
> > > > > Yes, but how do you call getattr() without a path?
> > > > 
> > > > You don't because inode numbers are irrelevant without the path.
> > > 
> > > They are for kernel messages and audit logs.  Please take a look at the
> > > use cases with the other patches.
> > 
> > It still is useless.  E.g. btrfs has duplicate inode numbers due to
> > subvolumes.
> 
> At least it reflects what users see.
> 
> > 
> > If you want a better pretty but not useful value just work on making
> > i_ino 64-bits wide, which is long overdue.
> 
> That would require too much work for me, and this would be a pain to
> backport to all stable kernels.
> 

Would it though? Adding this new inode operation seems sub-optimal.

Inode numbers are static information. Once an inode number is set on an
inode it basically never changes.  This patchset will turn all of those
direct inode->i_ino fetches into a pointer chase for the new inode
operation, which will then almost always just result in a direct fetch.

A better solution here would be to make inode->i_ino a u64, and just
fix up all of the places that touch it to expect that. Then, just
ensure that all of the filesystems set it properly when instantiating
new inodes. Even on 32-bit arch, you likely wouldn't need a seqcount
loop or anything to fetch this since the chance of a torn read there is
basically zero.

If there are places where we need to convert i_ino down to 32-bits,
then we can just use some scheme like nfs_fattr_to_ino_t(), or just
cast i_ino to a u32.

Yes, it'd be a larger patchset, but that seems like it would be a
better solution.
-- 
Jeff Layton <jlayton@kernel.org>