fs/efs/file.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
Firstly, the current code uses formatters that are incompatible with the
most recent GCC for x86_64. Replace them and explicitly cast the
formatted variables to their respective forms of long long.
Secondly, replace the legacy conditional compilation for DEBUG with the
more preferred pr_debug() to condense the code, while keeping the final
output functionally identical.
Fixes: f403d1dbac6d ("fs/efs: add pr_fmt / use __func__")
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
Test compiled only with gcc 16.1.1 for x86_64.
fs/efs/file.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/fs/efs/file.c b/fs/efs/file.c
index 9e641da6fab2..625b715f2d96 100644
--- a/fs/efs/file.c
+++ b/fs/efs/file.c
@@ -19,13 +19,12 @@ int efs_get_block(struct inode *inode, sector_t iblock,
if (create)
return error;
if (iblock >= inode->i_blocks) {
-#ifdef DEBUG
/*
* i have no idea why this happens as often as it does
*/
- pr_warn("%s(): block %d >= %ld (filesize %ld)\n",
- __func__, block, inode->i_blocks, inode->i_size);
-#endif
+ pr_debug("EFS: block %llu >= %llu (filesize %lld)\n",
+ (unsigned long long)iblock, (unsigned long long)inode->i_blocks,
+ (long long)inode->i_size);
return 0;
}
phys = efs_map_block(inode, iblock);
@@ -43,13 +42,12 @@ int efs_bmap(struct inode *inode, efs_block_t block) {
/* are we about to read past the end of a file ? */
if (!(block < inode->i_blocks)) {
-#ifdef DEBUG
/*
* i have no idea why this happens as often as it does
*/
- pr_warn("%s(): block %d >= %ld (filesize %ld)\n",
- __func__, block, inode->i_blocks, inode->i_size);
-#endif
+ pr_debug("EFS: block %llu >= %llu (filesize %lld)\n",
+ (unsigned long long)block, (unsigned long long)inode->i_blocks,
+ (long long)inode->i_size);
return 0;
}
--
2.54.0(cc linux-fsdevel)
On Thu, 4 Jun 2026 15:24:40 -0500 Maxwell Doose <m32285159@gmail.com> wrote:
> Firstly, the current code uses formatters that are incompatible with the
> most recent GCC for x86_64. Replace them and explicitly cast the
> formatted variables to their respective forms of long long.
Please quote the compiler output in the changelog.
> Secondly, replace the legacy conditional compilation for DEBUG with the
> more preferred pr_debug() to condense the code, while keeping the final
> output functionally identical.
>
> Fixes: f403d1dbac6d ("fs/efs: add pr_fmt / use __func__")
>
> ...
>
> Test compiled only with gcc 16.1.1 for x86_64.
Compiled how? DEBUG was defined in some fashion?
> --- a/fs/efs/file.c
> +++ b/fs/efs/file.c
> @@ -19,13 +19,12 @@ int efs_get_block(struct inode *inode, sector_t iblock,
> if (create)
> return error;
> if (iblock >= inode->i_blocks) {
> -#ifdef DEBUG
> /*
> * i have no idea why this happens as often as it does
> */
> - pr_warn("%s(): block %d >= %ld (filesize %ld)\n",
> - __func__, block, inode->i_blocks, inode->i_size);
That wouldn't have compiled anyway - `block' is undefined?
> -#endif
> + pr_debug("EFS: block %llu >= %llu (filesize %lld)\n",
> + (unsigned long long)iblock, (unsigned long long)inode->i_blocks,
> + (long long)inode->i_size);
It seems that nowadays sector_t and blkcnt_t are unconditionally u64
and loff_t is unconditionally `long long'. I think, please check.
If so, no casts are needed.
> return 0;
> }
> phys = efs_map_block(inode, iblock);
> @@ -43,13 +42,12 @@ int efs_bmap(struct inode *inode, efs_block_t block) {
>
> /* are we about to read past the end of a file ? */
> if (!(block < inode->i_blocks)) {
> -#ifdef DEBUG
> /*
> * i have no idea why this happens as often as it does
> */
> - pr_warn("%s(): block %d >= %ld (filesize %ld)\n",
> - __func__, block, inode->i_blocks, inode->i_size);
> -#endif
> + pr_debug("EFS: block %llu >= %llu (filesize %lld)\n",
> + (unsigned long long)block, (unsigned long long)inode->i_blocks,
> + (long long)inode->i_size);
> return 0;
> }
On Thu, Jun 4, 2026 at 4:30 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> (cc linux-fsdevel)
>
Whoops, I guess I'm too reliant on --cc-cmd :(
> On Thu, 4 Jun 2026 15:24:40 -0500 Maxwell Doose <m32285159@gmail.com> wrote:
>
> > Firstly, the current code uses formatters that are incompatible with the
> > most recent GCC for x86_64. Replace them and explicitly cast the
> > formatted variables to their respective forms of long long.
>
> Please quote the compiler output in the changelog.
>
It's rather lengthy:
fs/efs/file.c: In function ‘efs_get_block’:
fs/efs/file.c:26:35: error: ‘block’ undeclared (first use in this
function); did you mean ‘iblock’?
26 | __func__, block, inode->i_blocks, inode->i_size);
| ^~~~~
./include/linux/printk.h:483:33: note: in definition of macro
‘printk_index_wrap’
483 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
./include/linux/printk.h:564:9: note: in expansion of macro ‘printk’
564 | printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
fs/efs/file.c:25:17: note: in expansion of macro ‘pr_warn’
25 | pr_warn("%s(): block %d >= %ld (filesize %ld)\n",
| ^~~~~~~
fs/efs/file.c:26:35: note: each undeclared identifier is reported only
once for each function it appears i
n
./include/linux/printk.h:483:33: note: in definition of macro
‘printk_index_wrap’
483 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
./include/linux/printk.h:564:9: note: in expansion of macro ‘printk’
564 | printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
fs/efs/file.c:25:17: note: in expansion of macro ‘pr_warn’
25 | pr_warn("%s(): block %d >= %ld (filesize %ld)\n",
| ^~~~~~~
fs/efs/file.c: In function ‘efs_bmap’:
./include/linux/kern_levels.h:5:25: error: format ‘%ld’ expects
argument of type ‘long int’, but argument
4 has type ‘blkcnt_t’ {aka ‘long long unsigned int’} [-Werror=format=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
./include/linux/printk.h:483:25: note: in definition of macro
‘printk_index_wrap’
483 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
./include/linux/printk.h:564:9: note: in expansion of macro ‘printk’
564 | printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
./include/linux/kern_levels.h:12:25: note: in expansion of macro ‘KERN_SOH’
12 | #define KERN_WARNING KERN_SOH "4" /* warning conditions */
| ^~~~~~~~
./include/linux/printk.h:564:16: note: in expansion of macro ‘KERN_WARNING’
564 | printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~
fs/efs/file.c:47:17: note: in expansion of macro ‘pr_warn’
47 | pr_warn("%s(): block %d >= %ld (filesize %ld)\n",
| ^~~~~~~
./include/linux/kern_levels.h:5:25: error: format ‘%ld’ expects
argument of type ‘long int’, but argument
5 has type ‘loff_t’ {aka ‘long long int’} [-Werror=format=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
./include/linux/printk.h:483:25: note: in definition of macro
‘printk_index_wrap’
483 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
./include/linux/printk.h:564:9: note: in expansion of macro ‘printk’
564 | printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
./include/linux/kern_levels.h:12:25: note: in expansion of macro ‘KERN_SOH’
12 | #define KERN_WARNING KERN_SOH "4" /* warning conditions */
| ^~~~~~~~
./include/linux/printk.h:564:16: note: in expansion of macro ‘KERN_WARNING’
564 | printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~
fs/efs/file.c:47:17: note: in expansion of macro ‘pr_warn’
47 | pr_warn("%s(): block %d >= %ld (filesize %ld)\n",
| ^~~~~~~
(excuse the misalignment of some of this stuff, I'm using gmail to send this.)
> > Secondly, replace the legacy conditional compilation for DEBUG with the
> > more preferred pr_debug() to condense the code, while keeping the final
> > output functionally identical.
> >
> > Fixes: f403d1dbac6d ("fs/efs: add pr_fmt / use __func__")
> >
> > ...
> >
>
> > Test compiled only with gcc 16.1.1 for x86_64.
>
> Compiled how? DEBUG was defined in some fashion?
>
I cheated a little and removed the #ifdef DEBUG to compile-test it.
> > --- a/fs/efs/file.c
> > +++ b/fs/efs/file.c
> > @@ -19,13 +19,12 @@ int efs_get_block(struct inode *inode, sector_t iblock,
> > if (create)
> > return error;
> > if (iblock >= inode->i_blocks) {
> > -#ifdef DEBUG
> > /*
> > * i have no idea why this happens as often as it does
> > */
> > - pr_warn("%s(): block %d >= %ld (filesize %ld)\n",
> > - __func__, block, inode->i_blocks, inode->i_size);
>
> That wouldn't have compiled anyway - `block' is undefined?
>
Correct, so we're fixing two bugs in one patch (this one is so trivial
to fix that I just included it).
> > -#endif
> > + pr_debug("EFS: block %llu >= %llu (filesize %lld)\n",
> > + (unsigned long long)iblock, (unsigned long long)inode->i_blocks,
> > + (long long)inode->i_size);
>
> It seems that nowadays sector_t and blkcnt_t are unconditionally u64
> and loff_t is unconditionally `long long'. I think, please check.
>
> If so, no casts are needed.
>
Looks like I got a bit sloppy:
fs/efs/file.c: In function ‘efs_bmap’:
./include/linux/kern_levels.h:5:25: error: format ‘%llu’ expects
argument of type ‘long long unsigned int’
, but argument 3 has type ‘efs_block_t’ {aka ‘int’} [-Werror=format=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
This is the one I'm worried about, it seems I casted to ULL in error
and the formatter should be %d. And I think we can remove the casting,
just double checked (all compilations were HEAD~2 (so e8c2f9fdadee
("Merge tag 'for-7.1/hpfs-fixes' of
git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm")))
Will send a v2 tomorrow with that fix (unless you want to tweak while
applying, of course, but obviously bad practice to rely on that).
--
best regards,
max
On Thu, 4 Jun 2026 20:40:30 -0500 Maxwell Doose <m32285159@gmail.com> wrote: > On Thu, Jun 4, 2026 at 4:30 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > (cc linux-fsdevel) > > > > Whoops, I guess I'm too reliant on --cc-cmd :( > > > On Thu, 4 Jun 2026 15:24:40 -0500 Maxwell Doose <m32285159@gmail.com> wrote: > > > > > Firstly, the current code uses formatters that are incompatible with the > > > most recent GCC for x86_64. Replace them and explicitly cast the > > > formatted variables to their respective forms of long long. > > > > Please quote the compiler output in the changelog. > > > > It's rather lengthy: > fs/efs/file.c: In function ‘efs_get_block’: > fs/efs/file.c:26:35: error: ‘block’ undeclared (first use in this > function); did you mean ‘iblock’? Right. You're evidently the first person who tried to compile this in more than 20 years. How about we just delete it all? If some future developer wants to know block numbers and stuff, they know how to use printk.
On Thu, Jun 4, 2026 at 9:53 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Thu, 4 Jun 2026 20:40:30 -0500 Maxwell Doose <m32285159@gmail.com> wrote: > > > On Thu, Jun 4, 2026 at 4:30 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > > > (cc linux-fsdevel) > > > > > > > Whoops, I guess I'm too reliant on --cc-cmd :( > > > > > On Thu, 4 Jun 2026 15:24:40 -0500 Maxwell Doose <m32285159@gmail.com> wrote: > > > > > > > Firstly, the current code uses formatters that are incompatible with the > > > > most recent GCC for x86_64. Replace them and explicitly cast the > > > > formatted variables to their respective forms of long long. > > > > > > Please quote the compiler output in the changelog. > > > > > > > It's rather lengthy: > > fs/efs/file.c: In function ‘efs_get_block’: > > fs/efs/file.c:26:35: error: ‘block’ undeclared (first use in this > > function); did you mean ‘iblock’? > > Right. You're evidently the first person who tried to compile this in > more than 20 years. > > How about we just delete it all? If some future developer wants to > know block numbers and stuff, they know how to use printk. That sounds good, I'll get that patch going. -- best regards, max ps: I noticed this was listed as an orphan in MAINTAINERS. I'd be happy to step up and maintain this, I've got a bit of experience maintaining stuff in iio. Hopefully this isn't too far-fetched!
On Thu, Jun 04, 2026 at 10:16:44PM -0500, Maxwell Doose wrote: > ps: I noticed this was listed as an orphan in MAINTAINERS. I'd be > happy to step up and maintain this, I've got a bit of experience > maintaining stuff in iio. Hopefully this isn't too far-fetched! I'd be just as happy to see it deleted. efs was succeeded by XFS in the mid-90s. The last efs filesystem I personally saw was in 1999 (an SGI Freeware CD). If there's any filesystem crying out to be converted to FUSE and removed from the kernel, this is it.
On Thu, Jun 4, 2026 at 10:27 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Thu, Jun 04, 2026 at 10:16:44PM -0500, Maxwell Doose wrote: > > ps: I noticed this was listed as an orphan in MAINTAINERS. I'd be > > happy to step up and maintain this, I've got a bit of experience > > maintaining stuff in iio. Hopefully this isn't too far-fetched! > > I'd be just as happy to see it deleted. efs was succeeded by XFS in > the mid-90s. The last efs filesystem I personally saw was in 1999 (an > SGI Freeware CD). If there's any filesystem crying out to be converted > to FUSE and removed from the kernel, this is it. I guess that is a fair point. But given the fact that we quite literally just found a compile-time bug I think there may be further underlying issues that need to be resolved before conversion to FUSE :/
On Thu, Jun 04, 2026 at 10:33:10PM -0500, Maxwell Doose wrote: > On Thu, Jun 4, 2026 at 10:27 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > On Thu, Jun 04, 2026 at 10:16:44PM -0500, Maxwell Doose wrote: > > > ps: I noticed this was listed as an orphan in MAINTAINERS. I'd be > > > happy to step up and maintain this, I've got a bit of experience > > > maintaining stuff in iio. Hopefully this isn't too far-fetched! > > > > I'd be just as happy to see it deleted. efs was succeeded by XFS in > > the mid-90s. The last efs filesystem I personally saw was in 1999 (an > > SGI Freeware CD). If there's any filesystem crying out to be converted > > to FUSE and removed from the kernel, this is it. > > I guess that is a fair point. But given the fact that we quite > literally just found a compile-time bug I think there may be further > underlying issues that need to be resolved before conversion to FUSE > :/ I'd argue that shows the exact opposite. efs just isn't useful any more. While looking up some information on Wikipedia, I discovered that XFS filesystems from SGI used v2 of the directory format. Linux no longer supports anything from before v4 (and even that is now under a CONFIG option). So we're in the weird situation where SGI filesystems from 1992 are readable in Linux, but SGI filesystems from 1995 are not.
© 2016 - 2026 Red Hat, Inc.