On Wed, Aug 13, 2025 at 1:39 PM Lorenzo Stoakes lorenzo.stoakes@oracle.com wrote:
On Fri, Aug 08, 2025 at 08:28:47AM -0700, Suren Baghdasaryan wrote:
Extend /proc/pid/maps tearing tests to verify PROCMAP_QUERY ioctl operation correctness while the vma is being concurrently modified.
General comment, but I really feel like this stuff is mm-specific. Yes it uses proc, but it's using it to check for mm functionality.
I mean I'd love for these to be in the mm self tests but I get obviously why they're in the proc ones...
Signed-off-by: Suren Baghdasaryan surenb@google.com Tested-by: SeongJae Park sj@kernel.org Acked-by: SeongJae Park sj@kernel.org
The tests themselves look good, had a good look through. But I've given you some nice ASCII diagrams to sprinkle liberally around :)
Thanks for the commentary, Lorenzo, it is great! I think I'll post them as a follow-up patch since they do not change the functionality of the test.
Anyway for tests themselves:
Reviewed-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com
Thanks!
tools/testing/selftests/proc/proc-maps-race.c | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+)
diff --git a/tools/testing/selftests/proc/proc-maps-race.c b/tools/testing/selftests/proc/proc-maps-race.c index 94bba4553130..a546475db550 100644 --- a/tools/testing/selftests/proc/proc-maps-race.c +++ b/tools/testing/selftests/proc/proc-maps-race.c @@ -32,6 +32,8 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <linux/fs.h> +#include <sys/ioctl.h> #include <sys/mman.h> #include <sys/stat.h> #include <sys/types.h> @@ -317,6 +319,25 @@ static bool capture_mod_pattern(FIXTURE_DATA(proc_maps_race) *self, strcmp(restored_first_line->text, self->first_line.text) == 0; }
+static bool query_addr_at(int maps_fd, void *addr,
unsigned long *vma_start, unsigned long *vma_end)+{
struct procmap_query q;memset(&q, 0, sizeof(q));q.size = sizeof(q);/* Find the VMA at the split address */q.query_addr = (unsigned long long)addr;q.query_flags = 0;if (ioctl(maps_fd, PROCMAP_QUERY, &q))return false;*vma_start = q.vma_start;*vma_end = q.vma_end;return true;+}
static inline bool split_vma(FIXTURE_DATA(proc_maps_race) *self) { return mmap(self->mod_info->addr, self->page_size, self->mod_info->prot | PROT_EXEC, @@ -559,6 +580,8 @@ TEST_F(proc_maps_race, test_maps_tearing_from_split) do { bool last_line_changed; bool first_line_changed;
unsigned long vma_start;unsigned long vma_end; ASSERT_TRUE(read_boundary_lines(self, &new_last_line, &new_first_line));@@ -595,6 +618,19 @@ TEST_F(proc_maps_race, test_maps_tearing_from_split) 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 */Typo ioclt -> ioctl.
I think a little misleading, we're just testing whether we find a VMA at mod_info->addr + self->page_size.
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));So I'd make things clearer here with a comment like:
We are mmap()'ing a distinct VMA over the start of a 3 page mapping, which will cause the first page to be unmapped, and we can observe two states: read | v |---------|------------------| | | | | A | B | or: | | | |---------|------------------| | v |----------------------------| | | | A | | | |----------------------------| If we see entries in /proc/$pid/maps it'll be: 7fa86aa15000-7fa86aa16000 rw-p 00000000 00:00 0 (A) 7fa86aa16000-7fa86aa18000 rw-p 00000000 00:00 0 (B) Or: 7fa86aa15000-7fa86aa18000 rw-p 00000000 00:00 0 (A) So we assert that the reported range is equivalent to one of these.Obviously you can mix this in where you feel it makes sense.
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);@@ -636,6 +672,9 @@ TEST_F(proc_maps_race, test_maps_tearing_from_resize) clock_gettime(CLOCK_MONOTONIC_COARSE, &start_ts); start_test_loop(&start_ts, self->verbose); do {
unsigned long vma_start;unsigned long vma_end;ASSERT_TRUE(read_boundary_lines(self, &new_last_line, &new_first_line)); /* Check if we read vmas after shrinking it */@@ -662,6 +701,16 @@ TEST_F(proc_maps_race, test_maps_tearing_from_resize) "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));Same comments as above.
/** 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));So I'd make things clearer here with a comment like:
We are shrinking and expanding a VMA from 1 page to 3 pages: read | v |---------| | | | A | | | |---------| | v |----------------------------| | | | A | | | |----------------------------| If we see entries in /proc/$pid/maps it'll be: 7fa86aa15000-7fa86aa16000 rw-p 00000000 00:00 0 (A) Or: 7fa86aa15000-7fa86aa18000 rw-p 00000000 00:00 0 (A) So we assert that the reported range is equivalent to one of these.
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);@@ -703,6 +752,9 @@ TEST_F(proc_maps_race, test_maps_tearing_from_remap) clock_gettime(CLOCK_MONOTONIC_COARSE, &start_ts); start_test_loop(&start_ts, self->verbose); do {
unsigned long vma_start;unsigned long vma_end;ASSERT_TRUE(read_boundary_lines(self, &new_last_line, &new_first_line)); /* Check if we read vmas after remapping it */@@ -729,6 +781,19 @@ TEST_F(proc_maps_race, test_maps_tearing_from_remap) "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));Same comments as above.
/** 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));Again be good to have more explanation here, similar comments to abov.
We are mremap()'ing the last page of the next VMA (B) into the midle of the current one (A) (using MREMAP_DONTUNMAP leaving the last page of the original VMA zapped but in place: read | v R/W R/O |----------------------------| |------------------.---------| | | | . | | A | | B . | | | | . | |----------------------------| |------------------.---------| This will unmap the middle of A, splitting it in two, before placing a copy of B there (Which has different prot bits than A): | v R/W R/O R/W R/O |---------|---------|--------| |----------------------------| | | | | | | | A1 | B2 | A2 | | B | | | | | | | |---------|---------|--------| |----------------------------| But then we 'patch' B2 back to R/W prot bits, causing B2 to get merged: | v R/W R/O |----------------------------| |----------------------------| | | | | | A | | B | | | | | |----------------------------| |----------------------------| If we see entries in /proc/$pid/maps it'll be: 7fa86aa15000-7fa86aa18000 rw-p 00000000 00:00 0 (A) 7fa86aa19000-7fa86aa20000 r--p 00000000 00:00 0 (B) Or: 7fa86aa15000-7fa86aa16000 rw-p 00000000 00:00 0 (A1) 7fa86aa16000-7fa86aa17000 r--p 00000000 00:00 0 (B2) 7fa86aa17000-7fa86aa18000 rw-p 00000000 00:00 0 (A3) 7fa86aa19000-7fa86aa20000 r--p 00000000 00:00 0 (B) We are always examining the first line, so we simply assert that this remains in place and we observe 1 page or 3 pages.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.50.1.703.g449372360f-goog