Re: [PATCH v3 3/4] selftests/mm: vm_util: Split up /proc/self/smaps parsing

[David Hildenbrand <david@redhat.com>]

On 13.01.25 14:15, Thomas Weißschuh wrote:
> Upcoming changes want to reuse the /proc/self/smaps parsing logic to
> parse the VmFlags field.
> As that works differently from the currently parsed HugePage counters,
> split up the logic so common functionality can be shared.
> 
> While reworking this code, also use the correct sscanf placeholder for
> the "uint64_t thp" variable.
> 
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>   tools/testing/selftests/mm/vm_util.c | 42 +++++++++++++++++++++++++-----------
>   1 file changed, 29 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> index d8d0cf04bb57fd22bd7748fffec6a23c3103e35c..8468a19d6acca10c7e9228c03a935cdeb2402b5d 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -2,6 +2,7 @@
>   #include <string.h>
>   #include <fcntl.h>
>   #include <dirent.h>
> +#include <inttypes.h>
>   #include <sys/ioctl.h>
>   #include <linux/userfaultfd.h>
>   #include <linux/fs.h>
> @@ -193,13 +194,11 @@ unsigned long rss_anon(void)
>   	return rss_anon;
>   }
>   
> -bool __check_huge(void *addr, char *pattern, int nr_hpages,
> -		  uint64_t hpage_size)
> +char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len)
>   {
> -	uint64_t thp = -1;
>   	int ret;
>   	FILE *fp;
> -	char buffer[MAX_LINE_LENGTH];
> +	char *entry;
>   	char addr_pattern[MAX_LINE_LENGTH];
>   
>   	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "%08lx-",
> @@ -211,23 +210,40 @@ bool __check_huge(void *addr, char *pattern, int nr_hpages,
>   	if (!fp)
>   		ksft_exit_fail_msg("%s: Failed to open file %s\n", __func__, SMAP_FILE_PATH);
>   
> -	if (!check_for_pattern(fp, addr_pattern, buffer, sizeof(buffer)))
> +	if (!check_for_pattern(fp, addr_pattern, buf, len))
>   		goto err_out;
>   
> -	/*
> -	 * Fetch the pattern in the same block and check the number of
> -	 * hugepages.
> -	 */
> -	if (!check_for_pattern(fp, pattern, buffer, sizeof(buffer)))
> +	/* Fetch the pattern in the same block */
> +	if (!check_for_pattern(fp, pattern, buf, len))
>   		goto err_out;
>   
> -	snprintf(addr_pattern, MAX_LINE_LENGTH, "%s%%9ld kB", pattern);
> +	/* Trim trailing newline */
> +	entry = strchr(buf, '\n');
> +	if (entry)
> +		*entry = '\0';
>   
> -	if (sscanf(buffer, addr_pattern, &thp) != 1)
> -		ksft_exit_fail_msg("Reading smap error\n");
> +	entry = buf + strlen(pattern);
>   
>   err_out:
>   	fclose(fp);
> +	return entry;
> +}

Yes, LGTM!

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

-- 
Cheers,

David / dhildenb