Re: [PATCH 3/3] lib/bitmap: use bytes_to_page_end() helper
From: Lorenzo Stoakes
Date: Mon May 18 2026 - 07:06:48 EST
On Sun, May 17, 2026 at 02:34:31PM +0200, Thorsten Blum wrote:
> bitmap-str.c includes linux/mm.h for offset_in_page() and kfree().
I'd specifically say that it's including linux/mm.h which in turn includes
linux/slab.h and that you're fixing this, because saying you include mm.h for
kfree() is a little confusing.
> Instead, include linux/page_helpers.h and linux/slab.h directly, and
> use bytes_to_page_end() to simplify the code.
This is again, a useless commit message. You're not explaining why what for, how
etc., you're putting in words what the code is doing.
And I'm still very confused as to what the motive is here overall. What's so
problematic about including that header? Longer compile times, somehow? If so,
what are the measurements before/after this change? What was the problem that
led to it? Etc.
We could end up with hundreds of super specific headers files for things, so
there really has to be a good reason for it.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@xxxxxxxxx>
> ---
> lib/bitmap-str.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/bitmap-str.c b/lib/bitmap-str.c
> index be745209507a..bf245a3eae4a 100644
> --- a/lib/bitmap-str.c
> +++ b/lib/bitmap-str.c
> @@ -7,7 +7,8 @@
> #include <linux/export.h>
> #include <linux/hex.h>
> #include <linux/kernel.h>
> -#include <linux/mm.h>
> +#include <linux/page_helpers.h>
> +#include <linux/slab.h>
> #include <linux/string.h>
>
> #include "kstrtox.h"
> @@ -58,7 +59,7 @@ EXPORT_SYMBOL(bitmap_parse_user);
> int bitmap_print_to_pagebuf(bool list, char *buf, const unsigned long *maskp,
> int nmaskbits)
> {
> - ptrdiff_t len = PAGE_SIZE - offset_in_page(buf);
> + ptrdiff_t len = bytes_to_page_end(buf);
yeah this is kind of nasty, going from a clear thing to a 'I wonder how that is
implemented' mystery meat function. I think the original is better.
>
> return list ? scnprintf(buf, len, "%*pbl\n", nmaskbits, maskp) :
> scnprintf(buf, len, "%*pb\n", nmaskbits, maskp);
Thanks, Lorenzo