mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Suren Baghdasaryan <surenb@google.com>
Cc: akpm@linux-foundation.org, liam@infradead.org, vbabka@kernel.org,
	 david@redhat.com, willy@infradead.org, jannh@google.com,
	paulmck@kernel.org,  pfalcato@suse.de, xueyuan.chen21@gmail.com,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v3 7/7] selftests/proc: add /proc/pid/smaps_rollup tearing tests
Date: Fri, 11 Sep 2026 20:12:34 +0100	[thread overview]
Message-ID: <aqRRpPywkExEyDEw@gremlin> (raw)
In-Reply-To: <20260910234737.1340642-8-surenb@google.com>

On Thu, Sep 10, 2026 at 04:47:37PM -0700, Suren Baghdasaryan wrote:
> During tearing tests, smaps_rollup Pss* metrics should stay constant.
> Extend /proc/pid/smaps tearing tests to also check for smaps_rollup
> consistency.
>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>

All looks reasonable an dall passing locally so:

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  tools/testing/selftests/proc/proc-maps-race.c | 187 +++++++++++++++++-
>  1 file changed, 182 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/proc/proc-maps-race.c b/tools/testing/selftests/proc/proc-maps-race.c
> index 415eccb70468..8d00d7db1c65 100644
> --- a/tools/testing/selftests/proc/proc-maps-race.c
> +++ b/tools/testing/selftests/proc/proc-maps-race.c
> @@ -80,6 +80,61 @@ enum maps_file {
>
>  struct vma_modifier_info;
>
> +enum smaps_rollup_stat {
> +	Rss,
> +	Pss,
> +	Pss_Dirty,
> +	Pss_Anon,
> +	Pss_File,
> +	Pss_Shmem,
> +	Shared_Clean,
> +	Shared_Dirty,
> +	Private_Clean,
> +	Private_Dirty,
> +	Referenced,
> +	Anonymous,
> +	KSM,
> +	LazyFree,
> +	AnonHugePages,
> +	ShmemPmdMapped,
> +	FilePmdMapped,
> +	Shared_Hugetlb,
> +	Private_Hugetlb,
> +	Swap,
> +	SwapPss,
> +	Locked,
> +	RollupFieldCount
> +};
> +
> +static const char *smaps_rollup_stat_names[RollupFieldCount] = {
> +	"Rss",
> +	"Pss",
> +	"Pss_Dirty",
> +	"Pss_Anon",
> +	"Pss_File",
> +	"Pss_Shmem",
> +	"Shared_Clean",
> +	"Shared_Dirty",
> +	"Private_Clean",
> +	"Private_Dirty",
> +	"Referenced",
> +	"Anonymous",
> +	"KSM",
> +	"LazyFree",
> +	"AnonHugePages",
> +	"ShmemPmdMapped",
> +	"FilePmdMapped",
> +	"Shared_Hugetlb",
> +	"Private_Hugetlb",
> +	"Swap",
> +	"SwapPss",
> +	"Locked",
> +};
> +
> +struct smaps_rollup_stats {
> +	unsigned long values[RollupFieldCount];
> +};
> +
>  FIXTURE(proc_maps_race)
>  {
>  	struct vma_modifier_info *mod_info;
> @@ -91,6 +146,7 @@ FIXTURE(proc_maps_race)
>  	enum maps_file maps_file;
>  	int shared_mem_size;
>  	int skip_pages;
> +	int rollup_fd;
>  	int page_size;
>  	int vma_count;
>  	bool verbose;
> @@ -132,12 +188,12 @@ struct vma_modifier_info {
>  	void *child_mapped_addr[];
>  };
>
> -static bool read_page(FIXTURE_DATA(proc_maps_race) *self,
> +static bool read_page(FIXTURE_DATA(proc_maps_race) *self, int fd,
>  		      struct page_content *page)
>  {
>  	ssize_t  bytes_read;
>
> -	bytes_read = read(self->maps_fd, page->data, self->page_size);
> +	bytes_read = read(fd, page->data, self->page_size);
>  	if (bytes_read <= 0)
>  		return false;
>
> @@ -175,7 +231,7 @@ static int locate_containing_page(FIXTURE_DATA(proc_maps_race) *self,
>  		char *curr_pos;
>  		char *end_pos;
>
> -		if (!read_page(self, &self->page1))
> +		if (!read_page(self, self->maps_fd, &self->page1))
>  			return -1;
>
>  		curr_pos = self->page1.data;
> @@ -205,10 +261,11 @@ static bool read_two_pages(FIXTURE_DATA(proc_maps_race) *self)
>  		return false;
>
>  	for (int i = 0; i < self->skip_pages; i++)
> -		if (!read_page(self, &self->page1))
> +		if (!read_page(self, self->maps_fd, &self->page1))
>  			return false;
>
> -	return read_page(self, &self->page1) && read_page(self, &self->page2);
> +	return read_page(self, self->maps_fd, &self->page1) &&
> +	       read_page(self, self->maps_fd, &self->page2);
>  }
>
>  static void copy_line(const char *line_start, const char *line_end,
> @@ -317,6 +374,61 @@ static bool read_boundary_lines(FIXTURE_DATA(proc_maps_race) *self,
>  		      &first_line->end_addr) == 2;
>  }
>
> +static bool parse_smaps_rollup(FIXTURE_DATA(proc_maps_race) *self,
> +			       struct smaps_rollup_stats *stats)
> +{
> +	unsigned int dev_maj, dev_min, inode;
> +	unsigned long start, end, offs;
> +	unsigned long value;
> +	char name[32], perm[5];
> +	char *curr_pos;
> +	char *end_pos;
> +	char *line_end;
> +
> +	if (lseek(self->rollup_fd, 0, SEEK_SET) < 0)
> +		return false;
> +
> +	if (!read_page(self, self->rollup_fd, &self->page1))
> +		return false;
> +
> +	curr_pos = self->page1.data;
> +	end_pos = self->page1.data + self->page1.size;
> +
> +	line_end = strchr(curr_pos, '\n');
> +	if (!line_end)
> +		return false;
> +
> +	if (sscanf(curr_pos, "%lx-%lx %4s %lx %u:%u %u %31s",
> +		&start, &end, perm, &offs, &dev_maj, &dev_min, &inode, name) != 8)
> +		return false;
> +
> +	if (strcmp(name, "[rollup]"))
> +		return false;
> +
> +	for (int stat = 0; stat < ARRAY_SIZE(smaps_rollup_stat_names); stat++) {
> +		int len;
> +
> +		curr_pos = line_end + 1;
> +		if (curr_pos >= end_pos)
> +			return false;
> +
> +		line_end = strchr(curr_pos, '\n');
> +		if (!line_end)
> +			return false;
> +
> +		if (sscanf(curr_pos, "%31s %lu kB", name, &value) != 2)
> +			return false;
> +
> +		len = strlen(name);
> +		if (name[len - 1] != ':' || strncmp(name, smaps_rollup_stat_names[stat], len - 1))
> +			return false;
> +
> +		stats->values[stat] = value;
> +	}
> +
> +	return true;
> +}
> +
>  /* Thread synchronization routines */
>  static void wait_for_state(struct vma_modifier_info *mod_info, enum test_state state)
>  {
> @@ -397,6 +509,41 @@ static bool print_boundaries_on(bool condition, const char *title,
>  	return condition;
>  }
>
> +static void print_smaps_rollup_stats(const char *title, FIXTURE_DATA(proc_maps_race) *self,
> +				     struct smaps_rollup_stats *stats)
> +{
> +	printf("%s", title);
> +	for (int stat = 0; stat < ARRAY_SIZE(smaps_rollup_stat_names); stat++)
> +		printf("%64s %lu kB\n", smaps_rollup_stat_names[stat], stats->values[stat]);
> +}
> +
> +static bool cmp_smaps_rollup_stat(struct smaps_rollup_stats *s1,
> +				  struct smaps_rollup_stats *s2,
> +				  enum smaps_rollup_stat stat)
> +{
> +	return s1->values[stat] == s2->values[stat];
> +}
> +
> +static bool compare_smaps_rollup(FIXTURE_DATA(proc_maps_race) *self,
> +				 struct smaps_rollup_stats *expected,
> +				 struct smaps_rollup_stats *actual)
> +{
> +	/*
> +	 * Clean/dirty metrics might change but Pss-related ones
> +	 * should stay constant.
> +	 */
> +	if (cmp_smaps_rollup_stat(expected, actual, Pss) &&
> +	    cmp_smaps_rollup_stat(expected, actual, Pss_Anon) &&
> +	    cmp_smaps_rollup_stat(expected, actual, Pss_File) &&
> +	    cmp_smaps_rollup_stat(expected, actual, Pss_Shmem))
> +		return true;
> +
> +	print_smaps_rollup_stats("Expected stats:", self, expected);
> +	print_smaps_rollup_stats("Actual stats:", self, actual);
> +
> +	return false;
> +}
> +
>  static void report_test_start(const char *name, bool verbose)
>  {
>  	if (verbose)
> @@ -572,6 +719,7 @@ FIXTURE_SETUP(proc_maps_race)
>  	unsigned long first_map_addr;
>  	unsigned long last_map_addr;
>  	unsigned long duration_sec;
> +	char rollup_fname[32];
>  	char fname[32];
>
>  	self->page_size = (unsigned long)sysconf(_SC_PAGESIZE);
> @@ -649,6 +797,9 @@ FIXTURE_SETUP(proc_maps_race)
>  		break;
>  	case SMAPS:
>  		sprintf(fname, "/proc/%d/smaps", self->pid);
> +		sprintf(rollup_fname, "/proc/%d/smaps_rollup", self->pid);
> +		self->rollup_fd = open(rollup_fname, O_RDONLY);
> +		ASSERT_NE(self->rollup_fd, -1);
>  		break;
>  	default:
>  		ksft_exit_fail();
> @@ -711,6 +862,8 @@ FIXTURE_TEARDOWN(proc_maps_race)
>  	for (int i = 0; i < self->vma_count; i++)
>  		munmap(self->mod_info->child_mapped_addr[i], self->page_size);
>  	close(self->maps_fd);
> +	if (self->maps_file == SMAPS)
> +		close(self->rollup_fd);
>  	waitpid(self->pid, &status, 0);
>  	munmap(self->mod_info, self->shared_mem_size);
>  }
> @@ -723,6 +876,7 @@ TEST_F(proc_maps_race, test_maps_tearing_from_split)
>  	struct line_content split_first_line;
>  	struct line_content restored_last_line;
>  	struct line_content restored_first_line;
> +	struct smaps_rollup_stats orig_stats;
>
>  	wait_for_state(mod_info, SETUP_READY);
>
> @@ -736,6 +890,8 @@ TEST_F(proc_maps_race, test_maps_tearing_from_split)
>  	report_test_start("Tearing from split", self->verbose);
>  	ASSERT_TRUE(capture_mod_pattern(self, &split_last_line, &split_first_line,
>  					&restored_last_line, &restored_first_line));
> +	if (self->maps_file == SMAPS)
> +		ASSERT_TRUE(parse_smaps_rollup(self, &orig_stats));
>
>  	/* Now start concurrent modifications for self->duration_sec */
>  	signal_state(mod_info, TEST_READY);
> @@ -799,6 +955,11 @@ TEST_F(proc_maps_race, test_maps_tearing_from_split)
>  				     vma_end == self->last_line.end_addr) ||
>  				    (vma_start == split_first_line.start_addr &&
>  				     vma_end == split_first_line.end_addr));
> +		} else {
> +			struct smaps_rollup_stats stats;
> +
> +			ASSERT_TRUE(parse_smaps_rollup(self, &stats));
> +			ASSERT_TRUE(compare_smaps_rollup(self, &orig_stats, &stats));
>  		}
>  		clock_gettime(CLOCK_MONOTONIC_COARSE, &end_ts);
>  		end_test_iteration(&end_ts, self->verbose);
> @@ -817,6 +978,7 @@ TEST_F(proc_maps_race, test_maps_tearing_from_resize)
>  	struct line_content shrunk_first_line;
>  	struct line_content restored_last_line;
>  	struct line_content restored_first_line;
> +	struct smaps_rollup_stats orig_stats;
>
>  	wait_for_state(mod_info, SETUP_READY);
>
> @@ -830,6 +992,8 @@ TEST_F(proc_maps_race, test_maps_tearing_from_resize)
>  	report_test_start("Tearing from resize", self->verbose);
>  	ASSERT_TRUE(capture_mod_pattern(self, &shrunk_last_line, &shrunk_first_line,
>  					&restored_last_line, &restored_first_line));
> +	if (self->maps_file == SMAPS)
> +		ASSERT_TRUE(parse_smaps_rollup(self, &orig_stats));
>
>  	/* Now start concurrent modifications for self->duration_sec */
>  	signal_state(mod_info, TEST_READY);
> @@ -880,6 +1044,11 @@ TEST_F(proc_maps_race, test_maps_tearing_from_resize)
>  			ASSERT_TRUE(vma_start == self->last_line.start_addr &&
>  				    (vma_end - vma_start == self->page_size * 3 ||
>  				     vma_end - vma_start == self->page_size));
> +		} else {
> +			struct smaps_rollup_stats stats;
> +
> +			ASSERT_TRUE(parse_smaps_rollup(self, &stats));
> +			ASSERT_TRUE(compare_smaps_rollup(self, &orig_stats, &stats));
>  		}
>  		clock_gettime(CLOCK_MONOTONIC_COARSE, &end_ts);
>  		end_test_iteration(&end_ts, self->verbose);
> @@ -898,6 +1067,7 @@ TEST_F(proc_maps_race, test_maps_tearing_from_remap)
>  	struct line_content remapped_first_line;
>  	struct line_content restored_last_line;
>  	struct line_content restored_first_line;
> +	struct smaps_rollup_stats orig_stats;
>
>  	wait_for_state(mod_info, SETUP_READY);
>
> @@ -911,6 +1081,8 @@ TEST_F(proc_maps_race, test_maps_tearing_from_remap)
>  	report_test_start("Tearing from remap", self->verbose);
>  	ASSERT_TRUE(capture_mod_pattern(self, &remapped_last_line, &remapped_first_line,
>  					&restored_last_line, &restored_first_line));
> +	if (self->maps_file == SMAPS)
> +		ASSERT_TRUE(parse_smaps_rollup(self, &orig_stats));
>
>  	/* Now start concurrent modifications for self->duration_sec */
>  	signal_state(mod_info, TEST_READY);
> @@ -963,6 +1135,11 @@ TEST_F(proc_maps_race, test_maps_tearing_from_remap)
>  				     vma_end - vma_start == self->page_size * 3) ||
>  				    (vma_start == self->last_line.start_addr + self->page_size &&
>  				     vma_end - vma_start == self->page_size));
> +		} else {
> +			struct smaps_rollup_stats stats;
> +
> +			ASSERT_TRUE(parse_smaps_rollup(self, &stats));
> +			ASSERT_TRUE(compare_smaps_rollup(self, &orig_stats, &stats));
>  		}
>  		clock_gettime(CLOCK_MONOTONIC_COARSE, &end_ts);
>  		end_test_iteration(&end_ts, self->verbose);
> --
> 2.55.0.1007.g17ff1f9808-goog
>

--
Cheers, Lorenzo

      reply	other threads:[~2026-09-11 19:12 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-10 23:47 [PATCH v3 0/7] read proc/pid/smaps_rollup under per-vma lock Suren Baghdasaryan
2026-09-10 23:47 ` [PATCH v3 1/7] proc/task_mmu: remove unnecessary helpers Suren Baghdasaryan
2026-09-11 10:52   ` David Hildenbrand (Arm)
2026-09-11 14:28     ` Suren Baghdasaryan
2026-09-11 14:57       ` David Hildenbrand (Arm)
2026-09-11 15:20         ` Suren Baghdasaryan
2026-09-10 23:47 ` [PATCH v3 2/7] proc/task_mmu: remove unnecessary inlines in function definitions Suren Baghdasaryan
2026-09-11 15:33   ` David Hildenbrand (Arm)
2026-09-10 23:47 ` [PATCH v3 3/7] proc/task_mmu: clarify shmem mapping walk conditions in smap_gather_stats() Suren Baghdasaryan
2026-09-11 15:33   ` David Hildenbrand (Arm)
2026-09-11 16:28   ` Lorenzo Stoakes (ARM)
2026-09-11 16:58     ` Suren Baghdasaryan
2026-09-11 17:10       ` Lorenzo Stoakes (ARM)
2026-09-11 17:39         ` Suren Baghdasaryan
2026-09-11 17:52           ` David Hildenbrand (Arm)
2026-09-11 17:56           ` Lorenzo Stoakes (ARM)
2026-09-11 18:08             ` Suren Baghdasaryan
2026-09-10 23:47 ` [PATCH v3 4/7] proc/task_mmu: remove special-casing of smap_gather_stats() start parameter Suren Baghdasaryan
2026-09-11 15:34   ` David Hildenbrand (Arm)
2026-09-11 16:39   ` Lorenzo Stoakes (ARM)
2026-09-11 17:07     ` Suren Baghdasaryan
2026-09-11 17:49       ` Lorenzo Stoakes (ARM)
2026-09-11 18:06         ` Suren Baghdasaryan
2026-09-11 18:11           ` Lorenzo Stoakes (ARM)
2026-09-11 18:15             ` Suren Baghdasaryan
2026-09-10 23:47 ` [PATCH v3 5/7] proc/task_mmu: change proc_get_vma() to stop returning gate VMA at the end Suren Baghdasaryan
2026-09-11 15:35   ` David Hildenbrand (Arm)
2026-09-11 18:26   ` Lorenzo Stoakes (ARM)
2026-09-11 18:39     ` Suren Baghdasaryan
2026-09-11 19:03     ` Lorenzo Stoakes (ARM)
2026-09-11 19:11       ` Suren Baghdasaryan
2026-09-11 19:13         ` Lorenzo Stoakes (ARM)
2026-09-11 19:18           ` Suren Baghdasaryan
2026-09-11 19:26             ` Lorenzo Stoakes (ARM)
2026-09-11 19:44               ` Suren Baghdasaryan
2026-09-11 19:45                 ` Suren Baghdasaryan
2026-09-10 23:47 ` [PATCH v3 6/7] proc/task_mmu: read proc/pid/smaps_rollup under per-vma lock Suren Baghdasaryan
2026-09-11 19:07   ` Lorenzo Stoakes (ARM)
2026-09-10 23:47 ` [PATCH v3 7/7] selftests/proc: add /proc/pid/smaps_rollup tearing tests Suren Baghdasaryan
2026-09-11 19:12   ` Lorenzo Stoakes (ARM) [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aqRRpPywkExEyDEw@gremlin \
    --to=ljs@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=jannh@google.com \
    --cc=liam@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=paulmck@kernel.org \
    --cc=pfalcato@suse.de \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    --cc=willy@infradead.org \
    --cc=xueyuan.chen21@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®