Re: [PATCH 3/3] selftests/proc: add /proc/pid/smaps tearing tests

[Suren Baghdasaryan]

On Fri, Apr 24, 2026 at 11:23 AM Liam R. Howlett <liam@xxxxxxxxxxxxx> 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 <surenb@xxxxxxxxxx>
>
> One comment question, otherwise..
>
> Reviewed-by: Liam R. Howlett <liam@xxxxxxxxxxxxx>
>
> > ---
> >  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.

>
>
> > + * 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
> >