Re: [PATCH] bitmap: optimize bitmap_remap()

From: Andy Shevchenko
Date: Mon Aug 21 2023 - 04:56:36 EST


On Mon, Aug 21, 2023 at 09:48:38AM +0200, Rasmus Villemoes wrote:
> On 19/08/2023 04.03, Yury Norov wrote:
> > On Thu, Aug 17, 2023 at 06:37:01PM +0300, Andy Shevchenko wrote:
>
> >> But this gives +89 bytes on x86_64... :-(
> >
> > Who cares if it gives a boost of performance for regular users?
>
> "Regular users" never ever hit bitmap_remap, it's simply too esoteric.
> It has all of _two_ users in the whole tree, one in some gpio driver,
> another only reached via a system call that nobody ever uses, and if
> they do, it's most likely some one-time-per-process thing. It's about as
> far from a hot path that you can come.
>
> If it wasn't for that xilinx user, those bitmap_remap and bitmap_onto
> etc. should be moved to be private to the NUMA code.
>
> Anyway, I think those +89 was for Andy's own counterproposal. I haven't
> built Yury's patch, but from a quick look, it should not add that much,
> if anything - it adds a test, call, early return, but OTOH it helps the
> compiler to combine the two set_bit (since only the first argument
> differs), and loses the lock prefix.
>
> As for that latter point, I don't think a separate patch is worth it,
> just a comment in the commit log - we're already doing a bitmap_zero()
> on dst which certainly doesn't use any atomic ops, and in general all
> the bitmap_* functions expect the caller to handle locking.
>
> So I don't mind Yury's patch, but I highly doubt it matters at all. The
> comment mentions an example, do we even have that put in test code?

Btw, I have locally a patch that adds bitmap_scatter()/bitmap_gather() and
switches Xilins to use it. Because that is what GPIO may use, see the code
below for the reference (there are no test cases yet), Comments are welcome
as usual!