Re: [PATCH v5 8/8] mm: huge_memory: enable debugfs to split huge pages to any order.

Zi Yan
2024-03-04T15:57:36Z

[-- Attachment #1.1: Type: text/plain, Size: 4120 bytes --]

On 4 Mar 2024, at 10:44, Aishwarya TCV wrote:

> On 04/03/2024 14:58, Zi Yan wrote:
>> On 4 Mar 2024, at 4:50, Aishwarya TCV wrote:
>>
>>> On 01/03/2024 21:10, Zi Yan wrote:
>>>> On 1 Mar 2024, at 15:02, Zi Yan wrote:
>>>>
>>>>> On 1 Mar 2024, at 14:37, Zi Yan wrote:
>>>>>
>>>>>> On 1 Mar 2024, at 4:51, Aishwarya TCV wrote:
>>>>>>
>>>>>>> On 26/02/2024 20:55, Zi Yan wrote:
>>>>>>>> From: Zi Yan <ziy@nvidia.com>
>>>>>>>>
>>>>>>>> It is used to test split_huge_page_to_list_to_order for pagecache THPs.
>>>>>>>> Also add test cases for split_huge_page_to_list_to_order via both
>>>>>>>> debugfs.
>>>>>>>>
>>>>>>>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>>>>>>>> ---
>>>>>>>> mm/huge_memory.c | 34 ++++--
>>>>>>>> .../selftests/mm/split_huge_page_test.c | 115 +++++++++++++++++-
>>>>>>>> 2 files changed, 131 insertions(+), 18 deletions(-)
>>>>>>>>
>>>>>>>
>>>>>>> Hi Zi,
>>>>>>>
>>>>>>> When booting the kernel against next-master(20240228)with Arm64 on
>>>>>>> Marvell Thunder X2 (TX2), the kselftest-mm test 'split_huge_page_test'
>>>>>>> is failing in our CI (with rootfs over NFS). I can send the full logs if
>>>>>>> required.
>>>>>>>
>>>>>>> A bisect (full log below) identified this patch as introducing the
>>>>>>> failure. Bisected it on the tag "next-20240228" at repo
>>>>>>> "https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git".
>>>>>>>
>>>>>>> This works fine on Linux version 6.8.0-rc6
>>>>>>
>>>>>> Hi Aishwarya,
>>>>>>
>>>>>> Can you try the attached patch and see if it fixes the failure? I changed
>>>>>> the test to accept XFS dev as input, mount XFS on a temp folder under /tmp,
>>>>>> and skip if no XFS is mounted.
>>>>>
>>>>> Please try this updated one. It allows you to specify a XFS device path
>>>>> in SPLIT_HUGE_PAGE_TEST_XFS_PATH env variable, which is passed to
>>>>> split_huge_page_test in run_vmtests.sh. It at least allow CI/CD to run
>>>>> the test without too much change.
>>>>
>>>> OK. This hopefully will be my last churn. Now split_huge_page_test accepts
>>>> a path that is backed by XFS and run_vmtest.sh creates a XFS image in /tmp,
>>>> mounts it in /tmp, and gives the path to split_huge_page_test. I tested
>>>> it locally and it works. Let me know if you have any issue. Thanks.
>>>>
>>>> --
>>>> Best Regards,
>>>> Yan, Zi
>>>
>>> Hi Zi,
>>>
>>> Tested the patch by applying it on next-20240304. Logs from our CI with
>>> rootfs over nfs is attached below. "Bail out! cannot remove tmp dir:
>>> Directory not empty" is still observed.
>>
>> Hi Aishwarya,
>>
>> Do you have the config file for the CI kernel? And /tmp is also on nfs?
>> Any detailed information about CI machine environment? I cannot reproduce
>> the error locally, either on bare metal or VM. Maybe because my /tmp is
>> not NFS mounted?
>>
>
> Hi Zi,
>
> Please find the details below. Hope it helps.
>
> Do you have the config file for the CI kernel?
> - We are using:
> defconfig+https://github.com/torvalds/linux/blob/master/tools/testing/selftests/mm/config
>
> And /tmp is also on nfs?
> - Yes
>
> Any detailed information about CI machine environment?
> - We are running the test using LAVA device Cavium Thunder X2 (TX2),
> - We have very similar rootfs as - nfsrootfs:
> https://storage.kernelci.org/images/rootfs/debian/bullseye-kselftest/20240129.0/arm64/full.rootfs.tar.xz
> - We are using grub boot method over nfs
> - Additionally Ryan mentioned "Looks like it is failing because he is
> trying to delete the temp dir with rmdir() but rmdir() requires the
> directory to be empty, which it is not."

Hi Aishwarya,

Thank you for the information and I am able to reproduce it on a NFS folder.
The error comes from that the opened test files are not munmapped and their
file descriptors are not closed in the skip path. NFS creates .nfsXXX files
for them, making the temp folder not empty.

The attached patch cleans up properly and works on a NFS folder. Let me know
if it works on your side. Thanks.

--
Best Regards,
Yan, Zi

[-- Attachment #1.2: selftest.patch --]
[-- Type: text/plain, Size: 5829 bytes --]

diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index fe140a9f4f9d..ffdec5dc0b03 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -412,7 +412,27 @@ CATEGORY="thp" run_test ./khugepaged -s 2

CATEGORY="thp" run_test ./transhuge-stress -d 20

-CATEGORY="thp" run_test ./split_huge_page_test
+# Try to create XFS if not provided
+if [ -z "${SPLIT_HUGE_PAGE_TEST_XFS_PATH}" ]; then
+ if test_selected "thp"; then
+ if grep xfs /proc/filesystems &>/dev/null; then
+ XFS_IMG=$(mktemp /tmp/xfs_img_XXXXXX)
+ SPLIT_HUGE_PAGE_TEST_XFS_PATH=$(mktemp -d /tmp/xfs_dir_XXXXXX)
+ truncate -s 314572800 ${XFS_IMG}
+ mkfs.xfs -q ${XFS_IMG}
+ mount -o loop ${XFS_IMG} ${SPLIT_HUGE_PAGE_TEST_XFS_PATH}
+ MOUNTED_XFS=1
+ fi
+ fi
+fi
+
+CATEGORY="thp" run_test ./split_huge_page_test ${SPLIT_HUGE_PAGE_TEST_XFS_PATH}
+
+if [ -n "${MOUNTED_XFS}" ]; then
+ umount ${SPLIT_HUGE_PAGE_TEST_XFS_PATH}
+ rmdir ${SPLIT_HUGE_PAGE_TEST_XFS_PATH}
+ rm -f ${XFS_IMG}
+fi

CATEGORY="migration" run_test ./migration

diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
index cf09fdc9ef22..856662d2f87a 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -26,7 +26,6 @@ uint64_t pmd_pagesize;

#define SPLIT_DEBUGFS "/sys/kernel/debug/split_huge_pages"
#define SMAP_PATH "/proc/self/smaps"
-#define THP_FS_PATH "/mnt/thp_fs"
#define INPUT_MAX 80

#define PID_FMT "%d,0x%lx,0x%lx,%d"
@@ -268,7 +267,37 @@ void split_file_backed_thp(void)
ksft_exit_fail_msg("Error occurred\n");
}

-void create_pagecache_thp_and_fd(const char *testfile, size_t fd_size, int *fd, char **addr)
+bool prepare_thp_fs(const char *xfs_path, char *thp_fs_template,
+ const char **thp_fs_loc)
+{
+ if (xfs_path) {
+ *thp_fs_loc = xfs_path;
+ return false;
+ }
+
+ *thp_fs_loc = mkdtemp(thp_fs_template);
+
+ if (!*thp_fs_loc)
+ ksft_exit_fail_msg("cannot create temp folder\n");
+
+ return true;
+}
+
+void cleanup_thp_fs(const char *thp_fs_loc, bool created_tmp)
+{
+ int status;
+
+ if (!created_tmp)
+ return;
+
+ status = rmdir(thp_fs_loc);
+ if (status)
+ ksft_exit_fail_msg("cannot remove tmp dir: %s\n",
+ strerror(errno));
+}
+
+int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size, int *fd,
+ char **addr)
{
size_t i;
int dummy;
@@ -277,7 +306,7 @@ void create_pagecache_thp_and_fd(const char *testfile, size_t fd_size, int *fd,

*fd = open(testfile, O_CREAT | O_RDWR, 0664);
if (*fd == -1)
- ksft_exit_fail_msg("Failed to create a file at "THP_FS_PATH);
+ ksft_exit_fail_msg("Failed to create a file at %s\n", testfile);

for (i = 0; i < fd_size; i++) {
unsigned char byte = (unsigned char)i;
@@ -299,7 +328,7 @@ void create_pagecache_thp_and_fd(const char *testfile, size_t fd_size, int *fd,

*fd = open(testfile, O_RDWR);
if (*fd == -1) {
- ksft_perror("Failed to open a file at "THP_FS_PATH);
+ ksft_perror("Failed to open testfile\n");
goto err_out_unlink;
}

@@ -314,26 +343,39 @@ void create_pagecache_thp_and_fd(const char *testfile, size_t fd_size, int *fd,
dummy += *(*addr + i);

if (!check_huge_file(*addr, fd_size / pmd_pagesize, pmd_pagesize)) {
- ksft_print_msg("No large pagecache folio generated, please mount a filesystem supporting large folio at "THP_FS_PATH"\n");
- goto err_out_close;
+ ksft_print_msg("No large pagecache folio generated, please provide a filesystem supporting large folio\n");
+ munmap(*addr, fd_size);
+ close(*fd);
+ unlink(testfile);
+ ksft_test_result_skip("Pagecache folio split skipped\n");
+ return -2;
}
- return;
+ return 0;
err_out_close:
close(*fd);
err_out_unlink:
unlink(testfile);
ksft_exit_fail_msg("Failed to create large pagecache folios\n");
+ return -1;
}

-void split_thp_in_pagecache_to_order(size_t fd_size, int order)
+void split_thp_in_pagecache_to_order(size_t fd_size, int order, const char *fs_loc)
{
int fd;
char *addr;
size_t i;
- const char testfile[] = THP_FS_PATH "/test";
+ char testfile[INPUT_MAX];
int err = 0;

- create_pagecache_thp_and_fd(testfile, fd_size, &fd, &addr);
+ err = snprintf(testfile, INPUT_MAX, "%s/test", fs_loc);
+
+ if (err < 0)
+ ksft_exit_fail_msg("cannot generate right test file name\n");
+
+ err = create_pagecache_thp_and_fd(testfile, fd_size, &fd, &addr);
+ if (err)
+ return;
+ err = 0;

write_debugfs(PID_FMT, getpid(), (uint64_t)addr, (uint64_t)addr + fd_size, order);

@@ -351,6 +393,7 @@ void split_thp_in_pagecache_to_order(size_t fd_size, int order)
}

out:
+ munmap(addr, fd_size);
close(fd);
unlink(testfile);
if (err)
@@ -362,6 +405,10 @@ int main(int argc, char **argv)
{
int i;
size_t fd_size;
+ char *optional_xfs_path = NULL;
+ char fs_loc_template[] = "/tmp/thp_fs_XXXXXX";
+ const char *fs_loc;
+ bool created_tmp;

ksft_print_header();

@@ -370,6 +417,9 @@ int main(int argc, char **argv)
ksft_finished();
}

+ if (argc > 1)
+ optional_xfs_path = argv[1];
+
ksft_set_plan(3+9);

pagesize = getpagesize();
@@ -384,8 +434,11 @@ int main(int argc, char **argv)
split_pte_mapped_thp();
split_file_backed_thp();

+ created_tmp = prepare_thp_fs(optional_xfs_path, fs_loc_template,
+ &fs_loc);
for (i = 8; i >= 0; i--)
- split_thp_in_pagecache_to_order(fd_size, i);
+ split_thp_in_pagecache_to_order(fd_size, i, fs_loc);
+ cleanup_thp_fs(fs_loc, created_tmp);

ksft_finished();

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]