From: Prateek <kprateek283@gmail.com>
To: gabriel@krisman.be
Cc: io-uring@vger.kernel.org, kprateek283@gmail.com
Subject: Re: [PATCH] setup: dynamically detect default huge page size
Date: Tue, 23 Jun 2026 16:39:30 +0530 [thread overview]
Message-ID: <20260623110930.910263-1-kprateek283@gmail.com> (raw)
In-Reply-To: <87qzlyy0zd.fsf@mailhost.krisman.be>
Hi Gabriel,
Thanks for the review.
On Mon, Jun 22, 2026 at 16:49 Gabriel Krisman Bertazi wrote:
> > +static size_t get_huge_page_size(void)
> > +{
> > + static size_t hps;
>
> Please, initialize your static variables to makes it readable. I.e,
> should be initialized it to 2MB.
hps is left at 0 on purpose as a "not computed yet" flag -- same thing get_page_size() does in arch/aarch64/lib.h with cache_val. If I set hps = 2MB upfront, the first call just returns 2MB without ever reading /proc/meminfo, which defeats the point.
> > + size_t ret = 2 * 1024 * 1024; /* fallback: 2MB */
>
> ret redundant with hps, could go away.
The local ret is there so I only write to hps once at the end. If two threads race into this function, neither one sees a half-baked fallback value in hps. The race itself is harmless since both threads would compute the same result anyway.
> > + if (p + 13 <= end &&
> > + p[0] == 'H' && p[1] == 'u' && p[2] == 'g' &&
> > + p[3] == 'e' && p[4] == 'p' && p[5] == 'a' &&
> > + p[6] == 'g' && p[7] == 'e' && p[8] == 's' &&
> > + p[9] == 'i' && p[10] == 'z' && p[11] == 'e' &&
> > + p[12] == ':') {
>
> This is unreadable. It would be much better as a two line loop
> iterating over two strings... But then, why not create it a couple line
> implementation of memcmp and atoi in arch/generic/lib.h instead?
Yeah, the char-by-char match is ugly, agreed. For v2 I'll add a __uring_memcmp in nolibc.c and shim it in lib.h behind #ifdef CONFIG_NOLIBC, same way memset/malloc/free are done today. arch/generic/lib.h only gets included on archs without nolibc support, so putting memcmp there wouldn't help x86/aarch64/riscv64 nolibc builds. nolibc.c + lib.h shim covers all configs. Then setup.c just calls memcmp(p, "Hugepagesize:", 13) -- normal builds use libc's memcmp, nolibc builds use the shim. I'll keep the digit parsing loop as-is since it's simple enough and pulling in atoi feels like overkill.
> This function should go in arch/generic/lib.h too. A hint is the
> get_page_size is already there.
get_huge_page_size() only lives in setup.c and uses the __sys* wrappers from syscall.h, which work in all build configs. Unlike get_page_size() which is needed across multiple files, there's no reason to put this in the arch headers and duplicate it four times.
> That said, we should be looking into something like the kernel's nolibc
> instead of reinventing libc.
Agreed, worth looking into separately. This patch just fixes the immediate hugepage issue.
Will send a v2 with the memcmp approach.
Thanks,
Prateek
next prev parent reply other threads:[~2026-06-23 11:09 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-20 11:36 [PATCH] setup: dynamically detect default huge page size Prateek
2026-06-22 16:49 ` Gabriel Krisman Bertazi
2026-06-23 11:09 ` Prateek [this message]
2026-06-23 15:11 ` Gabriel Krisman Bertazi
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=20260623110930.910263-1-kprateek283@gmail.com \
--to=kprateek283@gmail.com \
--cc=gabriel@krisman.be \
--cc=io-uring@vger.kernel.org \
/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