Forum | Documentation | Website | Blog

Skip to content
Snippets Groups Projects
  1. Jul 18, 2024
    • Vlastimil Babka's avatar
      mm, slab: put should_failslab() back behind CONFIG_SHOULD_FAILSLAB · a7526fe8
      Vlastimil Babka authored
      Patch series "revert unconditional slab and page allocator fault injection
      calls".
      
      These two patches largely revert commits that added function call overhead
      into slab and page allocation hotpaths and that cannot be currently
      disabled even though related CONFIG_ options do exist.
      
      A much more involved solution that can keep the callsites always existing
      but hidden behind a static key if unused, is possible [1] and can be
      pursued by anyone who believes it's necessary.  Meanwhile the fact the
      should_failslab() error injection is already not functional on kernels
      built with current gcc without anyone noticing [2], and lukewarm response
      to [1] suggests the need is not there.  I believe it will be more fair to
      have the state after this series as a baseline for possible further
      optimisation, instead of the unconditional overhead.
      
      For example a possible compromise for anyone who's fine with an empty
      function call overhead but not the full CONFIG_FAILSLAB /
      CONFIG_FAIL_PAGE_ALLOC overhead is to reuse patch 1 from [1] but insert a
      static key check only inside should_failslab() and
      should_fail_alloc_page() before performing the more expensive checks.
      
      [1] https://lore.kernel.org/all/20240620-fault-injection-statickeys-v2-0-e23947d3d84b@suse.cz/#t
      [2] https://github.com/bpftrace/bpftrace/issues/3258
      
      
      This patch (of 2):
      
      This mostly reverts commit 4f6923fb ("mm: make should_failslab always
      available for fault injection").  The commit made should_failslab() a
      noinline function that's always called from the slab allocation hotpath,
      even if it's empty because CONFIG_SHOULD_FAILSLAB is not enabled, and
      there is no option to disable that call.  This is visible in profiles and
      the function call overhead can be noticeable especially with cpu
      mitigations.
      
      Meanwhile the bpftrace program example in the commit silently does not
      work without CONFIG_SHOULD_FAILSLAB anyway with a recent gcc, because the
      empty function gets a .constprop clone that is actually being called
      (uselessly) from the slab hotpath, while the error injection is hooked to
      the original function that's not being called at all [1].
      
      Thus put the whole should_failslab() function back behind
      CONFIG_SHOULD_FAILSLAB.  It's not a complete revert of 4f6923fb - the
      int return type that returns -ENOMEM on failure is preserved, as well
      ALLOW_ERROR_INJECTION annotation.  The BTF_ID() record that was meanwhile
      added is also guarded by CONFIG_SHOULD_FAILSLAB.
      
      [1] https://github.com/bpftrace/bpftrace/issues/3258
      
      Link: https://lkml.kernel.org/r/20240711-b4-fault-injection-reverts-v1-0-9e2651945d68@suse.cz
      Link: https://lkml.kernel.org/r/20240711-b4-fault-injection-reverts-v1-1-9e2651945d68@suse.cz
      
      
      Signed-off-by: default avatarVlastimil Babka <vbabka@suse.cz>
      Cc: Akinobu Mita <akinobu.mita@gmail.com>
      Cc: Alexei Starovoitov <ast@kernel.org>
      Cc: Andrii Nakryiko <andrii@kernel.org>
      Cc: Christoph Lameter <cl@linux.com>
      Cc: Daniel Borkmann <daniel@iogearbox.net>
      Cc: David Rientjes <rientjes@google.com>
      Cc: Eduard Zingerman <eddyz87@gmail.com>
      Cc: Hao Luo <haoluo@google.com>
      Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: John Fastabend <john.fastabend@gmail.com>
      Cc: KP Singh <kpsingh@kernel.org>
      Cc: Martin KaFai Lau <martin.lau@linux.dev>
      Cc: Mateusz Guzik <mjguzik@gmail.com>
      Cc: Roman Gushchin <roman.gushchin@linux.dev>
      Cc: Song Liu <song@kernel.org>
      Cc: Stanislav Fomichev <sdf@fomichev.me>
      Cc: Yonghong Song <yonghong.song@linux.dev>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      a7526fe8
    • Pei Li's avatar
      mm: ignore data-race in __swap_writepage · 7b7aca6d
      Pei Li authored
      Syzbot reported a possible data race:
      
      BUG: KCSAN: data-race in __swap_writepage / scan_swap_map_slots
      
      read-write to 0xffff888102fca610 of 8 bytes by task 7106 on cpu 1.
      read to 0xffff888102fca610 of 8 bytes by task 7080 on cpu 0.
      
      While we are in __swap_writepage to read sis->flags, scan_swap_map_slots
      is trying to update it with SWP_SCANNING.
      
      value changed: 0x0000000000008083 -> 0x0000000000004083.
      
      While this can be updated non-atomicially, this won't affect
      SWP_SYNCHRONOUS_IO, so we consider this data-race safe.
      
      This is possibly introduced by commit 3222d8c2 ("block: remove
      ->rw_page"), where this if branch is introduced.
      
      Link: https://lkml.kernel.org/r/20240711-bug13-v1-1-cea2b8ae8d76@gmail.com
      Fixes: 3222d8c2
      
       ("block: remove ->rw_page")
      Signed-off-by: default avatarPei Li <peili.dev@gmail.com>
      Reported-by: default avatar <syzbot+da25887cc13da6bf3b8c@syzkaller.appspotmail.com>
      Closes: https://syzkaller.appspot.com/bug?extid=da25887cc13da6bf3b8c
      
      
      Cc: Dan Williams <dan.j.williams@intel.com>
      Cc: Shuah Khan <skhan@linuxfoundation.org>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      7b7aca6d
    • Donet Tom's avatar
      hugetlbfs: ensure generic_hugetlb_get_unmapped_area() returns higher address than mmap_min_addr · dffe24e9
      Donet Tom authored
      generic_hugetlb_get_unmapped_area() was returning an address less than
      mmap_min_addr if the mmap argument addr, after alignment, was less than
      mmap_min_addr, causing mmap to fail.
      
      This is because current generic_hugetlb_get_unmapped_area() code does not
      take into account mmap_min_addr.
      
      This patch ensures that generic_hugetlb_get_unmapped_area() always returns
      an address that is greater than mmap_min_addr.  Additionally, similar to
      generic_get_unmapped_area(), vm_end_gap() checks are included to maintain
      stack gap.
      
      How to reproduce
      ================
      
       #include <stdio.h>
       #include <stdlib.h>
       #include <sys/mman.h>
       #include <unistd.h>
      
       #define HUGEPAGE_SIZE (16 * 1024 * 1024)
      
       int main() {
      
          void *addr = mmap((void *)-1, HUGEPAGE_SIZE,
                       PROT_READ | PROT_WRITE,
                       MAP_SHARED | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
          if (addr == MAP_FAILED) {
              perror("mmap");
              exi...
      dffe24e9
  2. Jul 12, 2024