mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: Jeff Layton <jlayton@kernel.org>
Cc: "Darrick J. Wong" <djwong@kernel.org>,
	 Matthew Wilcox <willy@infradead.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	 Jan Kara <jack@suse.cz>, Christoph Hellwig <hch@lst.de>,
	linux-fsdevel@vger.kernel.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH] vfs: remove externs from fs.h on functions modified by i_ino widening
Date: Mon, 9 Mar 2026 10:02:15 +0100	[thread overview]
Message-ID: <20260309-bartlos-ozonschicht-5416b30686bb@brauner> (raw)
In-Reply-To: <3d940ceb8026cff3ce8fc14ed7aa7be8ae677190.camel@kernel.org>

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...

  reply	other threads:[~2026-03-09  9:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-06 13:27 Jeff Layton
2026-03-06 14:54 ` Christoph Hellwig
2026-03-06 22:17 ` Mateusz Guzik
2026-03-07 18:05   ` Jeff Layton
2026-03-07  5:31 ` Matthew Wilcox
2026-03-07  6:10   ` Darrick J. Wong
2026-03-07 18:00     ` Jeff Layton
2026-03-09  9:02       ` Christian Brauner [this message]
2026-03-07 17:57   ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260309-bartlos-ozonschicht-5416b30686bb@brauner \
    --to=brauner@kernel.org \
    --cc=djwong@kernel.org \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®