linux-kernel - Re: [PATCH v5 01/10] selftests/mm: make file-backed THP split work by setting force option

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d1a5f942-0c43-475d-854a-9e4a34d04556@linux.alibaba.com>
Date: Wed, 22 Jan 2025 14:32:07 +0800
From: Baolin Wang <baolin.wang@...ux.alibaba.com>
To: Zi Yan <ziy@...dia.com>, linux-mm@...ck.org,
 Andrew Morton <akpm@...ux-foundation.org>,
 "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
 "Matthew Wilcox (Oracle)" <willy@...radead.org>
Cc: Ryan Roberts <ryan.roberts@....com>, Hugh Dickins <hughd@...gle.com>,
 David Hildenbrand <david@...hat.com>, Yang Shi
 <yang@...amperecomputing.com>, Miaohe Lin <linmiaohe@...wei.com>,
 Kefeng Wang <wangkefeng.wang@...wei.com>, Yu Zhao <yuzhao@...gle.com>,
 John Hubbard <jhubbard@...dia.com>, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 01/10] selftests/mm: make file-backed THP split work by
 setting force option



On 2025/1/17 05:10, 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.
> 
> Set /sys/kernel/mm/transparent_hugepage/shmem_enabled to "force" to force
> THP allocation.
> 
> Signed-off-by: Zi Yan <ziy@...dia.com>
> ---
>   .../selftests/mm/split_huge_page_test.c       | 47 +++++++++++++++++--
>   1 file changed, 43 insertions(+), 4 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..eccaa347140b 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");

You should close() shmem_sysctl_fd before returning.

> +		goto out;
> +	}
> +
> +	opt1 = strchr(sysctl_buf, '[');
> +	opt2 = strchr(sysctl_buf, ']');
> +	if (!opt1 || !opt2) {

Ditto.

> +		ksft_perror("cannot read shmem sysctl config");
> +		goto out;
> +	}
> +
> +	/* 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 out;
> +	}
> +
>   	status = mount("tmpfs", tmpfs_loc, "tmpfs", 0, "huge=always,size=4m");
>   
>   	if (status)
> @@ -317,13 +348,21 @@ 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");
> +
>   	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);

Seems the patch set split has issues. No 'order' variable in this patch.

Anyway, I've fixed these issues in my local tree, and it works well. If 
you fix them in the next version, please feel free to add:
Reviewed-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
Tested-by: Baolin Wang <baolin.wang@...ux.alibaba.com>


>   	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);
> +out:
>   	ksft_exit_fail_msg("Error occurred\n");
>   }
>