On 2024/6/26 7:57, Jiaqi Yan wrote: > On Tue, Jun 25, 2024 at 12:05 AM Miaohe Lin <linmiaohe@xxxxxxxxxx> wrote: >> >> On 2024/6/25 0:33, Jiaqi Yan wrote: >>> Add regression and new tests when hugepage has correctable memory >> ... >>> diff --git a/tools/testing/selftests/mm/hugetlb-soft-offline.c b/tools/testing/selftests/mm/hugetlb-soft-offline.c >>> new file mode 100644 >>> index 000000000000..16fe52f972e2 >>> --- /dev/null >>> +++ b/tools/testing/selftests/mm/hugetlb-soft-offline.c >>> @@ -0,0 +1,227 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * Test soft offline behavior for HugeTLB pages: >>> + * - if enable_soft_offline = 0, hugepages should stay intact and soft >>> + * offlining failed with EINVAL. >> >> s/failed with EINVAL/failed with EOPNOTSUPP/g > > To be fixed in v6. > >> >>> + * - if enable_soft_offline = 1, a hugepage should be dissolved and >>> + * nr_hugepages/free_hugepages should be reduced by 1. >>> + * >>> + * Before running, make sure more than 2 hugepages of default_hugepagesz >>> + * are allocated. For example, if /proc/meminfo/Hugepagesize is 2048kB: >>> + * echo 8 > /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages >>> + */ >>> + >> ... >>> +static void test_soft_offline_common(int enable_soft_offline) >>> +{ >>> + int fd; >>> + int expect_errno = enable_soft_offline ? 0 : EOPNOTSUPP; >>> + struct statfs file_stat; >>> + unsigned long hugepagesize_kb = 0; >>> + unsigned long nr_hugepages_before = 0; >>> + unsigned long nr_hugepages_after = 0; >>> + int ret; >>> + >>> + ksft_print_msg("Test soft-offline when enabled_soft_offline=%d\n", >>> + enable_soft_offline); >>> + >>> + fd = create_hugetlbfs_file(&file_stat); >>> + if (fd < 0) { >>> + ksft_exit_fail_msg("Failed to create hugetlbfs file\n"); >>> + return; >>> + } >>> + >>> + hugepagesize_kb = file_stat.f_bsize / 1024; >>> + ksft_print_msg("Hugepagesize is %ldkB\n", hugepagesize_kb); >>> + >>> + if (set_enable_soft_offline(enable_soft_offline)) { >>> + ksft_exit_fail_msg("Failed to set enable_soft_offline\n"); >> >> Call destroy_hugetlbfs_file() in error path? > > As the counterpart of destroy_hugetlbfs_file, I think the test only > needs to close(fd). Will add it in v6. > >> >>> + return; >>> + } >>> + >>> + if (read_nr_hugepages(hugepagesize_kb, &nr_hugepages_before) != 0) { >>> + ksft_exit_fail_msg("Failed to read nr_hugepages\n"); >>> + return; >>> + } >>> + >>> + ksft_print_msg("Before MADV_SOFT_OFFLINE nr_hugepages=%ld\n", >>> + nr_hugepages_before); >>> + >>> + ret = do_soft_offline(fd, 2 * file_stat.f_bsize, expect_errno); >>> + >>> + if (read_nr_hugepages(hugepagesize_kb, &nr_hugepages_after) != 0) { >>> + ksft_exit_fail_msg("Failed to read nr_hugepages\n"); >>> + return; >>> + } >>> + >>> + ksft_print_msg("After MADV_SOFT_OFFLINE nr_hugepages=%ld\n", >>> + nr_hugepages_after); >>> + >>> + if (enable_soft_offline) { >>> + if (nr_hugepages_before != nr_hugepages_after + 1) { >>> + ksft_test_result_fail("MADV_SOFT_OFFLINE should reduced 1 hugepage\n"); >>> + return; >>> + } >>> + } else { >>> + if (nr_hugepages_before != nr_hugepages_after) { >>> + ksft_test_result_fail("MADV_SOFT_OFFLINE reduced %lu hugepages\n", >>> + nr_hugepages_before - nr_hugepages_after); >>> + return; >>> + } >>> + } >>> + >>> + ksft_test_result(ret == 0, >>> + "Test soft-offline when enabled_soft_offline=%d\n", >>> + enable_soft_offline); >> >> Call destroy_hugetlbfs_file() when test finished ? > > Test can just close(fd) once nr_hugepages_after is read. I'm sorry but I can't find the code to call close(fd) after nr_hugepages_after is read. IMO create_hugetlbfs_file() would fail to create a new hugetlb file later if close(fd) is not called when testing previous enable_soft_offline = 1 testcase. Because a hugetlb file with same name is already there. But I might miss something. Thanks. .