Re: [PATCH 2/3] selftests/proc: ensure the test is performed at the right page boundary

[Liam R. Howlett]

On 26/04/24 12:02AM, Suren Baghdasaryan wrote:
> When running tearing tests we need to ensure the pages we use include
> VMAs that were mapped by the child process for this test. Currently we
> always use the first two pages, checking VMAs at their boundaries and
> this works, however once we add tests for /proc/pid/smaps, the first two
> pages might not contain the VMAs that child modifies.
> Locate the page that contains the first VMA mapped by the child and use
> that and the next page for the test.
> 
> Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>

Reviewed-by: Liam R. Howlett <liam@xxxxxxxxxxxxx>

> ---
>  tools/testing/selftests/proc/proc-maps-race.c | 78 ++++++++++++++++---
>  1 file changed, 68 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/testing/selftests/proc/proc-maps-race.c b/tools/testing/selftests/proc/proc-maps-race.c
> index a734553718da..c5031b0593b7 100644
> --- a/tools/testing/selftests/proc/proc-maps-race.c
> +++ b/tools/testing/selftests/proc/proc-maps-race.c
> @@ -39,6 +39,13 @@
>  #include <sys/types.h>
>  #include <sys/wait.h>
>  
> +#define min(a, b) \
> +	({ \
> +		typeof(a) _a = (a); \
> +		typeof(b) _b = (b); \
> +		_a < _b ? _a : _b; \
> +	})
> +

Another implementation of min in the selftests.  I guess it's somewhat
unavoidable.

>  /* /proc/pid/maps parsing routines */
>  struct page_content {
>  	char *data;
> @@ -77,6 +84,7 @@ FIXTURE(proc_maps_race)
>  	struct line_content first_line;
>  	unsigned long duration_sec;
>  	int shared_mem_size;
> +	int skip_pages;
>  	int page_size;
>  	int vma_count;
>  	bool verbose;
> @@ -105,27 +113,68 @@ struct vma_modifier_info {
>  	void *child_mapped_addr[];
>  };
>  
> -
> -static bool read_two_pages(FIXTURE_DATA(proc_maps_race) *self)
> +static bool read_page(FIXTURE_DATA(proc_maps_race) *self,
> +		      struct page_content *page)
>  {
>  	ssize_t  bytes_read;
>  
> -	if (lseek(self->maps_fd, 0, SEEK_SET) < 0)
> +	bytes_read = read(self->maps_fd, page->data, self->page_size);
> +	if (bytes_read <= 0)
>  		return false;
>  
> -	bytes_read = read(self->maps_fd, self->page1.data, self->page_size);
> -	if (bytes_read <= 0)
> +	/* Make sure data always ends with a newline character. */
> +	if (page->data[bytes_read - 1] != '\n')
>  		return false;
>  
> -	self->page1.size = bytes_read;
> +	page->size = bytes_read;
>  
> -	bytes_read = read(self->maps_fd, self->page2.data, self->page_size);
> -	if (bytes_read <= 0)
> +	return true;
> +}
> +
> +static int locate_containing_page(FIXTURE_DATA(proc_maps_race) *self,
> +				  unsigned long addr, unsigned long size)
> +{
> +	unsigned long start, end;
> +	int page = 0;
> +
> +	if (lseek(self->maps_fd, 0, SEEK_SET) < 0)
> +		return -1;
> +
> +	while (true) {
> +		char *curr_pos;
> +		char *end_pos;
> +
> +		if (!read_page(self, &self->page1))
> +			return -1;
> +
> +		curr_pos = self->page1.data;
> +		end_pos = self->page1.data + self->page1.size;
> +		while (curr_pos < end_pos) {
> +			if (sscanf(curr_pos, "%lx-%lx", &start, &end) == 2 &&
> +			    start == addr && end == addr + size)
> +				return page;
> +
> +			curr_pos = strchr(curr_pos, '\n');
> +			if (!curr_pos)
> +				break;
> +			curr_pos++;
> +		}
> +		page++;
> +	}
> +
> +	return 0;
> +}
> +
> +static bool read_two_pages(FIXTURE_DATA(proc_maps_race) *self)
> +{
> +	if (lseek(self->maps_fd, 0, SEEK_SET) < 0)
>  		return false;
>  
> -	self->page2.size = bytes_read;
> +	for (int i = 0; i < self->skip_pages; i++)
> +		if (!read_page(self, &self->page1))
> +			return false;
>  
> -	return true;
> +	return read_page(self, &self->page1) && read_page(self, &self->page2);
>  }
>  
>  static void copy_first_line(struct page_content *page, char *first_line)
> @@ -418,6 +467,8 @@ FIXTURE_SETUP(proc_maps_race)
>  	struct vma_modifier_info *mod_info;
>  	pthread_mutexattr_t mutex_attr;
>  	pthread_condattr_t cond_attr;
> +	unsigned long first_map_addr;
> +	unsigned long last_map_addr;
>  	unsigned long duration_sec;
>  	char fname[32];
>  
> @@ -502,6 +553,13 @@ FIXTURE_SETUP(proc_maps_race)
>  	self->page2.data = malloc(self->page_size);
>  	ASSERT_NE(self->page2.data, NULL);
>  
> +	first_map_addr = (unsigned long)mod_info->child_mapped_addr[0];
> +	last_map_addr = (unsigned long)mod_info->child_mapped_addr[mod_info->vma_count - 1];
> +
> +	self->skip_pages = locate_containing_page(self,
> +					min(first_map_addr, last_map_addr),
> +					self->page_size * 3);
> +	ASSERT_NE(self->skip_pages, -1);
>  	ASSERT_TRUE(read_boundary_lines(self, &self->last_line, &self->first_line));
>  
>  	/*
> -- 
> 2.54.0.545.g6539524ca2-goog
>