| Message ID | 20250122124047.1216024-1-ziy@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] selftests/mm: make file-backed THP split work by setting force option | expand |
On 22.01.25 13:40, Zi Yan wrote: > Commit acd7ccb284b8 ("mm: shmem: add large folio support for tmpfs") > changes huge=always to allocate THP/mTHP based on write size and > split_huge_page_test does not write PMD size data, so file-back THP is not > created during the test. Just curious, why can't we write PMD size data instead, to avoid messing with the "force" option?
On Wed Jan 22, 2025 at 9:26 AM EST, David Hildenbrand wrote: > On 22.01.25 13:40, Zi Yan wrote: >> Commit acd7ccb284b8 ("mm: shmem: add large folio support for tmpfs") >> changes huge=always to allocate THP/mTHP based on write size and >> split_huge_page_test does not write PMD size data, so file-back THP is not >> created during the test. > > Just curious, why can't we write PMD size data instead, to avoid messing > with the "force" option? It also works. I used "force", because I notice that it is intended for testing. Using it might be more future proof, in case huge=always changes its semantics again in the future.
On 22.01.25 16:16, Zi Yan wrote: > On Wed Jan 22, 2025 at 9:26 AM EST, David Hildenbrand wrote: >> On 22.01.25 13:40, Zi Yan wrote: >>> Commit acd7ccb284b8 ("mm: shmem: add large folio support for tmpfs") >>> changes huge=always to allocate THP/mTHP based on write size and >>> split_huge_page_test does not write PMD size data, so file-back THP is not >>> created during the test. >> >> Just curious, why can't we write PMD size data instead, to avoid messing >> with the "force" option? > > It also works. I used "force", because I notice that it is intended for > testing. Using it might be more future proof, in case huge=always changes > its semantics again in the future. I recall discussing with Hugh in an upstream call that "force" is a relict from older times, so naturally I would have just adjusted the test case to trigger the PMD scenario. No strong opinion, though, was just wondering.
On Wed Jan 22, 2025 at 10:27 AM EST, David Hildenbrand wrote: > On 22.01.25 16:16, Zi Yan wrote: >> On Wed Jan 22, 2025 at 9:26 AM EST, David Hildenbrand wrote: >>> On 22.01.25 13:40, Zi Yan wrote: >>>> Commit acd7ccb284b8 ("mm: shmem: add large folio support for tmpfs") >>>> changes huge=always to allocate THP/mTHP based on write size and >>>> split_huge_page_test does not write PMD size data, so file-back THP is not >>>> created during the test. >>> >>> Just curious, why can't we write PMD size data instead, to avoid messing >>> with the "force" option? >> >> It also works. I used "force", because I notice that it is intended for >> testing. Using it might be more future proof, in case huge=always changes >> its semantics again in the future. > > I recall discussing with Hugh in an upstream call that "force" is a > relict from older times, so naturally I would have just adjusted the > test case to trigger the PMD scenario. No strong opinion, though, was > just wondering. Got it. Let me change it and resend. Thank you for the feedback.
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c index 3f353f3d070f..8e15fc9dce3a 100644 --- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -264,15 +264,46 @@ void split_pte_mapped_thp(void) void split_file_backed_thp(void) { int status; - int fd; - ssize_t num_written; + int fd, shmem_sysctl_fd; + ssize_t num_written, num_read; char tmpfs_template[] = "/tmp/thp_split_XXXXXX"; const char *tmpfs_loc = mkdtemp(tmpfs_template); - char testfile[INPUT_MAX]; + char testfile[INPUT_MAX], sysctl_buf[INPUT_MAX] = {0}; uint64_t pgoff_start = 0, pgoff_end = 1024; + const char *shmem_sysctl = "/sys/kernel/mm/transparent_hugepage/shmem_enabled"; + char *opt1, *opt2; ksft_print_msg("Please enable pr_debug in split_huge_pages_in_file() for more info.\n"); + shmem_sysctl_fd = open(shmem_sysctl, O_RDWR); + if (shmem_sysctl_fd == -1) { + ksft_perror("cannot open shmem sysctl"); + goto out; + } + + num_read = read(shmem_sysctl_fd, sysctl_buf, INPUT_MAX); + if (num_read < 1) { + ksft_perror("Failed to read shmem sysctl"); + goto cleanup_sysctl; + } + + opt1 = strchr(sysctl_buf, '['); + opt2 = strchr(sysctl_buf, ']'); + if (!opt1 || !opt2) { + ksft_perror("cannot read shmem sysctl config"); + goto cleanup_sysctl; + } + + /* get existing shmem sysctl config into sysctl_buf */ + strncpy(sysctl_buf, opt1 + 1, opt2 - opt1 - 1); + memset(sysctl_buf + (opt2 - opt1 - 1), 0, INPUT_MAX); + + num_written = write(shmem_sysctl_fd, "force", sizeof("force")); + if (num_written < 1) { + ksft_perror("Fail to write force to shmem sysctl"); + goto cleanup_sysctl; + } + status = mount("tmpfs", tmpfs_loc, "tmpfs", 0, "huge=always,size=4m"); if (status) @@ -317,13 +348,24 @@ void split_file_backed_thp(void) if (status) ksft_exit_fail_msg("cannot remove tmp dir: %s\n", strerror(errno)); + num_written = write(shmem_sysctl_fd, sysctl_buf, strlen(sysctl_buf) + 1); + if (num_written < 1) + ksft_perror("Fail to restore shmem sysctl"); + + close(shmem_sysctl_fd); ksft_print_msg("Please check dmesg for more information\n"); ksft_test_result_pass("File-backed THP split test done\n"); return; cleanup: + num_written = write(shmem_sysctl_fd, sysctl_buf, strlen(sysctl_buf) + 1); + if (num_written < 1) + ksft_perror("Fail to restore shmem sysctl"); umount(tmpfs_loc); rmdir(tmpfs_loc); +cleanup_sysctl: + close(shmem_sysctl_fd); +out: ksft_exit_fail_msg("Error occurred\n"); }
Commit acd7ccb284b8 ("mm: shmem: add large folio support for tmpfs") changes huge=always to allocate THP/mTHP based on write size and split_huge_page_test does not write PMD size data, so file-back THP is not created during the test. Set /sys/kernel/mm/transparent_hugepage/shmem_enabled to "force" to force THP allocation. Signed-off-by: Zi Yan <ziy@nvidia.com> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com> Tested-by: Baolin Wang <baolin.wang@linux.alibaba.com> --- .../selftests/mm/split_huge_page_test.c | 48 +++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-)