Re: [PATCH] thp: Disable THP if THP isn't supported by kernel

On Wed, Jul 15, 2026 at 4:10 AM Adhemerval Zanella Netto
<[email protected]> wrote:
>
>
>
> On 04/07/26 00:05, H.J. Lu wrote:
> > Since DL_MAP_DEFAULT_THP_PAGESIZE is defined for x86-64, THP control
> > is to set to madvise by default.  If THP is disabled in x86-64 kernel,
> > madvise (..., MADV_HUGEPAGE) returns -EINVAL to indicate that THP isn't
> > supported.  Add _dl_thp_madvise to disable THP in this case.  This
> > fixes BZ #34348.
> >
> > Signed-off-by: H.J. Lu <[email protected]>
>
> LGTM, two suggestion below.
>
> Reviewed-by: Adhemerval Zanella  <[email protected]>
>
> > ---
> >  sysdeps/unix/sysv/linux/dl-exec-post.h   |  6 ++--
> >  sysdeps/unix/sysv/linux/dl-load-post.h   |  6 ++--
> >  sysdeps/unix/sysv/linux/dl-thp-madvise.h | 35 ++++++++++++++++++++++++
> >  3 files changed, 43 insertions(+), 4 deletions(-)
> >  create mode 100644 sysdeps/unix/sysv/linux/dl-thp-madvise.h
> >
> > diff --git a/sysdeps/unix/sysv/linux/dl-exec-post.h b/sysdeps/unix/sysv/linux/dl-exec-post.h
> > index 9a49486db2..4bc8a50dd1 100644
> > --- a/sysdeps/unix/sysv/linux/dl-exec-post.h
> > +++ b/sysdeps/unix/sysv/linux/dl-exec-post.h
> > @@ -17,6 +17,8 @@
> >     License along with the GNU C Library; if not, see
> >     <https://www.gnu.org/licenses/>.  */
> >
> > +#include "dl-thp-madvise.h"
> > +
> >  static inline void
> >  _dl_get_thp_config (void)
> >  {
> > @@ -116,8 +118,8 @@ _dl_executable_postprocess (struct link_map *main_map,
> >       && ((ph->p_vaddr | ph->p_offset) & (thp_pagesize - 1)) == 0
> >       && (ph->p_flags & (PF_W | PF_R)) == PF_R)
> >        {
> > -     int ret = __madvise ((void *) (main_map->l_addr + ph->p_vaddr),
> > -                          ph->p_memsz, MADV_HUGEPAGE);
> > +     void *addr = (void *) (main_map->l_addr + ph->p_vaddr);
> > +     int ret = _dl_thp_madvise (addr, ph->p_memsz);
> >       if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
> >         _dl_debug_printf ("\
> >  madvise (0x%0*lx, 0x%0*lx, MADV_HUGEPAGE) returns %d\n",
> > diff --git a/sysdeps/unix/sysv/linux/dl-load-post.h b/sysdeps/unix/sysv/linux/dl-load-post.h
> > index 741c4ddcb1..935298bda5 100644
> > --- a/sysdeps/unix/sysv/linux/dl-load-post.h
> > +++ b/sysdeps/unix/sysv/linux/dl-load-post.h
> > @@ -17,6 +17,8 @@
> >     License along with the GNU C Library; if not, see
> >     <https://www.gnu.org/licenses/>.  */
> >
> > +#include "dl-thp-madvise.h"
> > +
> >  static bool _dl_segment_thp_eligible (const struct loadcmd *, size_t);
> >
> >  /* After L has been mapped in, call madvise with MADV_HUGEPAGE for THP
> > @@ -28,8 +30,8 @@ _dl_postprocess_loadcmd_extra (struct link_map *l, const struct loadcmd *c)
> >    if (GL(dl_thp_mode) == thp_mode_madvise
> >        && _dl_segment_thp_eligible (c, GL(dl_elf_thp_pagesize)))
> >      {
> > -      int ret = __madvise ((void *) (l->l_addr + c->mapstart),
> > -                        c->mapend - c->mapstart, MADV_HUGEPAGE);
> > +      int ret = _dl_thp_madvise ((void *) (l->l_addr + c->mapstart),
> > +                              c->mapend - c->mapstart);
> >        if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
> >       _dl_debug_printf ("\
> >    madvise (0x%0*lx, 0x%0*lx, MADV_HUGEPAGE) returns %d\n",
> > diff --git a/sysdeps/unix/sysv/linux/dl-thp-madvise.h b/sysdeps/unix/sysv/linux/dl-thp-madvise.h
> > new file mode 100644
> > index 0000000000..1d54d19987
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/dl-thp-madvise.h
> > @@ -0,0 +1,35 @@
> > +/* _dl_thp_madvise.  Linux version.
> > +   Copyright (C) 2026 Free Software Foundation, Inc.
> > +   Copyright The GNU Toolchain Authors.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
>
> Maybe a guard include here.

Fixed in v2.

> > +
> > +/* Similar to madvise, but disable THP if the madvise syscall returns
> > +   -EINVAL which indicates that THP isn't supported by kernel.  */
> > +
> > +static inline int
> > +_dl_thp_madvise (void *addr, size_t size)
> > +{
> > +  int res = INTERNAL_SYSCALL_CALL (madvise, addr, size, MADV_HUGEPAGE);
> > +  if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))
> > +      && INTERNAL_SYSCALL_ERRNO (res) == EINVAL)
> > +    {
> > +      /* NB: Disable THP if THP isn't supported by kernel.  */
> > +      GL(dl_thp_mode) = thp_mode_not_supported;
> > +      GL(dl_elf_thp_control) = dl_elf_thp_control_disabled;
> > +    }
> > +  return res;
>
> This changes the DL_DEBUG_FILES to print the -errno value insteaf of the
> -1 previously, which is an improvement but a bit confusing.  Maybe return
> INTERNAL_SYSCALL_ERRNO.

Fixed in v2.

> > +}
>

Thanks.

-- 
H.J.