Currently the bitmap_onto() is available only for CONFIG_NUMA=y case,
while some users may benefit out of it and being independent to NUMA
code.
Make it available to users by moving out of ifdeffery and exporting for
modules.
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
lib/bitmap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 09522af227f1..2feccb5047dc 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -547,7 +547,6 @@ int bitmap_bitremap(int oldbit, const unsigned long *old,
}
EXPORT_SYMBOL(bitmap_bitremap);
-#ifdef CONFIG_NUMA
/**
* bitmap_onto - translate one bitmap relative to another
* @dst: resulting translated bitmap
@@ -681,7 +680,9 @@ void bitmap_onto(unsigned long *dst, const unsigned long *orig,
m++;
}
}
+EXPORT_SYMBOL(bitmap_onto);
+#ifdef CONFIG_NUMA
/**
* bitmap_fold - fold larger bitmap into smaller, modulo specified size
* @dst: resulting smaller bitmap
--
2.43.0On Mon, Feb 12, 2024 at 08:56:31AM +0100, Herve Codina wrote: > Currently the bitmap_onto() is available only for CONFIG_NUMA=y case, > while some users may benefit out of it and being independent to NUMA > code. > > Make it available to users by moving out of ifdeffery and exporting for > modules. Wondering if you are trying to have something like https://lore.kernel.org/lkml/20230926052007.3917389-1-andriy.shevchenko@linux.intel.com/ -- With Best Regards, Andy Shevchenko
Hi Andy, On Mon, 12 Feb 2024 14:27:16 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Mon, Feb 12, 2024 at 08:56:31AM +0100, Herve Codina wrote: > > Currently the bitmap_onto() is available only for CONFIG_NUMA=y case, > > while some users may benefit out of it and being independent to NUMA > > code. > > > > Make it available to users by moving out of ifdeffery and exporting for > > modules. > > Wondering if you are trying to have something like > https://lore.kernel.org/lkml/20230926052007.3917389-1-andriy.shevchenko@linux.intel.com/ > Yes, it looks like. Can you confirm that your bitmap_scatter() do the same operations as the existing bitmap_onto() ? If so, your bitmap_gather() will match my bitmap_off() (patch 4 in this series). Thanks, Hervé -- Hervé Codina, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Mon, Feb 12, 2024 at 02:37:53PM +0100, Herve Codina wrote: > On Mon, 12 Feb 2024 14:27:16 +0200 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Mon, Feb 12, 2024 at 08:56:31AM +0100, Herve Codina wrote: > > > Currently the bitmap_onto() is available only for CONFIG_NUMA=y case, > > > while some users may benefit out of it and being independent to NUMA > > > code. > > > > > > Make it available to users by moving out of ifdeffery and exporting for > > > modules. > > > > Wondering if you are trying to have something like > > https://lore.kernel.org/lkml/20230926052007.3917389-1-andriy.shevchenko@linux.intel.com/ > > Yes, it looks like. > Can you confirm that your bitmap_scatter() do the same operations as the > existing bitmap_onto() ? I have test cases to be 100% sure, but on the first glance, yes it does with the adjustment to the atomicity of the operations (which I do not understand why be atomic in the original bitmap_onto() implementation). This actually gives a question if we should use your approach or mine. At least the help of bitmap_onto() is kinda hard to understand. > If so, your bitmap_gather() will match my bitmap_off() (patch 4 in this > series). Yes. -- With Best Regards, Andy Shevchenko
On Mon, 12 Feb 2024 16:01:38 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Mon, Feb 12, 2024 at 02:37:53PM +0100, Herve Codina wrote: > > On Mon, 12 Feb 2024 14:27:16 +0200 > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > On Mon, Feb 12, 2024 at 08:56:31AM +0100, Herve Codina wrote: > > > > Currently the bitmap_onto() is available only for CONFIG_NUMA=y case, > > > > while some users may benefit out of it and being independent to NUMA > > > > code. > > > > > > > > Make it available to users by moving out of ifdeffery and exporting for > > > > modules. > > > > > > Wondering if you are trying to have something like > > > https://lore.kernel.org/lkml/20230926052007.3917389-1-andriy.shevchenko@linux.intel.com/ > > > > Yes, it looks like. > > Can you confirm that your bitmap_scatter() do the same operations as the > > existing bitmap_onto() ? > > I have test cases to be 100% sure, but on the first glance, yes it does with > the adjustment to the atomicity of the operations (which I do not understand > why be atomic in the original bitmap_onto() implementation). > > This actually gives a question if we should use your approach or mine. > At least the help of bitmap_onto() is kinda hard to understand. Agree, the bitmap_onto() code is simpler to understand than its help. I introduced bitmap_off() to be the "reverse" bitmap_onto() operations and I preferred to avoid duplicating function that do the same things. On my side, I initially didn't use the bitmap_*() functions and did the the bits manipulation by hand. During the review, it was suggested to use the bitmap_*() family and I followed this suggestion. I did tests to be sure that bitmap_onto() and bitmap_off() did exactly the same things as my previous code did. > > > If so, your bitmap_gather() will match my bitmap_off() (patch 4 in this > > series). > > Yes. > Regards, Hervé
On Mon, Feb 12, 2024 at 03:20:22PM +0100, Herve Codina wrote: > On Mon, 12 Feb 2024 16:01:38 +0200 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: ... > Agree, the bitmap_onto() code is simpler to understand than its help. > > I introduced bitmap_off() to be the "reverse" bitmap_onto() operations > and I preferred to avoid duplicating function that do the same things. > > On my side, I initially didn't use the bitmap_*() functions and did the the > bits manipulation by hand. > During the review, it was suggested to use the bitmap_*() family and I followed > this suggestion. I also would go this way, the problems I see with the current implementation are: - being related to NUMA (and as Rasmus once pointed out better to be there); - unclear naming, esp. proposed bitmap_off(); - the quite hard to understand help text - atomicity when it's not needed (AFAICT). > I did tests to be sure that bitmap_onto() and bitmap_off() did > exactly the same things as my previous code did. Yuri, what do you think about all this? -- With Best Regards, Andy Shevchenko
On Mon, Feb 12, 2024 at 04:36:36PM +0200, Andy Shevchenko wrote:
> On Mon, Feb 12, 2024 at 03:20:22PM +0100, Herve Codina wrote:
> > On Mon, 12 Feb 2024 16:01:38 +0200
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> ...
>
> > Agree, the bitmap_onto() code is simpler to understand than its help.
> >
> > I introduced bitmap_off() to be the "reverse" bitmap_onto() operations
> > and I preferred to avoid duplicating function that do the same things.
> >
> > On my side, I initially didn't use the bitmap_*() functions and did the the
> > bits manipulation by hand.
> > During the review, it was suggested to use the bitmap_*() family and I followed
> > this suggestion.
>
> I also would go this way, the problems I see with the current implementation are:
Sure, opencoding and duplicating the functionality is always a bad
idea.
> - being related to NUMA (and as Rasmus once pointed out better to be there);
It's 'related to NUMA' for the only reason - it's used by NUMA only.
Nothing NUMA-specific in the function itself.
Now that we've got a non-NUMA user, the bitmap_onto() is not related
to NUMA anymore.
> - unclear naming, esp. proposed bitmap_off();
That's I agree. Scatter/gather from your last approach sound better.
Do you plan to send a v2?
> - the quite hard to understand help text
Yes, we need a picture that would illustrate what actually happens
> - atomicity when it's not needed (AFAICT).
Agree. A series of atomic ops is not atomic. For example
if (test_bit(n, map))
set_bit(m, map);
is not atomic as a whole. And this is what we do in bitmap_onto/off()
in a loop. This must be fixed by using underscoded version.
> > I did tests to be sure that bitmap_onto() and bitmap_off() did
> > exactly the same things as my previous code did.
>
> Yuri, what do you think about all this?
I think your scatter/gather is better then this onto/off by naming and
implementation. If you'll send a v2, and it would work for Herve, I'd
prefer scatter/gather. But we can live with onto/off as well.
Thanks,
Yury
Hi Andy, Yury,
On Mon, 12 Feb 2024 11:13:13 -0800
Yury Norov <yury.norov@gmail.com> wrote:
...
>
> That's I agree. Scatter/gather from your last approach sound better.
> Do you plan to send a v2?
>
...
>
> I think your scatter/gather is better then this onto/off by naming and
> implementation. If you'll send a v2, and it would work for Herve, I'd
> prefer scatter/gather. But we can live with onto/off as well.
>
Andy, I tested your bitmap_{scatter,gather}() in my code.
I simply replaced my bitmap_{onto,off}() calls by calls to your helpers and
it works perfectly for my use case.
I didn't use your whole patch
"[PATCH v1 2/5] lib/bitmap: Introduce bitmap_scatter() and bitmap_gather() helpers"
because it didn't apply on a v6.8-rc1 based branch.
I just manually extracted the needed functions for my tests and I didn't look
at the lib/test_bitmap.c part.
Now what's the plan ?
Andy, do you want to send a v2 of this patch or may I get the patch, modify it
according to reviews already present in v1 and integrate it in my current
series ?
Yury, any preferences ?
Best regards,
Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Thu, Feb 15, 2024 at 06:46:12PM +0100, Herve Codina wrote:
> On Mon, 12 Feb 2024 11:13:13 -0800
> Yury Norov <yury.norov@gmail.com> wrote:
...
> > That's I agree. Scatter/gather from your last approach sound better.
> > Do you plan to send a v2?
See below.
...
> > I think your scatter/gather is better then this onto/off by naming and
> > implementation. If you'll send a v2, and it would work for Herve, I'd
> > prefer scatter/gather. But we can live with onto/off as well.
>
> Andy, I tested your bitmap_{scatter,gather}() in my code.
> I simply replaced my bitmap_{onto,off}() calls by calls to your helpers and
> it works perfectly for my use case.
>
> I didn't use your whole patch
> "[PATCH v1 2/5] lib/bitmap: Introduce bitmap_scatter() and bitmap_gather() helpers"
> because it didn't apply on a v6.8-rc1 based branch.
> I just manually extracted the needed functions for my tests and I didn't look
> at the lib/test_bitmap.c part.
>
> Now what's the plan ?
> Andy, do you want to send a v2 of this patch or may I get the patch, modify it
> according to reviews already present in v1 and integrate it in my current
> series ?
I would like to do that, but under pile of different things.
I would try my best but if you have enough time and motivation feel free
to take over, address the comments and integrate in your series.
I dunno what to do with bitmap_onto(), perhaps in a separate patch we can
replace it with bitmap_scatter() (IIUC) with explanation that the former
1) uses atomic ops while being non-atomic as a whole, and b) having quite
hard to get documentation. At least that's how I see it, I mean that I would
like to leave bitmap_onto() alone and address it separately.
> Yury, any preferences ?
--
With Best Regards,
Andy Shevchenko
Hi Andy, Yury,
On Thu, 15 Feb 2024 21:17:23 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
[...]
> > Now what's the plan ?
> > Andy, do you want to send a v2 of this patch or may I get the patch, modify it
> > according to reviews already present in v1 and integrate it in my current
> > series ?
>
> I would like to do that, but under pile of different things.
> I would try my best but if you have enough time and motivation feel free
> to take over, address the comments and integrate in your series.
>
> I dunno what to do with bitmap_onto(), perhaps in a separate patch we can
> replace it with bitmap_scatter() (IIUC) with explanation that the former
> 1) uses atomic ops while being non-atomic as a whole, and b) having quite
> hard to get documentation. At least that's how I see it, I mean that I would
> like to leave bitmap_onto() alone and address it separately.
>
I will take the Andy's bitmap_{scatter,gather}() patch in my next iteration.
And use bitmap_{scatter,gather}() in my code.
For bitmap_onto() replacement, nothing will be done in my next iteration as
it is out of this series scope.
Hervé
On Wed, Feb 21, 2024 at 02:44:31PM +0100, Herve Codina wrote:
> On Thu, 15 Feb 2024 21:17:23 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
[...]
> > > Now what's the plan ?
> > > Andy, do you want to send a v2 of this patch or may I get the patch, modify it
> > > according to reviews already present in v1 and integrate it in my current
> > > series ?
> >
> > I would like to do that, but under pile of different things.
> > I would try my best but if you have enough time and motivation feel free
> > to take over, address the comments and integrate in your series.
> >
> > I dunno what to do with bitmap_onto(), perhaps in a separate patch we can
> > replace it with bitmap_scatter() (IIUC) with explanation that the former
> > 1) uses atomic ops while being non-atomic as a whole, and b) having quite
> > hard to get documentation. At least that's how I see it, I mean that I would
> > like to leave bitmap_onto() alone and address it separately.
>
> I will take the Andy's bitmap_{scatter,gather}() patch in my next iteration.
> And use bitmap_{scatter,gather}() in my code.
Thank you and sorry that I have no time to finish that. I will be happy to help
reviewing if you Cc me.
> For bitmap_onto() replacement, nothing will be done in my next iteration as
> it is out of this series scope.
I agree on this. This will be a separate logical change related to NUMA with
explanation and replacement of all callers at once.
--
With Best Regards,
Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.