Re: [PATCH 1/3] mm: move offset_in_page() to page_helpers.h
From: Thorsten Blum
Date: Mon May 18 2026 - 09:40:59 EST
On Mon, May 18, 2026 at 01:33:54PM +0100, Lorenzo Stoakes wrote:
> On Mon, May 18, 2026 at 02:13:54PM +0200, Thorsten Blum wrote:
> > [...]
> > Patch 3/3 is one of many examples that pulls in all of mm.h just for
> > offset_in_page(). lib/string.c from the same thread [1] is another
> > example that would need to include mm.h just for offset_in_page().
>
> And that's a problem why? Compile time? Somehow binary bloat? Conflicts?
> Confusion?
This is more about separation of concerns than about compile time, which
can still matter here, since mm.h is large, pulls in many other headers,
and any changes to those can cause unrelated files to be recompiled.
I don't mind including mm.h where the code already depends on MM
interfaces, but offset_in_page() itself is a small page-arithmetic
helper. Having it buried 3000+ lines into mm.h also makes it less
discoverable, which may be one reason it's not used more often.
> > Many other files (hundreds) don't use offset_in_page(), but open-code it
> > in many different ways instead:
> >
> > (unsigned long)p & ~PAGE_MASK
> > (unsigned long)p & (PAGE_SIZE - 1)
> > (long)p & (PAGE_SIZE - 1)
> > ...
> >
> > I can't tell whether they didn't know about offset_in_page(), or
> > deliberately chose not to include mm.h.
>
> OK but this series doesn't address any of that? It just adds a new header, a
> questionable macro and uses it in one place? So those justifications are rather
> moot here.
My intent was to make the helper available from a smaller header first,
and to use patch 3/3 as a concrete example where including all of mm.h
is unnecessary. Other call sites can then be converted incrementally
over time, rather than sending a tree-wide conversion in the same
series.
Thanks,
Thorsten