lkml.org 
[lkml]   [2023]   [Jan]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] selftest/vm: add mremap expand merge offset test
On 16.12.22 22:44, Lorenzo Stoakes wrote:
> Add a test to assert that we can mremap() and expand a mapping starting
> from an offset within an existing mapping. We unmap the last page in a 3
> page mapping to ensure that the remap should always succeed, before
> remapping from the 2nd page.
>
> This is additionally a regression test for the issue solved in "mm, mremap:
> fix mremap() expanding vma with addr inside vma" and confirmed to fail
> prior to the change and pass after it.
>
> Finally, this patch updates the existing mremap expand merge test to check
> error conditions and reduce code duplication between the two tests.
>
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> ---
> tools/testing/selftests/vm/mremap_test.c | 111 +++++++++++++++++++----
> 1 file changed, 93 insertions(+), 18 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c
> index 9496346973d4..28a17d4e8afd 100644
> --- a/tools/testing/selftests/vm/mremap_test.c
> +++ b/tools/testing/selftests/vm/mremap_test.c
> @@ -119,30 +119,19 @@ static unsigned long long get_mmap_min_addr(void)
> }
>
> /*
> - * This test validates that merge is called when expanding a mapping.
> - * Mapping containing three pages is created, middle page is unmapped
> - * and then the mapping containing the first page is expanded so that
> - * it fills the created hole. The two parts should merge creating
> - * single mapping with three pages.
> + * Using /proc/self/maps, assert that the specified address range is contained
> + * within a single mapping.
> */
> -static void mremap_expand_merge(unsigned long page_size)
> +static bool is_range_mapped(void *start, void *end)
> {
> - char *test_name = "mremap expand merge";
> FILE *fp;
> char *line = NULL;
> size_t len = 0;
> bool success = false;
> - char *start = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE,
> - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> -
> - munmap(start + page_size, page_size);
> - mremap(start, page_size, 2 * page_size, 0);
>
> fp = fopen("/proc/self/maps", "r");
> - if (fp == NULL) {
> - ksft_test_result_fail("%s\n", test_name);
> - return;
> - }
> + if (fp == NULL)
> + return false;

This is unexpected. It would be valuable to ksft_print_msg("[INFO] .."
something, indicating that we don't know because we cannot access that info.

ksft_print_msg("[INFO] Opening /proc/self/maps failed"


But I'd even suggest opening "/proc/self/maps" once in main() and
failing directly there. Then we don't have to worry about it here.

>
> while (getline(&line, &len, fp) != -1) {
> char *first = strtok(line, "- ");
> @@ -150,16 +139,101 @@ static void mremap_expand_merge(unsigned long page_size)
> char *second = strtok(NULL, "- ");
> void *second_val = (void *) strtol(second, NULL, 16);
>
> - if (first_val == start && second_val == start + 3 * page_size) {
> + if (first_val <= start && second_val >= end) {
> success = true;
> break;
> }
> }
> +
> + fclose(fp);
> + return success;
> +}
> +
> +/*
> + * This test validates that merge is called when expanding a mapping.
> + * Mapping containing three pages is created, middle page is unmapped
> + * and then the mapping containing the first page is expanded so that
> + * it fills the created hole. The two parts should merge creating
> + * single mapping with three pages.
> + */
> +static void mremap_expand_merge(unsigned long page_size)
> +{
> + char *test_name = "mremap expand merge";
> + bool success = false;
> + int errsv = 0;
> + char *remap;
> + char *start = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

I'd suggest


char *remap, *start;

start = mmap()
if (start == MAP_FAILED) { ...

to make this easier to read.

> +
> + if (start == MAP_FAILED) {
> + errsv = errno;
> + goto error;
> + }
> +
> + munmap(start + page_size, page_size);
> + remap = mremap(start, page_size, 2 * page_size, 0);
> + if (remap == MAP_FAILED) {
> + errsv = errno;
> + munmap(start, page_size);
> + munmap(start + 2 * page_size, page_size);
> + goto error;
> + }
> +
> + success = is_range_mapped(start, start + 3 * page_size);
> +
> + munmap(start, 3 * page_size);
> + goto out;
> +
> +error:
> + ksft_print_msg("Unexpected mapping/remapping error: %s\n",
> + strerror(errsv));

Please avoid the "error" label and just print proper errors directly at
the two callsites. Then, remove the "goto out".

> +out:
> + if (success)
> + ksft_test_result_pass("%s\n", test_name);
> + else
> + ksft_test_result_fail("%s\n", test_name);
> +}
> +
> +/*
> + * Similar to mremap_expand_merge() except instead of removing the middle page,
> + * we remove the last then attempt to remap offset from the second page. This
> + * should result in the mapping being restored to its former state.
> + */
> +static void mremap_expand_merge_offset(unsigned long page_size)
> +{
> +
> + char *test_name = "mremap expand merge offset";
> + bool success = false;
> + int errsv = 0;
> + char *remap;
> + char *start = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

Dito.

> +
> + if (start == MAP_FAILED) {
> + errsv = errno;
> + goto error;
> + }
> +
> + /* Unmap final page to ensure we have space to expand. */
> + munmap(start + 2 * page_size, page_size);
> + remap = mremap(start + page_size, page_size, 2 * page_size, 0);
> + if (remap == MAP_FAILED) {
> + errsv = errno;
> + munmap(start, 2 * page_size);
> + goto error;
> + }
> +
> + success = is_range_mapped(start, start + 3 * page_size);
> + goto out;
> +
> +error:
> + ksft_print_msg("Unexpected mapping/remapping error: %s\n",
> + strerror(errsv));

Dito.

> +out:


--
Thanks,

David / dhildenb
\
 
 \ /
  Last update: 2023-03-26 23:24    [W:0.067 / U:0.480 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site