Re: [PATCH 1/3] mm: move offset_in_page() to page_helpers.h

[Lorenzo Stoakes <ljs@kernel.org>]

On Mon, May 18, 2026 at 03:36:14PM +0200, Thorsten Blum wrote:
> 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.

See the side thread with David, putting it in vdso/page.h is fine, and should
achieve your aims.

>
> > > 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.

Yeah I'm not sure it's at all clear anybody will pay attention to that (by not
sure I mean - happy to bet a beer or 2 against :)

Anyway I'm not going to demand that you update all the callsites, if you split
this into 2 patches as per above + send w/cover letter this is fine.

>
> Thanks,
> Thorsten

Thanks, Lorenzo