Linux io-uring development
 help / color / mirror / Atom feed
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

  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