[PATCH v2 0/2] mm/vdso: make offset_in_page() usable without linux/mm.h

[PATCH v2 0/2] mm/vdso: make offset_in_page() usable without linux/mm.h

[Thorsten Blum]

offset_in_page() is a small page-arithmetic helper that has been around
for 20+ years. However, page-offset calculations are still open-coded in
many places and in different ways:

	(unsigned long)p & ~PAGE_MASK
	(unsigned long)p & (PAGE_SIZE - 1)
	(long)p & (PAGE_SIZE - 1)
	...

Some of these open-coded instances may be due to offset_in_page() being
buried 3000+ lines into linux/mm.h; others may have avoided including
linux/mm.h, which is a large header that pulls in many others.

Patch 1 moves offset_in_page() from linux/mm.h to vdso/page.h, which
keeps the helper with other low-level page definitions and allows users
that only need offset_in_page() to avoid including linux/mm.h.

Patch 2 shows a concrete example where including vdso/page.h is
sufficient and including the large linux/mm.h would be unnecessary.

Existing users of offset_in_page() do not need to change because patch 1
also includes vdso/page.h from linux/mm.h.

This series is based on akpm/mm.git mm-nonmm-unstable commit
d067a83c8063 ("string: use min in sized_strscpy"), which is also in
linux-next.

Changes in v2:
- Add a cover letter and drop the bytes_to_page_end() helper
- Move offset_in_page() to vdso/page.h as suggested by David and Lorenzo
- Use offset_in_page() in lib/string.c as an example since
  bitmap_print_to_pagebuf() in lib/bitmap-str.c is being removed [1]
- v1: https://lore.kernel.org/lkml/20260517123428.1181981-4-thorsten.blum@linux.dev/

[1] https://lore.kernel.org/lkml/20260519163058.953690-3-ynorov@nvidia.com/
---
Thorsten Blum (2):
  vdso: move offset_in_page() from linux/mm.h to vdso/page.h
  string: use offset_in_page() in sized_strscpy()

 include/linux/mm.h  | 2 +-
 include/vdso/page.h | 2 ++
 lib/string.c        | 3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)


base-commit: d067a83c8063d1bdcbd9af8e1326d846f85138b8

[PATCH v2 1/2] vdso: move offset_in_page() from linux/mm.h to vdso/page.h

[Thorsten Blum]

Move offset_in_page() out of linux/mm.h so users that only need page
offset calculations can include the lightweight vdso/page.h header
instead of pulling in the large linux/mm.h.

Include vdso/page.h from linux/mm.h so existing users of
offset_in_page() continue to build.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 include/linux/mm.h  | 2 +-
 include/vdso/page.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 9cedc5e75aa9..d5c7f1ca80ef 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -37,6 +37,7 @@
 #include <linux/bitmap.h>
 #include <linux/bitops.h>
 #include <linux/iommu-debug-pagealloc.h>
+#include <vdso/page.h>
 
 struct mempolicy;
 struct anon_vma;
@@ -3023,7 +3024,6 @@ static inline void clear_page_pfmemalloc(struct page *page)
  */
 extern void pagefault_out_of_memory(void);
 
-#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
 #define offset_in_folio(folio, p) ((unsigned long)(p) & (folio_size(folio) - 1))
 
 /*
diff --git a/include/vdso/page.h b/include/vdso/page.h
index bc47186c07fc..948b13091084 100644
--- a/include/vdso/page.h
+++ b/include/vdso/page.h
@@ -28,4 +28,6 @@
 #define PAGE_MASK	(~(PAGE_SIZE - 1))
 #endif
 
+#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
+
 #endif	/* __VDSO_PAGE_H */

[PATCH v2 2/2] string: use offset_in_page() in sized_strscpy()

[Thorsten Blum]

Replace the open-coded implementation with offset_in_page() to simplify
sized_strscpy().

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 lib/string.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/string.c b/lib/string.c
index 1f9297e9776a..7c72adc7377c 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -25,6 +25,7 @@
 #include <linux/stddef.h>
 #include <linux/string.h>
 #include <linux/types.h>
+#include <vdso/page.h>
 
 #include <asm/page.h>
 #include <asm/rwonce.h>
@@ -127,7 +128,7 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count)
 	 * since we don't know if the next page is mapped.
 	 */
 	if ((long)src & (sizeof(long) - 1))
-		max = min(PAGE_SIZE - ((long)src & (PAGE_SIZE - 1)), max);
+		max = min(PAGE_SIZE - offset_in_page(src), max);
 #else
 	/* If src or dest is unaligned, don't do word-at-a-time. */
 	if (((long) dest | (long) src) & (sizeof(long) - 1))

Re: [PATCH v2 1/2] vdso: move offset_in_page() from linux/mm.h to vdso/page.h

[David Laight]

On Thu, 21 May 2026 11:06:57 +0200
Thorsten Blum <thorsten.blum@linux.dev> wrote:

> Move offset_in_page() out of linux/mm.h so users that only need page
> offset calculations can include the lightweight vdso/page.h header
> instead of pulling in the large linux/mm.h.
> 
> Include vdso/page.h from linux/mm.h so existing users of
> offset_in_page() continue to build.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  include/linux/mm.h  | 2 +-
>  include/vdso/page.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 9cedc5e75aa9..d5c7f1ca80ef 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -37,6 +37,7 @@
>  #include <linux/bitmap.h>
>  #include <linux/bitops.h>
>  #include <linux/iommu-debug-pagealloc.h>
> +#include <vdso/page.h>

That is already picked up otherwise PAGE_MASK would be undefined.

The whole vdso headers is a mess.
I moved one of the definitions (to do with bitmasks) back into its main header.
The vdso compiled fine because all the 'normal' kernel headers get included
in vdso builds.

-- David

>  
>  struct mempolicy;
>  struct anon_vma;
> @@ -3023,7 +3024,6 @@ static inline void clear_page_pfmemalloc(struct page *page)
>   */
>  extern void pagefault_out_of_memory(void);
>  
> -#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
>  #define offset_in_folio(folio, p) ((unsigned long)(p) & (folio_size(folio) - 1))
>  
>  /*
> diff --git a/include/vdso/page.h b/include/vdso/page.h
> index bc47186c07fc..948b13091084 100644
> --- a/include/vdso/page.h
> +++ b/include/vdso/page.h
> @@ -28,4 +28,6 @@
>  #define PAGE_MASK	(~(PAGE_SIZE - 1))
>  #endif
>  
> +#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
> +
>  #endif	/* __VDSO_PAGE_H */

Re: [PATCH v2 1/2] vdso: move offset_in_page() from linux/mm.h to vdso/page.h

[Lorenzo Stoakes]

On Thu, May 21, 2026 at 11:06:57AM +0200, Thorsten Blum wrote:
> Move offset_in_page() out of linux/mm.h so users that only need page
> offset calculations can include the lightweight vdso/page.h header
> instead of pulling in the large linux/mm.h.
>
> Include vdso/page.h from linux/mm.h so existing users of
> offset_in_page() continue to build.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>

LGTM other than dropping the include below (maybe Andrew can do it for
us?), so:

Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>

> ---
>  include/linux/mm.h  | 2 +-
>  include/vdso/page.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 9cedc5e75aa9..d5c7f1ca80ef 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -37,6 +37,7 @@
>  #include <linux/bitmap.h>
>  #include <linux/bitops.h>
>  #include <linux/iommu-debug-pagealloc.h>
> +#include <vdso/page.h>

Yeah as per David Laight, we don't need this.

>
>  struct mempolicy;
>  struct anon_vma;
> @@ -3023,7 +3024,6 @@ static inline void clear_page_pfmemalloc(struct page *page)
>   */
>  extern void pagefault_out_of_memory(void);
>
> -#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
>  #define offset_in_folio(folio, p) ((unsigned long)(p) & (folio_size(folio) - 1))
>
>  /*
> diff --git a/include/vdso/page.h b/include/vdso/page.h
> index bc47186c07fc..948b13091084 100644
> --- a/include/vdso/page.h
> +++ b/include/vdso/page.h
> @@ -28,4 +28,6 @@
>  #define PAGE_MASK	(~(PAGE_SIZE - 1))
>  #endif
>
> +#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
> +
>  #endif	/* __VDSO_PAGE_H */

Re: [PATCH v2 2/2] string: use offset_in_page() in sized_strscpy()

[Lorenzo Stoakes]

On Thu, May 21, 2026 at 11:06:58AM +0200, Thorsten Blum wrote:
> Replace the open-coded implementation with offset_in_page() to simplify
> sized_strscpy().
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>

I feel this patch actually makes sized_strscpy() more difficult to understand
unfortunately, so not really in favour of us taking it.

> ---
>  lib/string.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/string.c b/lib/string.c
> index 1f9297e9776a..7c72adc7377c 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -25,6 +25,7 @@
>  #include <linux/stddef.h>
>  #include <linux/string.h>
>  #include <linux/types.h>
> +#include <vdso/page.h>
>
>  #include <asm/page.h>
>  #include <asm/rwonce.h>
> @@ -127,7 +128,7 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count)
>  	 * since we don't know if the next page is mapped.
>  	 */
>  	if ((long)src & (sizeof(long) - 1))
> -		max = min(PAGE_SIZE - ((long)src & (PAGE_SIZE - 1)), max);
> +		max = min(PAGE_SIZE - offset_in_page(src), max);

This isn't strictly the same code, offset_in_page() is:

#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)

So this is now, effectively:

-		max = min(PAGE_SIZE - ((long)src & (PAGE_SIZE - 1)), max);
+		max = min(PAGE_SIZE - ((unsigned long)src & (PAGE_SIZE -1)), max);

So there could be some issues here with type conversions at least in theory.

But in any case you're now making the logic inconsistent (the next line uses
bitwise operations directly).

So I'd rather we didn't make this change.

>  #else
>  	/* If src or dest is unaligned, don't do word-at-a-time. */
>  	if (((long) dest | (long) src) & (sizeof(long) - 1))

I'm generally not hugely fond of this 'do a sample case to demonstrate that mm.h
doesn't need to be imported' approach.

Either make the change properly and consistently, or not at all.

Anyway, I'm fine with you just sending a v3 as a single patch moving
offset_in_page() to vdso/page.h then you can defer the use cases if you like?

Cheers, Lorenzo

Re: [PATCH v2 0/2] mm/vdso: make offset_in_page() usable without linux/mm.h

[Lorenzo Stoakes]

On Thu, May 21, 2026 at 11:06:56AM +0200, Thorsten Blum wrote:
> offset_in_page() is a small page-arithmetic helper that has been around
> for 20+ years. However, page-offset calculations are still open-coded in
> many places and in different ways:
>
> 	(unsigned long)p & ~PAGE_MASK
> 	(unsigned long)p & (PAGE_SIZE - 1)
> 	(long)p & (PAGE_SIZE - 1)
> 	...
>
> Some of these open-coded instances may be due to offset_in_page() being
> buried 3000+ lines into linux/mm.h; others may have avoided including
> linux/mm.h, which is a large header that pulls in many others.

This is good but I think you're still missing the 'why' here (as to why files
don't import mm.h, and thus what this series addreses), which presumably, is
compile time.

>
> Patch 1 moves offset_in_page() from linux/mm.h to vdso/page.h, which
> keeps the helper with other low-level page definitions and allows users
> that only need offset_in_page() to avoid including linux/mm.h.
>
> Patch 2 shows a concrete example where including vdso/page.h is
> sufficient and including the large linux/mm.h would be unnecessary.
>
> Existing users of offset_in_page() do not need to change because patch 1
> also includes vdso/page.h from linux/mm.h.
>
> This series is based on akpm/mm.git mm-nonmm-unstable commit
> d067a83c8063 ("string: use min in sized_strscpy"), which is also in
> linux-next.
>
> Changes in v2:
> - Add a cover letter and drop the bytes_to_page_end() helper
> - Move offset_in_page() to vdso/page.h as suggested by David and Lorenzo
> - Use offset_in_page() in lib/string.c as an example since
>   bitmap_print_to_pagebuf() in lib/bitmap-str.c is being removed [1]
> - v1: https://lore.kernel.org/lkml/20260517123428.1181981-4-thorsten.blum@linux.dev/
>
> [1] https://lore.kernel.org/lkml/20260519163058.953690-3-ynorov@nvidia.com/
> ---
> Thorsten Blum (2):
>   vdso: move offset_in_page() from linux/mm.h to vdso/page.h
>   string: use offset_in_page() in sized_strscpy()
>
>  include/linux/mm.h  | 2 +-
>  include/vdso/page.h | 2 ++
>  lib/string.c        | 3 ++-
>  3 files changed, 5 insertions(+), 2 deletions(-)
>
>
> base-commit: d067a83c8063d1bdcbd9af8e1326d846f85138b8

Re: [PATCH v2 1/2] vdso: move offset_in_page() from linux/mm.h to vdso/page.h

[Thomas Gleixner]

On Thu, May 21 2026 at 10:30, David Laight wrote:
> On Thu, 21 May 2026 11:06:57 +0200
> Thorsten Blum <thorsten.blum@linux.dev> wrote:
>> +#include <vdso/page.h>
>
> That is already picked up otherwise PAGE_MASK would be undefined.
>
> The whole vdso headers is a mess.
> I moved one of the definitions (to do with bitmasks) back into its main header.
> The vdso compiled fine because all the 'normal' kernel headers get included
> in vdso builds.

Which is just wrong and we had subtle issues due to that in the
past.

VDSO is a pure user space library and just because it compiles does not
mean it is correct.

Thanks,

        tglx

Re: [PATCH v2 2/2] string: use offset_in_page() in sized_strscpy()

[Thorsten Blum]

On Thu, May 21, 2026 at 12:31:25PM +0100, Lorenzo Stoakes wrote:
> On Thu, May 21, 2026 at 11:06:58AM +0200, Thorsten Blum wrote:
> > Replace the open-coded implementation with offset_in_page() to simplify
> > sized_strscpy().
> >
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> 
> I feel this patch actually makes sized_strscpy() more difficult to understand
> unfortunately, so not really in favour of us taking it.
> 
> > ---
> >  lib/string.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/string.c b/lib/string.c
> > index 1f9297e9776a..7c72adc7377c 100644
> > --- a/lib/string.c
> > +++ b/lib/string.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/stddef.h>
> >  #include <linux/string.h>
> >  #include <linux/types.h>
> > +#include <vdso/page.h>
> >
> >  #include <asm/page.h>
> >  #include <asm/rwonce.h>
> > @@ -127,7 +128,7 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count)
> >  	 * since we don't know if the next page is mapped.
> >  	 */
> >  	if ((long)src & (sizeof(long) - 1))
> > -		max = min(PAGE_SIZE - ((long)src & (PAGE_SIZE - 1)), max);
> > +		max = min(PAGE_SIZE - offset_in_page(src), max);
> 
> This isn't strictly the same code, offset_in_page() is:
> 
> #define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
> 
> So this is now, effectively:
> 
> -		max = min(PAGE_SIZE - ((long)src & (PAGE_SIZE - 1)), max);
> +		max = min(PAGE_SIZE - ((unsigned long)src & (PAGE_SIZE -1)), max);
> 
> So there could be some issues here with type conversions at least in theory.

I think this should have used unsigned long from the start, as long
seems a bit odd for a pointer, and PAGE_SIZE - offset_in_page() is a
common kernel idiom for this exact calculation.

Andrew also commented [1] on this line recently.

> But in any case you're now making the logic inconsistent (the next line uses
> bitwise operations directly).

Agreed, maybe there are other helpers we can use here too, but that
should probably be a follow-up patch.

> So I'd rather we didn't make this change.

FWIW, using offset_in_page() in lib/string.c (as suggested by Andy [2])
was the initial motivation for moving it out of mm.h, so I'd prefer to
keep the use-site in this series if possible.

Thanks,
Thorsten

[1] https://lore.kernel.org/lkml/20260519123740.458d905f958f39d41cb130fd@linux-foundation.org/
[2] https://lore.kernel.org/lkml/CAHp75VfQNkqEYsO4Uup0c-uiYuVyAWit=tmCz2BsYLp-sjXsZw@mail.gmail.com/

Re: [PATCH v2 0/2] mm/vdso: make offset_in_page() usable without linux/mm.h

[Yury Norov]

On Thu, May 21, 2026 at 11:06:56AM +0200, Thorsten Blum wrote:
> offset_in_page() is a small page-arithmetic helper that has been around
> for 20+ years. However, page-offset calculations are still open-coded in
> many places and in different ways:
> 
> 	(unsigned long)p & ~PAGE_MASK
> 	(unsigned long)p & (PAGE_SIZE - 1)
> 	(long)p & (PAGE_SIZE - 1)
> 	...
> 
> Some of these open-coded instances may be due to offset_in_page() being
> buried 3000+ lines into linux/mm.h; others may have avoided including
> linux/mm.h, which is a large header that pulls in many others.
> 
> Patch 1 moves offset_in_page() from linux/mm.h to vdso/page.h, which
> keeps the helper with other low-level page definitions and allows users
> that only need offset_in_page() to avoid including linux/mm.h.
> 
> Patch 2 shows a concrete example where including vdso/page.h is
> sufficient and including the large linux/mm.h would be unnecessary.

mm.h is already included there indirectly, otherwise it would be a
compile error, isn't?

--

Example for who? Are you expecting somebody to pick it up from you and
cleanup the whole kernel?

I'm not interested in 'example', I'm interested in a broad picture and
an estimate of how realistically would it be to switch kernel to
consistently use the helper.

So this is the broad picture for you:

 - 660 uses of offset_in_page()
 - 570 uses of & ~PAGE_MASK
 - 176 uses of & (PAGE_SIZE - 1)
 - 135 high-confidence candidates

Those 135 candidates are found with:

$ git grep -n -E \
  '\b(offset|off|ofs|poff|pg_off|pg_ofs|page_offset|page_offs|start_offset|dest_off|src_off|from|to|remainder)\b[^=]*= [^;]*&[[:space:]]*(~PAGE_MASK|\(?PAGE_SIZE[[:space:]]*-[[:space:]]*1\)?)' \
  -- ':(exclude)drivers/gpu/drm/amd/include/asic_reg/**' \
  ':(exclude)include/trace/events/**'

If you want to go ahead and convert at least those good candidates -
it would be about a couple dozens of patches. And it would look like a
meaningful commitment.

If you want to move the macro out of linux/mm.h and drop the header
from some inclusion paths - it would be another meaningful commitment.

In this series you move the macro to a questionable vdso header, and
convert just one user, which leads to *adding* another dependency,
instead of removing some. This doesn't look like a valuable commitment,
sorry.

Thanks,
Yury
 
> Existing users of offset_in_page() do not need to change because patch 1
> also includes vdso/page.h from linux/mm.h.
> 
> This series is based on akpm/mm.git mm-nonmm-unstable commit
> d067a83c8063 ("string: use min in sized_strscpy"), which is also in
> linux-next.
> 
> Changes in v2:
> - Add a cover letter and drop the bytes_to_page_end() helper
> - Move offset_in_page() to vdso/page.h as suggested by David and Lorenzo
> - Use offset_in_page() in lib/string.c as an example since
>   bitmap_print_to_pagebuf() in lib/bitmap-str.c is being removed [1]
> - v1: https://lore.kernel.org/lkml/20260517123428.1181981-4-thorsten.blum@linux.dev/
> 
> [1] https://lore.kernel.org/lkml/20260519163058.953690-3-ynorov@nvidia.com/
> ---
> Thorsten Blum (2):
>   vdso: move offset_in_page() from linux/mm.h to vdso/page.h
>   string: use offset_in_page() in sized_strscpy()
> 
>  include/linux/mm.h  | 2 +-
>  include/vdso/page.h | 2 ++
>  lib/string.c        | 3 ++-
>  3 files changed, 5 insertions(+), 2 deletions(-)
> 
> 
> base-commit: d067a83c8063d1bdcbd9af8e1326d846f85138b8

Re: [PATCH v2 1/2] vdso: move offset_in_page() from linux/mm.h to vdso/page.h

[Yury Norov]

On Thu, May 21, 2026 at 11:06:57AM +0200, Thorsten Blum wrote:
> Move offset_in_page() out of linux/mm.h so users that only need page
> offset calculations can include the lightweight vdso/page.h header
> instead of pulling in the large linux/mm.h.
> 
> Include vdso/page.h from linux/mm.h so existing users of
> offset_in_page() continue to build.

lib/string.c doesn't look like a vdso, and AFAIK there's no vdso
candidates for using the macro. So that please don't bloat the header.
 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  include/linux/mm.h  | 2 +-
>  include/vdso/page.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 9cedc5e75aa9..d5c7f1ca80ef 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -37,6 +37,7 @@
>  #include <linux/bitmap.h>
>  #include <linux/bitops.h>
>  #include <linux/iommu-debug-pagealloc.h>
> +#include <vdso/page.h>
>  
>  struct mempolicy;
>  struct anon_vma;
> @@ -3023,7 +3024,6 @@ static inline void clear_page_pfmemalloc(struct page *page)
>   */
>  extern void pagefault_out_of_memory(void);
>  
> -#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
>  #define offset_in_folio(folio, p) ((unsigned long)(p) & (folio_size(folio) - 1))
>  
>  /*
> diff --git a/include/vdso/page.h b/include/vdso/page.h
> index bc47186c07fc..948b13091084 100644
> --- a/include/vdso/page.h
> +++ b/include/vdso/page.h
> @@ -28,4 +28,6 @@
>  #define PAGE_MASK	(~(PAGE_SIZE - 1))
>  #endif
>  
> +#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
> +
>  #endif	/* __VDSO_PAGE_H */

Re: [PATCH v2 2/2] string: use offset_in_page() in sized_strscpy()

[Lorenzo Stoakes]

NAK.

I guess I wasn't clear before :)

On Thu, May 21, 2026 at 03:47:25PM +0200, Thorsten Blum wrote:
> On Thu, May 21, 2026 at 12:31:25PM +0100, Lorenzo Stoakes wrote:
> > On Thu, May 21, 2026 at 11:06:58AM +0200, Thorsten Blum wrote:
> > > Replace the open-coded implementation with offset_in_page() to simplify
> > > sized_strscpy().
> > >
> > > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> >
> > I feel this patch actually makes sized_strscpy() more difficult to understand
> > unfortunately, so not really in favour of us taking it.
> >
> > > ---
> > >  lib/string.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/string.c b/lib/string.c
> > > index 1f9297e9776a..7c72adc7377c 100644
> > > --- a/lib/string.c
> > > +++ b/lib/string.c
> > > @@ -25,6 +25,7 @@
> > >  #include <linux/stddef.h>
> > >  #include <linux/string.h>
> > >  #include <linux/types.h>
> > > +#include <vdso/page.h>
> > >
> > >  #include <asm/page.h>
> > >  #include <asm/rwonce.h>
> > > @@ -127,7 +128,7 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count)
> > >  	 * since we don't know if the next page is mapped.
> > >  	 */
> > >  	if ((long)src & (sizeof(long) - 1))
> > > -		max = min(PAGE_SIZE - ((long)src & (PAGE_SIZE - 1)), max);
> > > +		max = min(PAGE_SIZE - offset_in_page(src), max);
> >
> > This isn't strictly the same code, offset_in_page() is:
> >
> > #define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
> >
> > So this is now, effectively:
> >
> > -		max = min(PAGE_SIZE - ((long)src & (PAGE_SIZE - 1)), max);
> > +		max = min(PAGE_SIZE - ((unsigned long)src & (PAGE_SIZE -1)), max);
> >
> > So there could be some issues here with type conversions at least in theory.
>
> I think this should have used unsigned long from the start, as long
> seems a bit odd for a pointer, and PAGE_SIZE - offset_in_page() is a
> common kernel idiom for this exact calculation.
>
> Andrew also commented [1] on this line recently.

Yup, but it makes no sense as a singular change, it actively worsens the
code.

Hypotheticals about what should or should not be with you not changing
anything isn't useful.

>
> > But in any case you're now making the logic inconsistent (the next line uses
> > bitwise operations directly).
>
> Agreed, maybe there are other helpers we can use here too, but that
> should probably be a follow-up patch.

No, this patch is bad, we're not taking it.

>
> > So I'd rather we didn't make this change.
>
> FWIW, using offset_in_page() in lib/string.c (as suggested by Andy [2])
> was the initial motivation for moving it out of mm.h, so I'd prefer to
> keep the use-site in this series if possible.

Nope.

>
> Thanks,
> Thorsten
>
> [1] https://lore.kernel.org/lkml/20260519123740.458d905f958f39d41cb130fd@linux-foundation.org/
> [2] https://lore.kernel.org/lkml/CAHp75VfQNkqEYsO4Uup0c-uiYuVyAWit=tmCz2BsYLp-sjXsZw@mail.gmail.com/

Thanks, Lorenzo

Re: [PATCH v2 1/2] vdso: move offset_in_page() from linux/mm.h to vdso/page.h

[Lorenzo Stoakes]

On Thu, May 21, 2026 at 12:16:00PM +0100, Lorenzo Stoakes wrote:
> On Thu, May 21, 2026 at 11:06:57AM +0200, Thorsten Blum wrote:
> > Move offset_in_page() out of linux/mm.h so users that only need page
> > offset calculations can include the lightweight vdso/page.h header
> > instead of pulling in the large linux/mm.h.
> >
> > Include vdso/page.h from linux/mm.h so existing users of
> > offset_in_page() continue to build.
> >
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>
> LGTM other than dropping the include below (maybe Andrew can do it for
> us?), so:
>
> Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>

TBH I'd like to withdraw this at this point.

Unless a series can be put forwards that sensibly justifies this, not some
random change somewhere, I'd rather we not take it.

Also vdso/page.h is a VDSO-specific header by MAINTAINERS, offset_in_page() is
really an mm thing so that's another reason not to move it.

(A justifying change would show actually build time/binary bloat/etc. numbers +
involve actual substantive changes).

Thanks, Lorenzo

>
> > ---
> >  include/linux/mm.h  | 2 +-
> >  include/vdso/page.h | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 9cedc5e75aa9..d5c7f1ca80ef 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -37,6 +37,7 @@
> >  #include <linux/bitmap.h>
> >  #include <linux/bitops.h>
> >  #include <linux/iommu-debug-pagealloc.h>
> > +#include <vdso/page.h>
>
> Yeah as per David Laight, we don't need this.
>
> >
> >  struct mempolicy;
> >  struct anon_vma;
> > @@ -3023,7 +3024,6 @@ static inline void clear_page_pfmemalloc(struct page *page)
> >   */
> >  extern void pagefault_out_of_memory(void);
> >
> > -#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
> >  #define offset_in_folio(folio, p) ((unsigned long)(p) & (folio_size(folio) - 1))
> >
> >  /*
> > diff --git a/include/vdso/page.h b/include/vdso/page.h
> > index bc47186c07fc..948b13091084 100644
> > --- a/include/vdso/page.h
> > +++ b/include/vdso/page.h
> > @@ -28,4 +28,6 @@
> >  #define PAGE_MASK	(~(PAGE_SIZE - 1))
> >  #endif
> >
> > +#define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
> > +
> >  #endif	/* __VDSO_PAGE_H */

Re: [PATCH v2 1/2] vdso: move offset_in_page() from linux/mm.h to vdso/page.h

[Andy Shevchenko]

On Thu, May 21, 2026 at 4:56 PM Lorenzo Stoakes <ljs@kernel.org> wrote:
> On Thu, May 21, 2026 at 12:16:00PM +0100, Lorenzo Stoakes wrote:
> > On Thu, May 21, 2026 at 11:06:57AM +0200, Thorsten Blum wrote:

...

> Unless a series can be put forwards that sensibly justifies this, not some
> random change somewhere, I'd rather we not take it.

The problem is not only a compile time but more generic - the mess in
the Linux headers. People who want to use this match need to include
mm.h which is a bloated header with *tons* of unneeded dependencies
(for those pure users) and increases chaos in how the headers are
split.
If we don't care about it, then provide an "INCLUDE EVERYTHING" header
and let the driver just include that. But of course, we don't do that,
we do modular headers for a reason. Currently I see no reason to have
that simple helper to be mm.h as its use is much wider in the cases
where the whole hell of mm.h is not needed.

So, the best course of actions I see now is leave mm people alone and
simply duplicate what we need in the headers we want to see
(util_macros.h as a rough approximation, but maybe something better).


> Also vdso/page.h is a VDSO-specific header by MAINTAINERS, offset_in_page() is
> really an mm thing so that's another reason not to move it.
>
> (A justifying change would show actually build time/binary bloat/etc. numbers +
> involve actual substantive changes).



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] vdso: move offset_in_page() from linux/mm.h to vdso/page.h

[Lorenzo Stoakes]

On Thu, May 21, 2026 at 09:34:57PM +0300, Andy Shevchenko wrote:
> On Thu, May 21, 2026 at 4:56 PM Lorenzo Stoakes <ljs@kernel.org> wrote:
> > On Thu, May 21, 2026 at 12:16:00PM +0100, Lorenzo Stoakes wrote:
> > > On Thu, May 21, 2026 at 11:06:57AM +0200, Thorsten Blum wrote:
>
> ...
>
> > Unless a series can be put forwards that sensibly justifies this, not some
> > random change somewhere, I'd rather we not take it.
>
> The problem is not only a compile time but more generic - the mess in
> the Linux headers. People who want to use this match need to include
> mm.h which is a bloated header with *tons* of unneeded dependencies
> (for those pure users) and increases chaos in how the headers are
> split.

No question the headers are a bit of a mess, and I've had absolute headaches
with dependency chain stuff there.

> If we don't care about it, then provide an "INCLUDE EVERYTHING" header

That's a false dichotomy - the series isn't upstreamable as-is. I've provided
feedback that's not been addressed so we won't take it.

> and let the driver just include that. But of course, we don't do that,
> we do modular headers for a reason. Currently I see no reason to have
> that simple helper to be mm.h as its use is much wider in the cases
> where the whole hell of mm.h is not needed.

We also don't want to have wildly divergent overly-specific individual headers,
and series have to be upstreamable.

>
> So, the best course of actions I see now is leave mm people alone and
> simply duplicate what we need in the headers we want to see
> (util_macros.h as a rough approximation, but maybe something better).

I mean I feel engaging civilly and constructively with people who have taken
their time out to provide feedback is a better approach, but obviously what you
do elsewhere in the kernel is up to you.

>
>
> > Also vdso/page.h is a VDSO-specific header by MAINTAINERS, offset_in_page() is
> > really an mm thing so that's another reason not to move it.
> >
> > (A justifying change would show actually build time/binary bloat/etc. numbers +
> > involve actual substantive changes).
>
>
>
> --
> With Best Regards,
> Andy Shevchenko

Overall this all feels like a storm in a teapot, this is a very trivial macro
and the use case seems nebulous at best.

A series actually addressing header issues with _a justifying reason provided_
(I am astonished that it still hasn't!), would be more useful, as long as it was
upstreamable and didn't cause other issues.

We in mm are friendly and accepting of good series, engaging positively will get
a positive reception :)

Thanks, Lorenzo

Re: [PATCH v2 1/2] vdso: move offset_in_page() from linux/mm.h to vdso/page.h

[Thorsten Blum]

On Thu, May 21, 2026 at 03:56:19PM +0100, Lorenzo Stoakes wrote:
> On Thu, May 21, 2026 at 12:16:00PM +0100, Lorenzo Stoakes wrote:
> > On Thu, May 21, 2026 at 11:06:57AM +0200, Thorsten Blum wrote:
> > > Move offset_in_page() out of linux/mm.h so users that only need page
> > > offset calculations can include the lightweight vdso/page.h header
> > > instead of pulling in the large linux/mm.h.
> > >
> > > Include vdso/page.h from linux/mm.h so existing users of
> > > offset_in_page() continue to build.
> > >
> > > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> >
> > LGTM other than dropping the include below (maybe Andrew can do it for
> > us?), so:
> >
> > Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
> 
> TBH I'd like to withdraw this at this point.

OK

> Unless a series can be put forwards that sensibly justifies this, not some
> random change somewhere, I'd rather we not take it.

Using lib/bitmap-str.c as an example was apparently fine, but since that
is being removed, converting lib/string.c instead to use the same
idiomatic PAGE_SIZE - offset_in_page() calculation and include a
lightweight header is a "random change somewhere"?

> Also vdso/page.h is a VDSO-specific header by MAINTAINERS, offset_in_page() is
> really an mm thing so that's another reason not to move it.

Adding a new header was rejected (ok), but moving offset_in_page() to
vdso/page.h was your and David's suggestion, not mine.

Let's just drop this series as I don't think this is going anywhere.

Thanks,
Thorsten