Message-ID: <87o77w2nrw.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date: Thu, 20 Jun 2024 13:20:35 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Barry Song <21cnbao@...il.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

Barry Song <21cnbao@...il.com> writes:

> 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.

Is there any user visual functionality issue?

>>
>> > 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.

Repeating sequential anonymous pages writing is the best case.

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

IMHO, we don't really need this special purpose test.  We can have some
more general basic tests, for example, sequential anonymous pages
writing/reading, random anonymous pages writing/reading, and combination
of them.

--
Best Regards,
Huang, Ying

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

Powered by blists - more mailing lists