[LTP] [PATCH v8] mremap07.c: New test for mremap() with MREMAP_DONTUNMAP
Li Wang
wangli.ahau@gmail.com
Mon Apr 20 12:12:19 CEST 2026
Hi Wei,
This is an excellent test design, but still few places need
refactoring in my opinion.
Wei Gao via ltp <ltp@lists.linux.it> wrote:
> --- a/configure.ac
> +++ b/configure.ac
> @@ -46,6 +46,7 @@ AC_CHECK_DECLS([MADV_MERGEABLE],,,[#include <sys/mman.h>])
> AC_CHECK_DECLS([NFTA_CHAIN_ID, NFTA_VERDICT_CHAIN_ID],,,[#include <linux/netfilter/nf_tables.h>])
> AC_CHECK_DECLS([PR_CAPBSET_DROP, PR_CAPBSET_READ],,,[#include <sys/prctl.h>])
> AC_CHECK_DECLS([SEM_STAT_ANY],,,[#include <sys/sem.h>])
> +AC_CHECK_DECLS([MREMAP_DONTUNMAP],,,[#include <linux/mman.h>])
I didn't go through all the previous discussion, but can we define
MREMAP_DONTUNMAP in the lapi/mmap.h then remove the .min_kver
and TST_TEST_TCONF?
> +++ b/testcases/kernel/syscalls/mremap/mremap07.c
...
> +#define _GNU_SOURCE
> +#include <poll.h>
> +#include <pthread.h>
> +#include <linux/mman.h>
> +
> +#include "tst_test.h"
> +#include "tst_safe_pthread.h"
> +#include "lapi/userfaultfd.h"
> +#include "config.h"
> +
> +#if HAVE_DECL_MREMAP_DONTUNMAP
> +
> +static int page_size;
> +static int uffd = -1;
> +static char *fault_addr;
> +static char *new_remap_addr;
The two variables are not clear enough, I would suggest using
old_addr, new_addr directly.
> +static const char *test_string = "Hello, world! This is a test string that fills up a page.";
We can just define in macro:
#define TEST_STRING_A "ABCD"
#define TEST_STRING_B "EFGH"
> +static void setup(void)
> +{
> + page_size = getpagesize();
> + struct uffdio_api uffdio_api;
> + struct uffdio_register uffdio_register;
> +
> + uffd = SAFE_USERFAULTFD(O_CLOEXEC | O_NONBLOCK, true);
> +
> + uffdio_api.api = UFFD_API;
> + uffdio_api.features = 0;
> + SAFE_IOCTL(uffd, UFFDIO_API, &uffdio_api);
> +
> + fault_addr = SAFE_MMAP(NULL, page_size,
> + PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
When we need indentation it's best to format it via "=" (if you use vim:).
And make the newline start from the first "(" position in a function.
> +
> + tst_res(TINFO, "Original mapping created at %p", (void *)fault_addr);
> +
> + strcpy(fault_addr, "ABCD");
> +
> + uffdio_register.range.start = (unsigned long)fault_addr;
> + uffdio_register.range.len = page_size;
> + uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
> + SAFE_IOCTL(uffd, UFFDIO_REGISTER, &uffdio_register);
> +}
> +
...
> +static void run(void)
> +{
> + pthread_t handler_thread;
> +
> + SAFE_PTHREAD_CREATE(&handler_thread, NULL,
> + fault_handler_thread, NULL);
> +
> + new_remap_addr = mremap(fault_addr, page_size, page_size,
> + MREMAP_DONTUNMAP | MREMAP_MAYMOVE, NULL);
> +
> + if (new_remap_addr == MAP_FAILED)
> + tst_brk(TBROK | TERRNO, "mremap failed");
> +
> + tst_res(TINFO, "New mapping created at %p", (void *)new_remap_addr);
> +
I strongly suggest verifying content of thenew_addr right after the
page migration
to ensure the PTEs were moved correctly without data loss.
TST_EXP_EQ_STR(new_addr, "ABCD");
> + strcpy(new_remap_addr, test_string);
> +
> + TST_CHECKPOINT_WAKE(0);
> +
> + tst_res(TINFO, "Main thread accessing old address %p to trigger fault",
> + (void *)fault_addr);
> +
> + (void)*(volatile char *)fault_addr;
> +
> + SAFE_PTHREAD_JOIN(handler_thread, NULL);
> +
> + TST_EXP_EQ_STR(fault_addr, test_string);
> +
> + SAFE_MUNMAP(new_remap_addr, page_size);
Add strcpy(old_addr, TEST_STRING_A); to support -i N.
Below is a diff version base on your patch:
--- a/testcases/kernel/syscalls/mremap/mremap07.c
+++ b/testcases/kernel/syscalls/mremap/mremap07.c
@@ -25,10 +25,11 @@
static int page_size;
static int uffd = -1;
-static char *fault_addr;
-static char *new_remap_addr;
+static char *old_addr;
+static char *new_addr;
-static const char *test_string = "Hello, world! This is a test string
that fills up a page.";
+#define TEST_STRING_A "ABCD"
+#define TEST_STRING_B "EFGH"
static void *fault_handler_thread(void *arg LTP_ATTRIBUTE_UNUSED)
{
@@ -55,13 +56,13 @@ static void *fault_handler_thread(void *arg
LTP_ATTRIBUTE_UNUSED)
if (msg.event != UFFD_EVENT_PAGEFAULT)
tst_brk(TBROK, "Received unexpected UFFD_EVENT: %d", msg.event);
- if ((char *)msg.arg.pagefault.address != fault_addr)
+ if ((char *)msg.arg.pagefault.address != old_addr)
tst_brk(TBROK, "Page fault on unexpected address: %p",
(void *)msg.arg.pagefault.address);
tst_res(TINFO, "Userfaultfd handler caught a page fault at
%p", (void *)msg.arg.pagefault.address);
- uffdio_copy.src = (unsigned long)new_remap_addr;
- uffdio_copy.dst = (unsigned long)fault_addr;
+ uffdio_copy.src = (unsigned long)new_addr;
+ uffdio_copy.dst = (unsigned long)old_addr;
uffdio_copy.len = page_size;
uffdio_copy.mode = 0;
uffdio_copy.copy = 0;
@@ -74,24 +75,26 @@ static void *fault_handler_thread(void *arg
LTP_ATTRIBUTE_UNUSED)
static void setup(void)
{
- page_size = getpagesize();
struct uffdio_api uffdio_api;
struct uffdio_register uffdio_register;
+ page_size = getpagesize();
uffd = SAFE_USERFAULTFD(O_CLOEXEC | O_NONBLOCK, true);
uffdio_api.api = UFFD_API;
uffdio_api.features = 0;
SAFE_IOCTL(uffd, UFFDIO_API, &uffdio_api);
- fault_addr = SAFE_MMAP(NULL, page_size,
- PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ old_addr = SAFE_MMAP(NULL, page_size,
+ PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS,
+ -1, 0);
- tst_res(TINFO, "Original mapping created at %p", (void *)fault_addr);
+ tst_res(TINFO, "Original mapping created at %p", (void *)old_addr);
- strcpy(fault_addr, "ABCD");
+ strcpy(old_addr, TEST_STRING_A);
- uffdio_register.range.start = (unsigned long)fault_addr;
+ uffdio_register.range.start = (unsigned long)old_addr;
uffdio_register.range.len = page_size;
uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
SAFE_IOCTL(uffd, UFFDIO_REGISTER, &uffdio_register);
@@ -99,11 +102,11 @@ static void setup(void)
static void cleanup(void)
{
- if (new_remap_addr && new_remap_addr != MAP_FAILED)
- SAFE_MUNMAP(new_remap_addr, page_size);
+ if (new_addr && new_addr != MAP_FAILED)
+ SAFE_MUNMAP(new_addr, page_size);
- if (fault_addr && fault_addr != MAP_FAILED)
- SAFE_MUNMAP(fault_addr, page_size);
+ if (old_addr && old_addr != MAP_FAILED)
+ SAFE_MUNMAP(old_addr, page_size);
if (uffd != -1)
SAFE_CLOSE(uffd);
@@ -111,34 +114,36 @@ static void cleanup(void)
static void run(void)
{
+ new_addr = NULL;
pthread_t handler_thread;
SAFE_PTHREAD_CREATE(&handler_thread, NULL,
- fault_handler_thread, NULL);
+ fault_handler_thread, NULL);
- new_remap_addr = mremap(fault_addr, page_size, page_size,
- MREMAP_DONTUNMAP | MREMAP_MAYMOVE, NULL);
+ new_addr = mremap(old_addr, page_size, page_size,
+ MREMAP_DONTUNMAP | MREMAP_MAYMOVE, NULL);
- if (new_remap_addr == MAP_FAILED)
+ if (new_addr == MAP_FAILED)
tst_brk(TBROK | TERRNO, "mremap failed");
- tst_res(TINFO, "New mapping created at %p", (void *)new_remap_addr);
+ tst_res(TINFO, "New mapping created at %p", (void *)new_addr);
- strcpy(new_remap_addr, test_string);
+ TST_EXP_EQ_STR(new_addr, TEST_STRING_A);
+ strcpy(new_addr, TEST_STRING_B);
TST_CHECKPOINT_WAKE(0);
tst_res(TINFO, "Main thread accessing old address %p to trigger fault",
- (void *)fault_addr);
+ (void *)old_addr);
- (void)*(volatile char *)fault_addr;
+ (void)*(volatile char *)old_addr;
SAFE_PTHREAD_JOIN(handler_thread, NULL);
- TST_EXP_EQ_STR(fault_addr, test_string);
+ TST_EXP_EQ_STR(old_addr, TEST_STRING_B);
- SAFE_MUNMAP(new_remap_addr, page_size);
- new_remap_addr = NULL;
+ SAFE_MUNMAP(new_addr, page_size);
+ strcpy(old_addr, TEST_STRING_A);
}
static struct tst_test test = {
--
Regards,
Li Wang
More information about the ltp
mailing list