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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.