Re: [RFC Patch 1/2] selftests/mm: put general ksm operation into vm_util

[David Hildenbrand <david@redhat.com>]

> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> index 1357e2d6a7b6..115422e9eb68 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -486,3 +486,74 @@ int close_procmap(struct procmap_fd *procmap)
>   {
>   	return close(procmap->fd);
>   }
> +

I think we should just let all these functions open/close the fds. So 
there will not be a need to pass in the fds.

> +int ksm_use_zero_pages(int ksm_use_zero_pages_fd)
> +{
> +	return write(ksm_use_zero_pages_fd, "1", 1);
> +}
> +
> +int ksm_start_and_merge(int ksm_fd)
> +{
> +	return write(ksm_fd, "1", 1);
 > +}> +
> +int ksm_stop_and_unmerge(int ksm_fd)
> +{
> +	return write(ksm_fd, "2", 1);
> +}

Can we make all these functions return "0" on success? This, way, the 
"write" will be an internal implementation detail.

E.g.,

int ksm_stop_and_unmerge(void)
{
	int ksm_fd = ...
	ssize_t ret;

	...

	ret = write(ksm_fd, "2", 1);
	close(ksm_fd);
	return ret == 1 ? 0 : ret;
}

> +
> +long ksm_get_full_scans(int ksm_full_scans_fd)
> +{
> +	char buf[10];
> +	ssize_t ret;
> +
> +	ret = pread(ksm_full_scans_fd, buf, sizeof(buf) - 1, 0);
> +	if (ret <= 0)
> +		return -errno;
> +	buf[ret] = 0;
> +
> +	return strtol(buf, NULL, 10);
> +}
> +
> +long ksm_get_self_merging_pages(int proc_self_ksm_merging_pages_fd)
> +{
> +	char buf[10];
> +	ssize_t ret;
> +
> +	if (proc_self_ksm_merging_pages_fd < 0)
> +		return proc_self_ksm_merging_pages_fd;
> +
> +	ret = pread(proc_self_ksm_merging_pages_fd, buf, sizeof(buf) - 1, 0);
> +	if (ret <= 0)
> +		return -errno;
> +	buf[ret] = 0;
> +
> +	return strtol(buf, NULL, 10);
> +}
> +
> +long ksm_get_self_zero_pages(int proc_self_ksm_stat_fd)
> +{
> +	char buf[200];
> +	char *substr_ksm_zero;
> +	size_t value_pos;
> +	ssize_t read_size;
> +	unsigned long my_ksm_zero_pages;
> +
> +	if (!proc_self_ksm_stat_fd)
> +		return 0;
> +
> +	read_size = pread(proc_self_ksm_stat_fd, buf, sizeof(buf) - 1, 0);
> +	if (read_size < 0)
> +		return -errno;
> +
> +	buf[read_size] = 0;
> +
> +	substr_ksm_zero = strstr(buf, "ksm_zero_pages");
> +	if (!substr_ksm_zero)
> +		return 0;
> +
> +	value_pos = strcspn(substr_ksm_zero, "0123456789");
> +	my_ksm_zero_pages = strtol(substr_ksm_zero + value_pos, NULL, 10);
> +
> +	return my_ksm_zero_pages;
> +}
> diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
> index 9211ba640d9c..99c1b1aa1813 100644
> --- a/tools/testing/selftests/mm/vm_util.h
> +++ b/tools/testing/selftests/mm/vm_util.h
> @@ -95,6 +95,13 @@ static inline int open_self_procmap(struct procmap_fd *procmap_out)
>   	return open_procmap(pid, procmap_out);
>   }
>   
> +int ksm_use_zero_pages(int ksm_use_zero_pages_fd);
> +int ksm_start_and_merge(int ksm_fd);
> +int ksm_stop_and_unmerge(int ksm_fd);
> +long ksm_get_full_scans(int ksm_full_scans_fd);
> +long ksm_get_self_merging_pages(int proc_self_ksm_merging_pages_fd);
> +long ksm_get_self_zero_pages(int proc_self_ksm_stat_fd);

With the fd parameters removed, that interface will look quite neat I think.

-- 
Cheers,

David / dhildenb