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

[Lorenzo Stoakes]

On Mon, May 18, 2026 at 02:13:54PM +0200, Thorsten Blum wrote:
> Hi Lorenzo,
>
> On Mon, May 18, 2026 at 11:02:10AM +0100, Lorenzo Stoakes wrote:
> > Seriously, please resend this.
> >
> > This is 3 patches with 2 in-reply-to 1/3, that's not how we do series in mm,
> > take a look around :)
> >
> > Write a cover letter, and have all the patches in-reply-to that.
>
> A cover letter seemed unnecessary for such a seemingly trivial 3-patch
> series, but this thread [1] might have been helpful for context.

OK, but this is how we do multi-patch series in mm, as I said. It's always a
good idea to look around a subsystem to get a sense of how things are done as a
courtesy.

So politely request you do this :) though as I said I question this being broken
out into as many patches as it is.

>
> [1] https://lore.kernel.org/lkml/CAHp75VfQNkqEYsO4Uup0c-uiYuVyAWit=tmCz2BsYLp-sjXsZw@xxxxxxxxxxxxxx/
>
> > Was there not previous versions of this? I _seem_ to remember that, but might be
> > misremembering :)
>
> I only know about the one Yury mentioned. This is v1 of my series.
>
> > Also I'm really questioning the value of this, you've not sold why we should
> > take this whatsoever.
> >
> > 'Add a random new header file we have to maintain because it's smaller' is not
> > really hugely compelling.
> >
> > Also a _lot_ of stuff in the kernel ultimately pulls in mm.h. So what exactly
> > has the specific requirement of both needing this define and (somehow) doesn't
> > use mm?
>
> 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?

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

>
> Thanks,
> Thorsten

Cheers, Lorenzo