Date: Thu, 20 Jun 2024 14:04:03 +1200
From: Barry Song <21cnbao@...il.com>
To: "Huang, Ying" <ying.huang@...el.com>
Cc: akpm@...ux-foundation.org, shuah@...nel.org, linux-mm@...ck.org, 
	ryan.roberts@....com, chrisl@...nel.org, david@...hat.com, hughd@...gle.com, 
	kaleshsingh@...gle.com, kasong@...cent.com, linux-kernel@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, Barry Song <v-songbaohua@...o.com>
Subject: Re: [PATCH] selftests/mm: Introduce a test program to assess swap
 entry allocation for thp_swapout

On Thu, Jun 20, 2024 at 1:55 PM Huang, Ying <ying.huang@...el.com> wrote:
>
> Barry Song <21cnbao@...il.com> writes:
>
> > From: Barry Song <v-songbaohua@...o.com>
> >
> > Both Ryan and Chris have been utilizing the small test program to aid
> > in debugging and identifying issues with swap entry allocation. While
> > a real or intricate workload might be more suitable for assessing the
> > correctness and effectiveness of the swap allocation policy, a small
> > test program presents a simpler means of understanding the problem and
> > initially verifying the improvements being made.
> >
> > Let's endeavor to integrate it into the self-test suite. Although it
> > presently only accommodates 64KB and 4KB, I'm optimistic that we can
> > expand its capabilities to support multiple sizes and simulate more
> > complex systems in the future as required.
>
> IIUC, this is a performance test program instead of functionality test
> program.  Does it match the purpose of the kernel selftest?

I have a differing perspective. I maintain that the functionality is
not functioning
as expected. Despite having all the necessary resources for allocation, failure
persists, indicating a lack of functionality.

>
> > Signed-off-by: Barry Song <v-songbaohua@...o.com>
> > ---
> >  tools/testing/selftests/mm/Makefile           |   1 +
> >  .../selftests/mm/thp_swap_allocator_test.c    | 192 ++++++++++++++++++
> >  2 files changed, 193 insertions(+)
> >  create mode 100644 tools/testing/selftests/mm/thp_swap_allocator_test.c
> >
> > diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> > index e1aa09ddaa3d..64164ad66835 100644
> > --- a/tools/testing/selftests/mm/Makefile
> > +++ b/tools/testing/selftests/mm/Makefile
> > @@ -65,6 +65,7 @@ TEST_GEN_FILES += mseal_test
> >  TEST_GEN_FILES += seal_elf
> >  TEST_GEN_FILES += on-fault-limit
> >  TEST_GEN_FILES += pagemap_ioctl
> > +TEST_GEN_FILES += thp_swap_allocator_test
> >  TEST_GEN_FILES += thuge-gen
> >  TEST_GEN_FILES += transhuge-stress
> >  TEST_GEN_FILES += uffd-stress
> > diff --git a/tools/testing/selftests/mm/thp_swap_allocator_test.c b/tools/testing/selftests/mm/thp_swap_allocator_test.c
> > new file mode 100644
> > index 000000000000..4443a906d0f8
> > --- /dev/null
> > +++ b/tools/testing/selftests/mm/thp_swap_allocator_test.c
> > @@ -0,0 +1,192 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * thp_swap_allocator_test
> > + *
> > + * The purpose of this test program is helping check if THP swpout
> > + * can correctly get swap slots to swap out as a whole instead of
> > + * being split. It randomly releases swap entries through madvise
> > + * DONTNEED and do swapout on two memory areas: a memory area for
> > + * 64KB THP and the other area for small folios. The second memory
> > + * can be enabled by "-s".
> > + * Before running the program, we need to setup a zRAM or similar
> > + * swap device by:
> > + *  echo lzo > /sys/block/zram0/comp_algorithm
> > + *  echo 64M > /sys/block/zram0/disksize
> > + *  echo never > /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/enabled
> > + *  echo always > /sys/kernel/mm/transparent_hugepage/hugepages-64kB/enabled
> > + *  mkswap /dev/zram0
> > + *  swapon /dev/zram0
> > + * The expected result should be 0% anon swpout fallback ratio w/ or
> > + * w/o "-s".
> > + *
> > + * Author(s): Barry Song <v-songbaohua@...o.com>
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <sys/mman.h>
> > +#include <errno.h>
> > +#include <time.h>
> > +
> > +#define MEMSIZE_MTHP (60 * 1024 * 1024)
> > +#define MEMSIZE_SMALLFOLIO (1 * 1024 * 1024)
> > +#define ALIGNMENT_MTHP (64 * 1024)
> > +#define ALIGNMENT_SMALLFOLIO (4 * 1024)
> > +#define TOTAL_DONTNEED_MTHP (16 * 1024 * 1024)
> > +#define TOTAL_DONTNEED_SMALLFOLIO (768 * 1024)
> > +#define MTHP_FOLIO_SIZE (64 * 1024)
> > +
> > +#define SWPOUT_PATH \
> > +     "/sys/kernel/mm/transparent_hugepage/hugepages-64kB/stats/swpout"
> > +#define SWPOUT_FALLBACK_PATH \
> > +     "/sys/kernel/mm/transparent_hugepage/hugepages-64kB/stats/swpout_fallback"
> > +
> > +static void *aligned_alloc_mem(size_t size, size_t alignment)
> > +{
> > +     void *mem = NULL;
> > +
> > +     if (posix_memalign(&mem, alignment, size) != 0) {
> > +             perror("posix_memalign");
> > +             return NULL;
> > +     }
> > +     return mem;
> > +}
> > +
> > +static void random_madvise_dontneed(void *mem, size_t mem_size,
> > +             size_t align_size, size_t total_dontneed_size)
> > +{
> > +     size_t num_pages = total_dontneed_size / align_size;
> > +     size_t i;
> > +     size_t offset;
> > +     void *addr;
> > +
> > +     for (i = 0; i < num_pages; ++i) {
> > +             offset = (rand() % (mem_size / align_size)) * align_size;
> > +             addr = (char *)mem + offset;
> > +             if (madvise(addr, align_size, MADV_DONTNEED) != 0)
> > +                     perror("madvise dontneed");
>
> IIUC, this simulates align_size (generally 64KB) swap-in.  That is, it
> simulate the effect of large size swap-in when it's not available in
> kernel.  If we have large size swap-in in kernel in the future, this
> becomes unnecessary.
>
> Additionally, we have not reached the consensus that we should always
> swap-in with swapped-out size.  So, I suspect that this test may not
> reflect real situation in the future.  Although it doesn't reflect
> current situation too.

Disagree again. releasing the whole mTHP swaps is the best case. Even in
the best-case scenario, if we fail, it raises concerns for handling potentially
more challenging situations.

I don't find it hard to incorporate additional features into this test
program to simulate more intricate scenarios.

>
> > +
> > +             memset(addr, 0x11, align_size);
> > +     }
> > +}
> > +
>
> [snip]
>
> --
> Best Regards,
> Huang, Ying

Thanks
Barry

Powered by blists - more mailing lists