Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, Mar 03, 2026 at 10:14:27AM -0500, Jeff Layton wrote: > On Tue, 2026-03-03 at 06:30 -0800, Christoph Hellwig wrote: > > On Tue, Mar 03, 2026 at 09:19:42AM -0500, Jeff Layton wrote: > > > On Tue, 2026-03-03 at 05:59 -0800, Christoph Hellwig wrote: > > > > On Tue, Mar 03, 2026 at 08:43:15AM -0500, Jeff Layton wrote: > > > > > On Tue, 2026-03-03 at 05:37 -0800, Christoph Hellwig wrote: > > > > > > On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote: > > > > > > > Like I said to Ted, this is just temporary scaffolding for the > > > > > > > change. > > > > > > > The PRIino macro is removed in the end. Given that, perhaps you > > > > > > > can > > > > > > > overlook the bikeshed's color in this instance? > > > > > > > > > > > > So why add it in the first place? > > > > > > > > > > Bisectability. The first version I did of this would have broken the > > > > > ability to bisect properly across these changes. I don't love the > > > > > "churn" here either, but this should be cleanly bisectable. > > > > > > > > What do you need to bisect in format string changes? Splitting > > > > every variable type change outside of the main i_ino out - sure. > > > > But bisecting that "change to u64 in ext4" really broke ext4 and > > > > not "change to u64" is not very useful. Commits should do one > > > > well defined thing. Adding a weird transition layer for a format > > > > thing that just gets dropped is not one well defined thing. > > > > > > In the middle stages of the series, you will get warnings or errors on > > > 32-bit hosts when i_ino's type doesn't match what the format string > > > expects. > > > > > > There are really only three options here: > > > > > > 1/ Do (almost) all of the changes in one giant patch > > > > > > 2/ Accept that the build may break during the interim stages > > > > > > 3/ This series: using a typedef and macro to work around the breakage > > > until the type can be changed, at the expense of some extra churn in > > > the codebase > > > > > > 3 seems like the lesser evil. > > > > No, 1 is by far the least evil. Note that it's not really almost all, > > as all the local variables can easily and sanely be split out. It's > > all of the format strings, and that makes sense. The only "regressions" > > there are incorrect format strings which have good warnings and can > > be fixed easily. > > Well, I've done 2 and 3 already. Why not 1? :) > > It's not so much the regressions that are a problem here, but the merge > conflicts for anyone wanting to backport later patches that are near > these format changes. Having that change broken up by subsystem makes > it easier to handle that piecemeal later. > > I think we'll be looking at close to a 1000 line patch that touches > nearly 200 files if go that route. Roughly: > > 182 files changed, 910 insertions(+), 912 deletions(-) > > There are some tracepoint changes in some of the per-subsystem patches > that will need to be split out, so the count isn't exact, but it'll be > fairly close. > > Since Christian will probably end up taking this series, I'd like to > get his opinion before I respin anything. I'm kinda surprised that we suddenly started caring about the amount of individual patches. I personally don't care either way. Do it in one giant patch if this moves us forward. I've done 1 and 3 and what you did. And I'd be really annoyed if during a bisect I start to get pointless build failures because someone did 2.
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, Mar 03, 2026 at 09:19:42AM -0500, Jeff Layton wrote:
> There are really only three options here:
>
> 1/ Do (almost) all of the changes in one giant patch
>
> 2/ Accept that the build may break during the interim stages
>
> 3/ This series: using a typedef and macro to work around the breakage
> until the type can be changed, at the expense of some extra churn in
> the codebase
4/ Don't do anything, drop the patch series (I'm not in favour of this,
but it is an option)
5/ Do the conversion(s) _once_ per filesystem. Here's one way to do
it:
- unsigned long i_ino;
+ union {
+ u64 i_ino64;
+ struct {
+#if defined(CONFIG_64BIT)
+ unsigned long i_ino;
+#elif defined(CONFIG_CPU_BIG_ENDIAN)
+ unsigned long i_ino;
+ unsigned long i_ino_pad;
+#else
+ unsigned long i_ino_pad;
+ unsigned long i_ino;
+#endif
+ };
+ };
[...]
#define i_ino(inode)(inode)->i_ino64
So that's patch one. All plain references to i_ino access the lower
bits of i_ino64, so everything will continue to work as it does today.
Once you've got the VFS core in shape, you can convert filesystems one
at a time to use i_ino(inode). Once you're done you can delete the
scaffolding from the core and go back to calling i_ino64 just i_ino.
You could delete the i_ino() uses from filesystems at that point, but
why bother?
I'm sure there are other ways to do it, this is just the one I came up
with. But for the love of god stop spamming hundreds of people on the
cc of this patchset. In fact, take me off for next time -- I get each
one of these fucking patches four times.
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, Mar 03, 2026 at 08:43:15AM -0500, Jeff Layton wrote: > On Tue, 2026-03-03 at 05:37 -0800, Christoph Hellwig wrote: > > On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote: > > > Like I said to Ted, this is just temporary scaffolding for the change. > > > The PRIino macro is removed in the end. Given that, perhaps you can > > > overlook the bikeshed's color in this instance? > > > > So why add it in the first place? > > Bisectability. The first version I did of this would have broken the > ability to bisect properly across these changes. I don't love the > "churn" here either, but this should be cleanly bisectable. What do you need to bisect in format string changes? Splitting every variable type change outside of the main i_ino out - sure. But bisecting that "change to u64 in ext4" really broke ext4 and not "change to u64" is not very useful. Commits should do one well defined thing. Adding a weird transition layer for a format thing that just gets dropped is not one well defined thing.
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, 2026-03-03 at 10:16 -0500, Theodore Tso wrote: > On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote: > > > > Like I said to Ted, this is just temporary scaffolding for the change. > > The PRIino macro is removed in the end. Given that, perhaps you can > > overlook the bikeshed's color in this instance? > > I didn't realize that this was going to disappear in the end. That > makes me feel much better about the change. I'd suggest changing the > commit description where it claims that we're using something that > follows the inttypes.h convention and making it clear that this is > temporary and only to preserve bisectability. > > One question though --- are there *really* places that are using > signed inode numbers and trying to print them? If people are trying > to use negative inodes to signal an error or some such, the it implies > that at least for some file systems, an inode number larger than 2**63 > might be problematic. If there is core VFS code that uses a negative > inode number then this could be a real potential trap. > > So are there really code which is doing a printf of 'PRIino "d"'? Or > was this to allow the use of of 'PRiino "x"'? > Mostly it's to allow 'PRIino "x"'. There are a number of places that (for whatever reason) print the inode number in hex. I don't want to change those. There are also some places that print it as a signed value (PRIino "d"). I suspect most of those are bugs, or just holdovers from a simpler time when we didn't worry so much about the signedness of inode numbers. I fixed a few of those in the context of this series, fwiw. -- Jeff Layton
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, 2026-03-03 at 05:37 -0800, Christoph Hellwig wrote: > On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote: > > Like I said to Ted, this is just temporary scaffolding for the change. > > The PRIino macro is removed in the end. Given that, perhaps you can > > overlook the bikeshed's color in this instance? > > So why add it in the first place? Bisectability. The first version I did of this would have broken the ability to bisect properly across these changes. I don't love the "churn" here either, but this should be cleanly bisectable. -- Jeff Layton
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, Mar 03, 2026 at 09:19:42AM -0500, Jeff Layton wrote: > On Tue, 2026-03-03 at 05:59 -0800, Christoph Hellwig wrote: > > On Tue, Mar 03, 2026 at 08:43:15AM -0500, Jeff Layton wrote: > > > On Tue, 2026-03-03 at 05:37 -0800, Christoph Hellwig wrote: > > > > On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote: > > > > > Like I said to Ted, this is just temporary scaffolding for the change. > > > > > The PRIino macro is removed in the end. Given that, perhaps you can > > > > > overlook the bikeshed's color in this instance? > > > > > > > > So why add it in the first place? > > > > > > Bisectability. The first version I did of this would have broken the > > > ability to bisect properly across these changes. I don't love the > > > "churn" here either, but this should be cleanly bisectable. > > > > What do you need to bisect in format string changes? Splitting > > every variable type change outside of the main i_ino out - sure. > > But bisecting that "change to u64 in ext4" really broke ext4 and > > not "change to u64" is not very useful. Commits should do one > > well defined thing. Adding a weird transition layer for a format > > thing that just gets dropped is not one well defined thing. > > In the middle stages of the series, you will get warnings or errors on > 32-bit hosts when i_ino's type doesn't match what the format string > expects. > > There are really only three options here: > > 1/ Do (almost) all of the changes in one giant patch > > 2/ Accept that the build may break during the interim stages > > 3/ This series: using a typedef and macro to work around the breakage > until the type can be changed, at the expense of some extra churn in > the codebase > > 3 seems like the lesser evil. No, 1 is by far the least evil. Note that it's not really almost all, as all the local variables can easily and sanely be split out. It's all of the format strings, and that makes sense. The only "regressions" there are incorrect format strings which have good warnings and can be fixed easily.
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, Mar 03, 2026 at 10:14:27AM -0500, Jeff Layton wrote: > I think we'll be looking at close to a 1000 line patch that touches > nearly 200 files if go that route. Roughly: > > 182 files changed, 910 insertions(+), 912 deletions(-) That's not actually a lot, especially for a patch that should be scriped and mechnanical. I also really don't understand the backport argument. It's not like you could backport any of the split out patches individually anyway.
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, 2026-03-03 at 05:59 -0800, Christoph Hellwig wrote: > On Tue, Mar 03, 2026 at 08:43:15AM -0500, Jeff Layton wrote: > > On Tue, 2026-03-03 at 05:37 -0800, Christoph Hellwig wrote: > > > On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote: > > > > Like I said to Ted, this is just temporary scaffolding for the change. > > > > The PRIino macro is removed in the end. Given that, perhaps you can > > > > overlook the bikeshed's color in this instance? > > > > > > So why add it in the first place? > > > > Bisectability. The first version I did of this would have broken the > > ability to bisect properly across these changes. I don't love the > > "churn" here either, but this should be cleanly bisectable. > > What do you need to bisect in format string changes? Splitting > every variable type change outside of the main i_ino out - sure. > But bisecting that "change to u64 in ext4" really broke ext4 and > not "change to u64" is not very useful. Commits should do one > well defined thing. Adding a weird transition layer for a format > thing that just gets dropped is not one well defined thing. In the middle stages of the series, you will get warnings or errors on 32-bit hosts when i_ino's type doesn't match what the format string expects. There are really only three options here: 1/ Do (almost) all of the changes in one giant patch 2/ Accept that the build may break during the interim stages 3/ This series: using a typedef and macro to work around the breakage until the type can be changed, at the expense of some extra churn in the codebase 3 seems like the lesser evil. -- Jeff Layton
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, 2026-03-03 at 06:30 -0800, Christoph Hellwig wrote: > On Tue, Mar 03, 2026 at 09:19:42AM -0500, Jeff Layton wrote: > > On Tue, 2026-03-03 at 05:59 -0800, Christoph Hellwig wrote: > > > On Tue, Mar 03, 2026 at 08:43:15AM -0500, Jeff Layton wrote: > > > > On Tue, 2026-03-03 at 05:37 -0800, Christoph Hellwig wrote: > > > > > On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote: > > > > > > Like I said to Ted, this is just temporary scaffolding for the > > > > > > change. > > > > > > The PRIino macro is removed in the end. Given that, perhaps you can > > > > > > overlook the bikeshed's color in this instance? > > > > > > > > > > So why add it in the first place? > > > > > > > > Bisectability. The first version I did of this would have broken the > > > > ability to bisect properly across these changes. I don't love the > > > > "churn" here either, but this should be cleanly bisectable. > > > > > > What do you need to bisect in format string changes? Splitting > > > every variable type change outside of the main i_ino out - sure. > > > But bisecting that "change to u64 in ext4" really broke ext4 and > > > not "change to u64" is not very useful. Commits should do one > > > well defined thing. Adding a weird transition layer for a format > > > thing that just gets dropped is not one well defined thing. > > > > In the middle stages of the series, you will get warnings or errors on > > 32-bit hosts when i_ino's type doesn't match what the format string > > expects. > > > > There are really only three options here: > > > > 1/ Do (almost) all of the changes in one giant patch > > > > 2/ Accept that the build may break during the interim stages > > > > 3/ This series: using a typedef and macro to work around the breakage > > until the type can be changed, at the expense of some extra churn in > > the codebase > > > > 3 seems like the lesser evil. > > No, 1 is by far the least evil. Note that it's not really almost all, > as all the local variables can easily and sanely be split out. It's > all of the format strings, and that makes sense. The only "regressions" > there are incorrect format strings which have good warnings and can > be fixed easily. Well, I've done 2 and 3 already. Why not 1? :) It's not so much the regressions that are a problem here, but the merge conflicts for anyone wanting to backport later patches that are near these format changes. Having that change broken up by subsystem makes it easier to handle that piecemeal later. I think we'll be looking at close to a 1000 line patch that touches nearly 200 files if go that route. Roughly: 182 files changed, 910 insertions(+), 912 deletions(-) There are some tracepoint changes in some of the per-subsystem patches that will need to be split out, so the count isn't exact, but it'll be fairly close. Since Christian will probably end up taking this series, I'd like to get his opinion before I respin anything. -- Jeff Layton
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote: > > Like I said to Ted, this is just temporary scaffolding for the change. > The PRIino macro is removed in the end. Given that, perhaps you can > overlook the bikeshed's color in this instance? I didn't realize that this was going to disappear in the end. That makes me feel much better about the change. I'd suggest changing the commit description where it claims that we're using something that follows the inttypes.h convention and making it clear that this is temporary and only to preserve bisectability. One question though --- are there *really* places that are using signed inode numbers and trying to print them? If people are trying to use negative inodes to signal an error or some such, the it implies that at least for some file systems, an inode number larger than 2**63 might be problematic. If there is core VFS code that uses a negative inode number then this could be a real potential trap. So are there really code which is doing a printf of 'PRIino "d"'? Or was this to allow the use of of 'PRiino "x"'? - Ted
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Mon, Mar 02, 2026 at 08:25:46PM -0800, Darrick J. Wong wrote: > > That being said, the userspace PRIu64, et. al macros are complete > > format specifiers, not just a length modifier. And I think this > > results in less ugly format specifiers in our kernel code. > > Yeah, I don't like "ino=%" PRIino "u, lolz\n" either. I'd rather have > the whole format in the PRIino definition -- it /is/ unsigned long > after all. Just drop the bloody macro and the pointless micro-splitting of the change. After this the inode is always 64-bit and we can just use normal ll specifiers without messing things up.
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote: > Like I said to Ted, this is just temporary scaffolding for the change. > The PRIino macro is removed in the end. Given that, perhaps you can > overlook the bikeshed's color in this instance? So why add it in the first place?
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Mon, 2026-03-02 at 20:25 -0800, Darrick J. Wong wrote: > On Mon, Mar 02, 2026 at 08:25:56PM -0500, Theodore Tso wrote: > > On Mon, Mar 02, 2026 at 03:23:45PM -0500, Jeff Layton wrote: > > > The PRIino macro is a length modifier, not a complete format specifier. > > > It is used as: "%" PRIino "u" for decimal, "%" PRIino "x" for hex, etc. > > > This follows the pattern used by userspace PRIu64/PRIx64 macros. > > > > For the record, I really hate the inttypes.h format specifiers, but I > > agree that we should forward the example of the C99 spec, for better > > or for worse. > > > > That being said, the userspace PRIu64, et. al macros are complete > > format specifiers, not just a length modifier. And I think this > > results in less ugly format specifiers in our kernel code. > > Yeah, I don't like "ino=%" PRIino "u, lolz\n" either. I'd rather have > the whole format in the PRIino definition -- it /is/ unsigned long > after all. > Like I said to Ted, this is just temporary scaffolding for the change. The PRIino macro is removed in the end. Given that, perhaps you can overlook the bikeshed's color in this instance? -- Jeff Layton
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Mon, 2026-03-02 at 20:25 -0500, Theodore Tso wrote:
> On Mon, Mar 02, 2026 at 03:23:45PM -0500, Jeff Layton wrote:
> > The PRIino macro is a length modifier, not a complete format specifier.
> > It is used as: "%" PRIino "u" for decimal, "%" PRIino "x" for hex, etc.
> > This follows the pattern used by userspace PRIu64/PRIx64 macros.
>
> For the record, I really hate the inttypes.h format specifiers, but I
> agree that we should forward the example of the C99 spec, for better
> or for worse.
>
> That being said, the userspace PRIu64, et. al macros are complete
> format specifiers, not just a length modifier. And I think this
> results in less ugly format specifiers in our kernel code.
>
> cut here ---
> #!/bin/sh
> cat < /tmp/blah.c
> #include
> #include
>
> int main(int arg, char **argv)
> {
> printf("PRIu64 is %s\n", PRIu64);
> printf("PRId64 is %s\n", PRId64);
> printf("PRIx64 is %s\n", PRIx64);
> return 0;
> }
> EOF
>
> clang -m32 -o /tmp/blah /tmp/blah.c
> /tmp/blah
> cut here ---
>
> % /tmp/blah.sh
> PRIu64 is llu
> PRId64 is lld
> PRIx64 is llx
>
I originally had them that way, but ended up going with just a single
"PRIino" macro since it was simpler.
If we were going to keep these macros in perpetuity then I'd be
inclined to follow your suggestion, but since they're just temporary
scaffolding to make this change, maybe I can ask you to overlook this
preference for now?
--
Jeff Layton
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Mon 02-03-26 15:23:45, Jeff Layton wrote:
> Introduce a kino_t typedef and PRIino format macro to enable a
> bisect-clean transition of i_ino from unsigned long to u64.
>
> kino_t is initially defined as unsigned long (matching the original
> i_ino type), and PRIino is "l" (the format length modifier for
> unsigned long). A later patch will change these to u64 and "ll"
> respectively once all format strings have been updated to use PRIino.
>
> The PRIino macro is a length modifier, not a complete format specifier.
> It is used as: "%" PRIino "u" for decimal, "%" PRIino "x" for hex, etc.
> This follows the pattern used by userspace PRIu64/PRIx64 macros.
>
> Format strings using i_ino should be updated to use PRIino instead of
> a hard-coded length modifier to ensure warning-free compilation on
> both 32-bit and 64-bit architectures throughout the transition.
>
> Signed-off-by: Jeff Layton
Yeah, as a temporary solution for bisectability this looks fine to me. Feel
free to add:
Reviewed-by: Jan Kara
Honza
> ---
> include/linux/fs.h | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index
> 8b3dd145b25ec12b00ac1df17a952d9116b88047..e38bc5ece1f360d679a8f30b8171292f7a65c218
> 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -758,6 +758,9 @@ struct inode_state_flags {
> enum inode_state_flags_enum __state;
> };
>
> +typedef unsigned longkino_t;
> +#define PRIino "l"
> +
> /*
> * Keep mostly read-only and often accessed (especially for
> * the RCU path lookup and 'stat' data) fields at the beginning
> @@ -783,7 +786,7 @@ struct inode {
> #endif
>
> /* Stat data, not accessed from path walking */
> - unsigned long i_ino;
> + kino_t i_ino;
> /*
>* Filesystems may only read i_nlink directly. They shall use the
>* following functions for modification:
>
> --
> 2.53.0
>
--
Jan Kara
SUSE Labs, CR
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Mon, Mar 02, 2026 at 08:25:56PM -0500, Theodore Tso wrote:
> On Mon, Mar 02, 2026 at 03:23:45PM -0500, Jeff Layton wrote:
> > The PRIino macro is a length modifier, not a complete format specifier.
> > It is used as: "%" PRIino "u" for decimal, "%" PRIino "x" for hex, etc.
> > This follows the pattern used by userspace PRIu64/PRIx64 macros.
>
> For the record, I really hate the inttypes.h format specifiers, but I
> agree that we should forward the example of the C99 spec, for better
> or for worse.
>
> That being said, the userspace PRIu64, et. al macros are complete
> format specifiers, not just a length modifier. And I think this
> results in less ugly format specifiers in our kernel code.
Yeah, I don't like "ino=%" PRIino "u, lolz\n" either. I'd rather have
the whole format in the PRIino definition -- it /is/ unsigned long
after all.
--D
> cut here ---
> #!/bin/sh
> cat < /tmp/blah.c
> #include
> #include
>
> int main(int arg, char **argv)
> {
> printf("PRIu64 is %s\n", PRIu64);
> printf("PRId64 is %s\n", PRId64);
> printf("PRIx64 is %s\n", PRIx64);
> return 0;
> }
> EOF
>
> clang -m32 -o /tmp/blah /tmp/blah.c
> /tmp/blah
> cut here ---
>
> % /tmp/blah.sh
> PRIu64 is llu
> PRId64 is lld
> PRIx64 is llx
>
> Thanks!
>
> - Ted
Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
On Mon, Mar 02, 2026 at 03:23:45PM -0500, Jeff Layton wrote:
> The PRIino macro is a length modifier, not a complete format specifier.
> It is used as: "%" PRIino "u" for decimal, "%" PRIino "x" for hex, etc.
> This follows the pattern used by userspace PRIu64/PRIx64 macros.
For the record, I really hate the inttypes.h format specifiers, but I
agree that we should forward the example of the C99 spec, for better
or for worse.
That being said, the userspace PRIu64, et. al macros are complete
format specifiers, not just a length modifier. And I think this
results in less ugly format specifiers in our kernel code.
cut here ---
#!/bin/sh
cat < /tmp/blah.c
#include
#include
int main(int arg, char **argv)
{
printf("PRIu64 is %s\n", PRIu64);
printf("PRId64 is %s\n", PRId64);
printf("PRIx64 is %s\n", PRIx64);
return 0;
}
EOF
clang -m32 -o /tmp/blah /tmp/blah.c
/tmp/blah
cut here ---
% /tmp/blah.sh
PRIu64 is llu
PRId64 is lld
PRIx64 is llx
Thanks!
- Ted
