On Fri, Apr 24, 2026 at 12:59 PM Suren Baghdasaryan <[email protected]> wrote:
>
> On Fri, Apr 24, 2026 at 11:23 AM Liam R. Howlett <[email protected]> wrote:
> >
> > On 26/04/24 12:02AM, Suren Baghdasaryan wrote:
> > > Add MAPS_FILE environment variable to select whether the tests to be run
> > > for /proc/pid/maps or /proc/pid/smaps. Add support for testing smaps file
> > > by reusing the same logic as with maps file but skipping all the data
> > > except for the VMA addresses, which are the only part relevant for the
> > > tearing tests. Skip PROCMAP_QUERY parts of the tests because smaps does
> > > not implement that ioctl.
> > >
> > > Signed-off-by: Suren Baghdasaryan <[email protected]>
> >
> > One comment question, otherwise..
> >
> > Reviewed-by: Liam R. Howlett <[email protected]>
> >
> > > ---
> > > tools/testing/selftests/proc/proc-maps-race.c | 176 +++++++++++++-----
> > > 1 file changed, 132 insertions(+), 44 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/proc/proc-maps-race.c
> > > b/tools/testing/selftests/proc/proc-maps-race.c
> > > index c5031b0593b7..b677bc23cab9 100644
> > > --- a/tools/testing/selftests/proc/proc-maps-race.c
> > > +++ b/tools/testing/selftests/proc/proc-maps-race.c
> > > @@ -17,8 +17,8 @@
> > > */
> > > /*
> > > * Fork a child that concurrently modifies address space while the main
> > > - * process is reading /proc/$PID/maps and verifying the results. Address
> > > - * space modifications include:
> > > + * process is reading /proc/$PID/maps and /proc/$PID/smaps, verifying the
> > ^^^ and? Should this be or? It
> > doesn't match your commit
> > message.
>
> Quite right. I'll fix it. Need to post a new version anyway since
> Sashiko found a couple of issues.
Actually, I realized that I can use FIXTURE_VARIANT to easily run
tests for both smaps and maps in the same run. I think that's a better
option.
v2 has a number of fixups for issues flagged by Sashiko but they are
quite straight-forward, so I think I can keep your Reviewed-by.
Will post an update shortly after I verify that Sashiko is fine with it.
>
> >
> >
> > > + * results. Address space modifications include:
> > > * VMA splitting and merging
> > > *
> > > */
> > > @@ -73,6 +73,11 @@ enum test_state {
> > > TEST_DONE,
> > > };
> > >
> > > +enum maps_file {
> > > + MAPS,
> > > + SMAPS,
> > > +};
> > > +
> > > struct vma_modifier_info;
> > >
> > > FIXTURE(proc_maps_race)
> > > @@ -83,6 +88,7 @@ FIXTURE(proc_maps_race)
> > > struct line_content last_line;
> > > struct line_content first_line;
> > > unsigned long duration_sec;
> > > + enum maps_file maps_file;
> > > int shared_mem_size;
> > > int skip_pages;
> > > int page_size;
> > > @@ -199,6 +205,57 @@ static void copy_last_line(struct page_content
> > > *page, char *last_line)
> > > last_line[end - pos] = '\0';
> > > }
> > >
> > > +static bool copy_first_entry(struct page_content *page, char *first_line)
> > > +{
> > > + const char *start_pos = page->data;
> > > +
> > > + while (start_pos < page->data + page->size) {
> > > + unsigned long start_addr;
> > > + unsigned long end_addr;
> > > + char *end_pos;
> > > +
> > > + end_pos = strchr(start_pos, '\n');
> > > + if (!end_pos)
> > > + break;
> > > +
> > > + if (sscanf(start_pos, "%lx-%lx", &start_addr, &end_addr) ==
> > > 2) {
> > > + strncpy(first_line, start_pos, end_pos - start_pos);
> > > + first_line[end_pos - start_pos] = '\0';
> > > + return true;
> > > + }
> > > +
> > > + start_pos = end_pos + 1;
> > > + }
> > > +
> > > + return false;
> > > +}
> > > +
> > > +static bool copy_last_entry(struct page_content *page, char *last_line)
> > > +{
> > > + const char *end_pos = page->data + page->size - 1;
> > > + const char *start_pos;
> > > +
> > > + while (end_pos > page->data) {
> > > + unsigned long start_addr;
> > > + unsigned long end_addr;
> > > +
> > > + /* skip last newline */
> > > + start_pos = end_pos - 1;
> > > + /* search previous newline */
> > > + while (start_pos > page->data && start_pos[-1] != '\n')
> > > + start_pos--;
> > > + if (sscanf(start_pos, "%lx-%lx", &start_addr, &end_addr) ==
> > > 2) {
> > > + strncpy(last_line, start_pos, end_pos - start_pos);
> > > + last_line[end_pos - start_pos] = '\0';
> > > + return true;
> > > + }
> > > +
> > > + end_pos = start_pos - 1;
> > > + }
> > > +
> > > + return false;
> > > +}
> > > +
> > > /* Read the last line of the first page and the first line of the second
> > > page */
> > > static bool read_boundary_lines(FIXTURE_DATA(proc_maps_race) *self,
> > > struct line_content *last_line,
> > > @@ -207,8 +264,16 @@ static bool
> > > read_boundary_lines(FIXTURE_DATA(proc_maps_race) *self,
> > > if (!read_two_pages(self))
> > > return false;
> > >
> > > - copy_last_line(&self->page1, last_line->text);
> > > - copy_first_line(&self->page2, first_line->text);
> > > + if (self->maps_file == MAPS) {
> > > + copy_last_line(&self->page1, last_line->text);
> > > + copy_first_line(&self->page2, first_line->text);
> > > + } else if (self->maps_file == SMAPS) {
> > > + if (!copy_last_entry(&self->page1, last_line->text) ||
> > > + !copy_first_entry(&self->page2, first_line->text))
> > > + return false;
> > > + } else {
> > > + return false;
> > > + }
> > >
> > > return sscanf(last_line->text, "%lx-%lx", &last_line->start_addr,
> > > &last_line->end_addr) == 2 &&
> > > @@ -464,6 +529,7 @@ FIXTURE_SETUP(proc_maps_race)
> > > {
> > > const char *verbose = getenv("VERBOSE");
> > > const char *duration = getenv("DURATION");
> > > + const char *maps_file = getenv("MAPS_FILE");
> > > struct vma_modifier_info *mod_info;
> > > pthread_mutexattr_t mutex_attr;
> > > pthread_condattr_t cond_attr;
> > > @@ -474,6 +540,18 @@ FIXTURE_SETUP(proc_maps_race)
> > >
> > > self->page_size = (unsigned long)sysconf(_SC_PAGESIZE);
> > > self->verbose = verbose && !strncmp(verbose, "1", 1);
> > > + if (maps_file) {
> > > + if (!strcmp(maps_file, "maps")) {
> > > + self->maps_file = MAPS;
> > > + } else if (!strcmp(maps_file, "smaps")) {
> > > + self->maps_file = SMAPS;
> > > + } else {
> > > + ksft_print_msg("Unknown maps file %s\n", maps_file);
> > > + ksft_exit_fail();
> > > + }
> > > + } else {
> > > + self->maps_file = MAPS;
> > > + }
> > > duration_sec = duration ? atol(duration) : 0;
> > > self->duration_sec = duration_sec ? duration_sec : 5UL;
> > >
> > > @@ -540,7 +618,16 @@ FIXTURE_SETUP(proc_maps_race)
> > > exit(0);
> > > }
> > >
> > > - sprintf(fname, "/proc/%d/maps", self->pid);
> > > + switch (self->maps_file) {
> > > + case MAPS:
> > > + sprintf(fname, "/proc/%d/maps", self->pid);
> > > + break;
> > > + case SMAPS:
> > > + sprintf(fname, "/proc/%d/smaps", self->pid);
> > > + break;
> > > + default:
> > > + ksft_exit_fail();
> > > + }
> > > self->maps_fd = open(fname, O_RDONLY);
> > > ASSERT_NE(self->maps_fd, -1);
> > >
> > > @@ -675,20 +762,20 @@ TEST_F(proc_maps_race, test_maps_tearing_from_split)
> > > last_line_changed = strcmp(new_last_line.text,
> > > self->last_line.text) != 0;
> > > first_line_changed = strcmp(new_first_line.text,
> > > self->first_line.text) != 0;
> > > ASSERT_EQ(last_line_changed, first_line_changed);
> > > -
> > > - /* Check if PROCMAP_QUERY ioclt() finds the right VMA */
> > > - ASSERT_TRUE(query_addr_at(self->maps_fd, mod_info->addr +
> > > self->page_size,
> > > - &vma_start, &vma_end));
> > > - /*
> > > - * The vma at the split address can be either the same as
> > > - * original one (if read before the split) or the same as
> > > the
> > > - * first line in the second page (if read after the split).
> > > - */
> > > - ASSERT_TRUE((vma_start == self->last_line.start_addr &&
> > > - vma_end == self->last_line.end_addr) ||
> > > - (vma_start == split_first_line.start_addr &&
> > > - vma_end == split_first_line.end_addr));
> > > -
> > > + if (self->maps_file == MAPS) {
> > > + /* Check if PROCMAP_QUERY ioclt() finds the right
> > > VMA */
> > > + ASSERT_TRUE(query_addr_at(self->maps_fd,
> > > mod_info->addr + self->page_size,
> > > + &vma_start, &vma_end));
> > > + /*
> > > + * The vma at the split address can be either the
> > > same as
> > > + * original one (if read before the split) or the
> > > same as the
> > > + * first line in the second page (if read after the
> > > split).
> > > + */
> > > + ASSERT_TRUE((vma_start ==
> > > self->last_line.start_addr &&
> > > + vma_end == self->last_line.end_addr) ||
> > > + (vma_start ==
> > > split_first_line.start_addr &&
> > > + vma_end == split_first_line.end_addr));
> > > + }
> > > clock_gettime(CLOCK_MONOTONIC_COARSE, &end_ts);
> > > end_test_iteration(&end_ts, self->verbose);
> > > } while (end_ts.tv_sec - start_ts.tv_sec < self->duration_sec);
> > > @@ -758,17 +845,18 @@ TEST_F(proc_maps_race,
> > > test_maps_tearing_from_resize)
> > > strcmp(new_first_line.text,
> > > restored_first_line.text),
> > > "Expand result invalid", self));
> > > }
> > > -
> > > - /* Check if PROCMAP_QUERY ioclt() finds the right VMA */
> > > - ASSERT_TRUE(query_addr_at(self->maps_fd, mod_info->addr,
> > > &vma_start, &vma_end));
> > > - /*
> > > - * The vma should stay at the same address and have either
> > > the
> > > - * original size of 3 pages or 1 page if read after
> > > shrinking.
> > > - */
> > > - ASSERT_TRUE(vma_start == self->last_line.start_addr &&
> > > - (vma_end - vma_start == self->page_size * 3 ||
> > > - vma_end - vma_start == self->page_size));
> > > -
> > > + if (self->maps_file == MAPS) {
> > > + /* Check if PROCMAP_QUERY ioclt() finds the right
> > > VMA */
> > > + ASSERT_TRUE(query_addr_at(self->maps_fd,
> > > mod_info->addr,
> > > + &vma_start, &vma_end));
> > > + /*
> > > + * The vma should stay at the same address and have
> > > either the
> > > + * original size of 3 pages or 1 page if read after
> > > shrinking.
> > > + */
> > > + ASSERT_TRUE(vma_start == self->last_line.start_addr
> > > &&
> > > + (vma_end - vma_start == self->page_size
> > > * 3 ||
> > > + vma_end - vma_start ==
> > > self->page_size));
> > > + }
> > > clock_gettime(CLOCK_MONOTONIC_COARSE, &end_ts);
> > > end_test_iteration(&end_ts, self->verbose);
> > > } while (end_ts.tv_sec - start_ts.tv_sec < self->duration_sec);
> > > @@ -838,20 +926,20 @@ TEST_F(proc_maps_race, test_maps_tearing_from_remap)
> > > strcmp(new_first_line.text,
> > > restored_first_line.text),
> > > "Remap restore result invalid",
> > > self));
> > > }
> > > -
> > > - /* Check if PROCMAP_QUERY ioclt() finds the right VMA */
> > > - ASSERT_TRUE(query_addr_at(self->maps_fd, mod_info->addr +
> > > self->page_size,
> > > - &vma_start, &vma_end));
> > > - /*
> > > - * The vma should either stay at the same address and have
> > > the
> > > - * original size of 3 pages or we should find the remapped
> > > vma
> > > - * at the remap destination address with size of 1 page.
> > > - */
> > > - ASSERT_TRUE((vma_start == self->last_line.start_addr &&
> > > - 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));
> > > -
> > > + if (self->maps_file == MAPS) {
> > > + /* Check if PROCMAP_QUERY ioclt() finds the right
> > > VMA */
> > > + ASSERT_TRUE(query_addr_at(self->maps_fd,
> > > mod_info->addr + self->page_size,
> > > + &vma_start, &vma_end));
> > > + /*
> > > + * The vma should either stay at the same address
> > > and have the
> > > + * original size of 3 pages or we should find the
> > > remapped vma
> > > + * at the remap destination address with size of 1
> > > page.
> > > + */
> > > + ASSERT_TRUE((vma_start ==
> > > self->last_line.start_addr &&
> > > + 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));
> > > + }
> > > clock_gettime(CLOCK_MONOTONIC_COARSE, &end_ts);
> > > end_test_iteration(&end_ts, self->verbose);
> > > } while (end_ts.tv_sec - start_ts.tv_sec < self->duration_sec);
> > > --
> > > 2.54.0.545.g6539524ca2-goog
> > >
