Re: [PATCH] vfs: remove externs from fs.h on functions modified by i_ino widening
From: Jeff Layton
Date: Sat Mar 07 2026 - 13:00:31 EST
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@xxxxxxxxxx>