From: Lorenzo Stoakes <ljs@kernel.org>
To: Thorsten Blum <thorsten.blum@linux.dev>
Cc: Andrew Morton <akpm@linux-foundation.org>,
David Hildenbrand <david@kernel.org>,
"Liam R. Howlett" <liam@infradead.org>,
Vlastimil Babka <vbabka@kernel.org>,
Mike Rapoport <rppt@kernel.org>,
Suren Baghdasaryan <surenb@google.com>,
Michal Hocko <mhocko@suse.com>,
Yury Norov <yury.norov@gmail.com>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Andy Shevchenko <andriy.shevchenko@intel.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH 3/3] lib/bitmap: use bytes_to_page_end() helper
Date: Mon, 18 May 2026 11:33:24 +0100 [thread overview]
Message-ID: <agrpVfiBkTl3Vf-t@lucifer> (raw)
In-Reply-To: <20260517123428.1181981-6-thorsten.blum@linux.dev>
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@linux.dev>
> ---
> 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
next prev parent reply other threads:[~2026-05-18 10:33 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-17 12:34 [PATCH 1/3] mm: move offset_in_page() to page_helpers.h Thorsten Blum
2026-05-17 12:34 ` [PATCH 2/3] mm: add bytes_to_page_end() helper Thorsten Blum
2026-05-17 15:28 ` Yury Norov
2026-05-18 6:48 ` Andy Shevchenko
2026-05-18 10:23 ` Lorenzo Stoakes
2026-05-18 11:33 ` William Kucharski
2026-05-18 11:53 ` William Kucharski
2026-05-18 9:09 ` David Laight
2026-05-18 10:24 ` Lorenzo Stoakes
2026-05-18 6:49 ` Andy Shevchenko
2026-05-18 10:24 ` Lorenzo Stoakes
2026-05-17 12:34 ` [PATCH 3/3] lib/bitmap: use " Thorsten Blum
2026-05-18 10:33 ` Lorenzo Stoakes [this message]
2026-05-18 6:51 ` [PATCH 1/3] mm: move offset_in_page() to page_helpers.h Andy Shevchenko
2026-05-18 10:02 ` Lorenzo Stoakes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=agrpVfiBkTl3Vf-t@lucifer \
--to=ljs@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=andriy.shevchenko@intel.com \
--cc=david@kernel.org \
--cc=liam@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux@rasmusvillemoes.dk \
--cc=mhocko@suse.com \
--cc=rppt@kernel.org \
--cc=surenb@google.com \
--cc=thorsten.blum@linux.dev \
--cc=vbabka@kernel.org \
--cc=yury.norov@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox