Hi Johannes,
On Tue, Nov 26, 2024 at 10:37 AM Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> wrote:
On Tue, 2024-11-26 at 09:38 +0100, Geert Uytterhoeven wrote:
Thanks for your patch, which is now commit 8dc6d81c6b2acc43 ("debugfs:
add small file operations for most files") upstream.
Or rather "no thanks" ;-)
+#define DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT BIT(1)
As the minimum alignment is 2 on m68k, you cannot use bit 1 in pointers
for your own private use.
D'oh. Sorry about that. Though honestly even if I _had_ seen such a
comment deep in maple tree code, on that struct I'd probably have
assumed it's because there's no pointer in it and thus no alignment
anyway...
But sounds like you're pointers don't need to be naturally aligned, and
so 2-byte alignment is sufficient.
I guess we _could_ solve that by __aligned(4) on the fops structs, but
... I'm not sure that makes sense.
Reverting commit 8dc6d81c6b2acc43 fixes the issue,
Clearly. Though have to also revert the related patch in wireless :)
and reduces the
atari_defconfig kernel size by 447 bytes, according to bloat-o-meter.
Well, fair point, but if we care about size then we can win back more
than this by converting about a handful of debugfs files to short ops,
and if there are no debugfs files then we wouldn't need debugfs either,
I'd think.
So I think in a way the size argument goes the other way (with a little
bit of extra work), if that was meant to be an argument at all? :-)
Assuming non-wireless drivers can be converted, too? As none of the
classical m68k machines support wireless, so far nothing is gained...
From: Johannes Berg <johannes.berg@xxxxxxxxx>
Date: Tue, 26 Nov 2024 10:29:23 +0100
Subject: [PATCH] fs: debugfs: differentiate short fops with proxy ops
Geert reported that my previous short fops debugfs changes
broke m68k, because it only has mandatory alignement of two,
so we can't stash the "is it short" information into the
pointer (as we already did with the "is it real" bit.)
Instead, exploit the fact that debugfs_file_get() called on
an already open file will already find that the fsdata is
no longer the real fops but rather the allocated data that
already distinguishes full/short ops, so only open() needs
to be able to distinguish. We can achieve that by using two
different open functions.
Unfortunately this requires another set of full file ops,
increasing the size by 536 bytes (x86-64), but that's still
226 on m68k.
a reasonable trade-off given that only converting some of
the wireless stack gained over 28k. This brings the total
cost of this to around 1k, for wins of 28k (all x86-64).
Reported-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Link: https://lore.kernel.org/CAMuHMdWu_9-L2Te101w8hU7H_2yobJFPXSwwUmGHSJfaPWDKiQ@xxxxxxxxxxxxxx
Signed-off-by: Johannes Berg <johannes.berg@xxxxxxxxx>
Thanks, that fixed the issue!
Tested-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds