Re: [PATCH] vfs: remove externs from fs.h on functions modified by i_ino widening

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2026-03-06 at 22:10 -0800, Darrick J. Wong wrote:
> On Sat, Mar 07, 2026 at 05:31:24AM +0000, Matthew Wilcox wrote:
> > On Fri, Mar 06, 2026 at 08:27:01AM -0500, Jeff Layton wrote:
> > > @@ -2951,26 +2951,26 @@ struct inode *iget5_locked(struct super_block *, u64,
> > >  struct inode *iget5_locked_rcu(struct super_block *, u64,
> > >  			       int (*test)(struct inode *, void *),
> > >  			       int (*set)(struct inode *, void *), void *);
> > > -extern struct inode *iget_locked(struct super_block *, u64);
> > > +struct inode *iget_locked(struct super_block *, u64);
> > 
> > I think plain 'u64' deserves a name.  I know some people get very
> > upset when they see any unnamed parameter, but I don't think that you
> > need to put "sb" in the first parameter.  A u64 is non-obvious though;
> > is it i_ino?  Or hashval?
> > 
> > > -extern struct inode *find_inode_nowait(struct super_block *,
> > > +struct inode *find_inode_nowait(struct super_block *,
> > >  				       u64,
> > >  				       int (*match)(struct inode *,
> > >  						    u64, void *),
> > >  				       void *data);
> > 
> > I think these need to be reflowed.  Before they were aligned with the
> > open bracket, and this demonstrates why that's a stupid convention.
> > And the u64 needs a name.
> 
> I think inode numbers ought to be their own typedef to make it *really
> obvious* when you're dealing with one, and was pretty sad to see "vfs:
> remove kino_t typedef and PRIino format macro" so soon after one was
> added.  But our really excellent checkpatch tool says "do not add new
> typedefs" so instead everyone else has to be really smart about what
> "u64" represents when they see one, particularly because arithmetic is
> meaningless for this particular "u64".
> 
> Yay.
> 

My take on typedefs is that they are mostly useful when you have a
variable type that needs to be accessed in a particular way. For
instance, I added a typedef for errseq_t since it's not just a plain
integer (there are fields within it), and you need to use the correct
functions to work with it.

In this case though, it really is just a write-once read-many times
bog-standard u64. I sort of wonder if we ought to label it const since
inode numbers really shouldn't ever change after being established, but
I didn't go that far.

-- 
Jeff Layton <jlayton@kernel.org>