[PATCH 1/3] selftests/mm: make file-backed THP split work by setting force option

[PATCH 1/3] selftests/mm: make file-backed THP split work by setting force option

[Zi Yan]

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(-)

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");
 }
 
-- 
2.45.2

[PATCH 2/3] mm/huge_memory: allow split shmem large folio to any lower order

[Zi Yan]

Commit 4d684b5f92ba ("mm: shmem: add large folio support for tmpfs") has
added large folio support to shmem. Remove the restriction in
split_huge_page*().

Signed-off-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/huge_memory.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 3d3ebdc002d5..deb4e72daeb9 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3299,7 +3299,7 @@ static void __split_huge_page(struct page *page, struct list_head *list,
 		/* Some pages can be beyond EOF: drop them from page cache */
 		if (tail->index >= end) {
 			if (shmem_mapping(folio->mapping))
-				nr_dropped++;
+				nr_dropped += new_nr;
 			else if (folio_test_clear_dirty(tail))
 				folio_account_cleaned(tail,
 					inode_to_wb(folio->mapping->host));
@@ -3465,12 +3465,6 @@ int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
 			return -EINVAL;
 		}
 	} else if (new_order) {
-		/* Split shmem folio to non-zero order not supported */
-		if (shmem_mapping(folio->mapping)) {
-			VM_WARN_ONCE(1,
-				"Cannot split shmem folio to non-0 order");
-			return -EINVAL;
-		}
 		/*
 		 * No split if the file system does not support large folio.
 		 * Note that we might still have THPs in such mappings due to
-- 
2.45.2

[PATCH 3/3] selftests/mm: test splitting file-backed THP to any lower order.

[Zi Yan]

Now split_huge_page*() supports shmem THP split to any lower order.
Test it.

The test now reads file content out after split to check if the split
corrupts the file data.

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       | 32 ++++++++++++++-----
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
index 8e15fc9dce3a..13ad5ac7d178 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -261,14 +261,16 @@ void split_pte_mapped_thp(void)
 	close(kpageflags_fd);
 }
 
-void split_file_backed_thp(void)
+void split_file_backed_thp(int order)
 {
 	int status;
 	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], sysctl_buf[INPUT_MAX] = {0};
+	char testfile[INPUT_MAX];
+	char sysctl_buf[INPUT_MAX] = {0};
+	char file_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;
@@ -314,7 +316,7 @@ void split_file_backed_thp(void)
 		ksft_exit_fail_msg("Fail to create file-backed THP split testing file\n");
 	}
 
-	fd = open(testfile, O_CREAT|O_WRONLY, 0664);
+	fd = open(testfile, O_CREAT|O_RDWR, 0664);
 	if (fd == -1) {
 		ksft_perror("Cannot open testing file");
 		goto cleanup;
@@ -322,7 +324,6 @@ void split_file_backed_thp(void)
 
 	/* write something to the file, so a file-backed THP can be allocated */
 	num_written = write(fd, tmpfs_loc, strlen(tmpfs_loc) + 1);
-	close(fd);
 
 	if (num_written < 1) {
 		ksft_perror("Fail to write data to testing file");
@@ -330,8 +331,22 @@ void split_file_backed_thp(void)
 	}
 
 	/* split the file-backed THP */
-	write_debugfs(PATH_FMT, testfile, pgoff_start, pgoff_end, 0);
+	write_debugfs(PATH_FMT, testfile, pgoff_start, pgoff_end, order);
+
+	/* check file content after split */
+	num_read = lseek(fd, 0, SEEK_SET);
+	if (num_read == -1) {
+		ksft_perror("Cannot lseek file");
+		goto cleanup;
+	}
 
+	num_read = read(fd, file_buf, num_written);
+	if (num_read < 1 || strncmp(file_buf, tmpfs_loc, num_read)) {
+		ksft_print_msg("File content changed, origin: %s, now: %s\n", tmpfs_loc, file_buf);
+		goto cleanup;
+	}
+
+	close(fd);
 	status = unlink(testfile);
 	if (status) {
 		ksft_perror("Cannot remove testing file");
@@ -354,7 +369,7 @@ void split_file_backed_thp(void)
 
 	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");
+	ksft_test_result_pass("File-backed THP split to order %d test done\n", order);
 	return;
 
 cleanup:
@@ -523,7 +538,7 @@ int main(int argc, char **argv)
 	if (argc > 1)
 		optional_xfs_path = argv[1];
 
-	ksft_set_plan(1+8+2+9);
+	ksft_set_plan(1+8+1+9+9);
 
 	pagesize = getpagesize();
 	pageshift = ffs(pagesize) - 1;
@@ -540,7 +555,8 @@ int main(int argc, char **argv)
 			split_pmd_thp_to_order(i);
 
 	split_pte_mapped_thp();
-	split_file_backed_thp();
+	for (i = 0; i < 9; i++)
+		split_file_backed_thp(i);
 
 	created_tmp = prepare_thp_fs(optional_xfs_path, fs_loc_template,
 			&fs_loc);
-- 
2.45.2

Re: [PATCH 1/3] selftests/mm: make file-backed THP split work by setting force option

[David Hildenbrand]

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?

-- 
Cheers,

David / dhildenb

Re: [PATCH 2/3] mm/huge_memory: allow split shmem large folio to any lower order

[David Hildenbrand]

On 22.01.25 13:40, Zi Yan wrote:
> Commit 4d684b5f92ba ("mm: shmem: add large folio support for tmpfs") has
> added large folio support to shmem. Remove the restriction in
> split_huge_page*().
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
>   mm/huge_memory.c | 8 +-------
>   1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 3d3ebdc002d5..deb4e72daeb9 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3299,7 +3299,7 @@ static void __split_huge_page(struct page *page, struct list_head *list,
>   		/* Some pages can be beyond EOF: drop them from page cache */
>   		if (tail->index >= end) {
>   			if (shmem_mapping(folio->mapping))
> -				nr_dropped++;
> +				nr_dropped += new_nr;
>   			else if (folio_test_clear_dirty(tail))
>   				folio_account_cleaned(tail,
>   					inode_to_wb(folio->mapping->host));
> @@ -3465,12 +3465,6 @@ int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
>   			return -EINVAL;
>   		}
>   	} else if (new_order) {
> -		/* Split shmem folio to non-zero order not supported */
> -		if (shmem_mapping(folio->mapping)) {
> -			VM_WARN_ONCE(1,
> -				"Cannot split shmem folio to non-0 order");
> -			return -EINVAL;
> -		}
>   		/*
>   		 * No split if the file system does not support large folio.
>   		 * Note that we might still have THPs in such mappings due to


Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 1/3] selftests/mm: make file-backed THP split work by setting force option

[Zi Yan]

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.


-- 
Best Regards,
Yan, Zi

Re: [PATCH 1/3] selftests/mm: make file-backed THP split work by setting force option

[David Hildenbrand]

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.

-- 
Cheers,

David / dhildenb

Re: [PATCH 1/3] selftests/mm: make file-backed THP split work by setting force option

[Zi Yan]

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.



-- 
Best Regards,
Yan, Zi