mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Jan Kara <jack@suse.cz>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	 linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] vfs: remove externs from fs.h on functions modified by i_ino widening
Date: Mon, 09 Mar 2026 09:27:47 -0400	[thread overview]
Message-ID: <214341c4753f7ce61d9b01155e9c493e880b7bbd.camel@kernel.org> (raw)
In-Reply-To: <wlwvnfrhpw4yyzdnxte73nv6rs5lh2jilvnfd2mtocyct4jyel@4l4km3lehq2c>

On Mon, 2026-03-09 at 13:27 +0100, Jan Kara wrote:
> On Mon 09-03-26 07:53:51, Jeff Layton wrote:
> > On Mon, 2026-03-09 at 11:02 +0100, Jan Kara wrote:
> > > On Sat 07-03-26 14:54:31, Jeff Layton wrote:
> > > > Christoph says, in response to one of the patches in the i_ino widening
> > > > series, which changes the prototype of several functions in fs.h:
> > > > 
> > > >     "Can you please drop all these pointless externs while you're at it?"
> > > > 
> > > > Remove extern keyword from functions touched by that patch (and a few
> > > > that happened to be nearby). Also add missing argument names to
> > > > declarations that lacked them.
> > > > 
> > > > Suggested-by: Christoph Hellwig <hch@lst.de>
> > > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ...
> > > > -extern void inode_init_once(struct inode *);
> > > > -extern void address_space_init_once(struct address_space *mapping);
> > > > -extern struct inode * igrab(struct inode *);
> > > > -extern ino_t iunique(struct super_block *, ino_t);
> > > > -extern int inode_needs_sync(struct inode *inode);
> > > > -extern int inode_just_drop(struct inode *inode);
> > > > +void inode_init_once(struct inode *inode);
> > > > +void address_space_init_once(struct address_space *mapping);
> > > > +struct inode *igrab(struct inode *inode);
> > > > +ino_t iunique(struct super_block *sb, ino_t max_reserved);
> > > 
> > > I've just noticed that we probably forgot to convert iunique() to use u64
> > > for inode numbers... Although the iunique() number allocator might prefer
> > > to stay within 32 bits, the interfaces should IMO still use u64 for
> > > consistency.
> > > 
> > 
> > I went back and forth on that one, but I left iunique() changes off
> > since they weren't strictly required. Most filesystems that use it
> > won't have more than 2^32 inodes anyway.
> > 
> > If they worked before with iunique() limited to 32-bit values, they
> > should still after this. After the i_ino widening we could certainly
> > change it to return a u64 though.
> 
> Yes, it won't change anything wrt functionality. I just think that if we go
> for "ino_t is the userspace API type and kernel-internal inode numbers
> (i.e.  what gets stored in inode->i_ino) are passed as u64", then this
> place should logically have u64...
> 

I think we'll need a real plan for this.

First, here was Claude's analysis of this piece. (FWIW, we didn't
change ino_t type since that could have had userland implications):

-----------------------8<------------------------
## Phase 5: Related Type Updates

### 5.1 `get_next_ino()` (`fs/inode.c:1145`)

Returns `unsigned int` (32-bit). Used by pseudo-filesystems (tmpfs, sysfs,
debugfs, etc.). This is deliberately limited to 32 bits to avoid EOVERFLOW from
`stat()` on 32-bit userspace. **No change needed** — widening to `u64` storage
is fine; the values still fit.

### 5.2 `iunique()` (`fs/inode.c:1556`)

Uses `static unsigned int counter`. Returns `ino_t`. Same consideration as
`get_next_ino()` — keeps values in 32-bit range for compat. **No change needed**
for the counter, but return type should follow `ino_t` (which may or may not
change per Phase 1.2).

### 5.3 `is_zero_ino()` (`include/linux/fs.h:2986`)

Already casts to `(u32)` explicitly. **No change needed.**
-----------------------8<------------------------

It certainly wouldn't hurt to make these functions return a u64 type,
but I worry a little about letting them return values bigger than
UINT_MAX:

One of my very first patches was 866b04fccbf1 ("inode numbering: make
static counters in new_inode and iunique be 32 bits"), and it made
get_next_ino() and iunique() always return 32 bit values. 

At the time, 32-bit machines and legacy binaries were a lot more
prevalent than they are today and this was real problem. I'm guessing
it's not so much today, so we could probably make them return real 64-
bit values. That might also obviate the need for locking in these
functions too.

Still, I think we'll want to tread carefully here. Maybe we could add
64-bit versions of these functions that are lockless, and add a kernel
command-line switch that makes them fall back to the older functions in
case there are issues?
-- 
Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2026-03-09 13:27 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-07 19:54 Jeff Layton
2026-03-09 10:02 ` Jan Kara
2026-03-09 11:53   ` Jeff Layton
2026-03-09 12:27     ` Jan Kara
2026-03-09 13:27       ` Jeff Layton [this message]
2026-03-09 14:46         ` Jan Kara
2026-03-09 14:56           ` Jeff Layton
2026-03-09 15:21             ` Jan Kara
2026-03-09 10:28 ` Christian Brauner

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=214341c4753f7ce61d9b01155e9c493e880b7bbd.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=brauner@kernel.org \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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®