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
