[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMuHMdXbCaMBwjrb+ZJj+MMQvOm8Y=xKfaxVhYVb79WyO4Z-4Q@mail.gmail.com>
Date: Tue, 26 Nov 2024 15:16:59 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Johannes Berg <johannes@...solutions.net>
Cc: linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Alexander Viro <viro@...iv.linux.org.uk>,
"Rafael J . Wysocki" <rafael@...nel.org>, linux-m68k <linux-m68k@...ts.linux-m68k.org>
Subject: Re: [PATCH 1/2] debugfs: add small file operations for most files
Hi Johannes,
On Tue, Nov 26, 2024 at 10:37 AM Johannes Berg
<johannes@...solutions.net> 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@...el.com>
> 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@...ux-m68k.org>
> Link: https://lore.kernel.org/CAMuHMdWu_9-L2Te101w8hU7H_2yobJFPXSwwUmGHSJfaPWDKiQ@mail.gmail.com
> Signed-off-by: Johannes Berg <johannes.berg@...el.com>
Thanks, that fixed the issue!
Tested-by: Geert Uytterhoeven <geert@...ux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
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
Powered by blists - more mailing lists