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>,
Andy Lutomirski <luto@kernel.org>,
Thomas Gleixner <tglx@kernel.org>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
Kees Cook <kees@kernel.org>, Andy Shevchenko <andy@kernel.org>,
Yury Norov <yury.norov@gmail.com>,
David Laight <david.laight.linux@gmail.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] string: use offset_in_page() in sized_strscpy()
Date: Thu, 21 May 2026 12:31:25 +0100 [thread overview]
Message-ID: <ag7pg__Dzs-tKfGw@lucifer> (raw)
In-Reply-To: <20260521090655.160282-6-thorsten.blum@linux.dev>
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
next prev parent reply other threads:[~2026-05-21 11:31 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-21 9:06 [PATCH v2 0/2] mm/vdso: make offset_in_page() usable without linux/mm.h Thorsten Blum
2026-05-21 9:06 ` [PATCH v2 1/2] vdso: move offset_in_page() from linux/mm.h to vdso/page.h Thorsten Blum
2026-05-21 9:30 ` David Laight
2026-05-21 11:45 ` Thomas Gleixner
2026-05-21 11:15 ` Lorenzo Stoakes
2026-05-21 14:56 ` Lorenzo Stoakes
2026-05-21 18:34 ` Andy Shevchenko
2026-05-22 10:52 ` Lorenzo Stoakes
2026-05-22 12:18 ` Thorsten Blum
2026-05-21 14:03 ` Yury Norov
2026-05-21 9:06 ` [PATCH v2 2/2] string: use offset_in_page() in sized_strscpy() Thorsten Blum
2026-05-21 11:31 ` Lorenzo Stoakes [this message]
2026-05-21 13:47 ` Thorsten Blum
2026-05-21 14:53 ` Lorenzo Stoakes
2026-05-21 11:33 ` [PATCH v2 0/2] mm/vdso: make offset_in_page() usable without linux/mm.h Lorenzo Stoakes
2026-05-21 13:59 ` Yury Norov
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=ag7pg__Dzs-tKfGw@lucifer \
--to=ljs@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=andy@kernel.org \
--cc=david.laight.linux@gmail.com \
--cc=david@kernel.org \
--cc=kees@kernel.org \
--cc=liam@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@kernel.org \
--cc=mhocko@suse.com \
--cc=rppt@kernel.org \
--cc=surenb@google.com \
--cc=tglx@kernel.org \
--cc=thorsten.blum@linux.dev \
--cc=vbabka@kernel.org \
--cc=vincenzo.frascino@arm.com \
--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