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

From: Christian Brauner

Date: Mon Mar 09 2026 - 05:04:04 EST


On Sat, Mar 07, 2026 at 01:00:15PM -0500, Jeff Layton wrote:
> 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.

I would be very interested in seeing any filesystem that changes inode
numbers...