Re: [LTP] [PATCH v2] userfaultfd: Add new test using UFFDIO_CONTINUE

[Petr Vorel <pvorel@suse.cz>]

Hi Ricardo,

> We test it with UFFD_PAGEFAULT_FLAG_MINOR.
Thanks, minor things below.

...
> diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd07.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd07.c
> new file mode 100644
> index 000000000..644e7724e
> --- /dev/null
> +++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd07.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2026 SUSE LLC
> + * Author: Ricardo Branco <rbranco@suse.com>
> + */
> +
> +/*\
> + * Force a pagefault event and handle it using :manpage:`userfaultfd(2)`
> + * from a different thread testing UFFDIO_CONTINUE.
> + */
> +
> +#include "config.h"
> +#include <poll.h>
> +#include <unistd.h>
> +#include "tst_test.h"
> +#include "tst_safe_macros.h"
> +#include "tst_safe_prw.h"
> +#include "tst_safe_pthread.h"
> +#include "lapi/memfd.h"
> +#include "lapi/userfaultfd.h"
> +#include "lapi/syscalls.h"
> +
> +static long page_size;
> +static char *page;
> +static int uffd = -1;
> +static int memfd = -1;
> +
> +static int sys_memfd_create(const char *name, unsigned int flags)
> +{
> +	return tst_syscall(__NR_memfd_create, name, flags);
We have this in testcases/kernel/syscalls/memfd_create/memfd_create_common.c.
Moving it as static inline into include/lapi/memfd.h would help to DRY.

> +}
> +
> +static void set_pages(void)
> +{
> +	char ch = 'A';
> +
> +	page_size = sysconf(_SC_PAGE_SIZE);
Maybe SAFE_SYSCONF(_SC_PAGE_SIZE) or because it should be safe:
getpagesize();

> +
> +	memfd = sys_memfd_create("ltp-uffd-continue", MFD_CLOEXEC);
> +	if (memfd < 0)
> +		tst_brk(TBROK | TERRNO, "memfd_create failed");
> +
> +	SAFE_FTRUNCATE(memfd, page_size);
> +
> +	/*
> +	 * Populate page cache so that after MADV_DONTNEED the next access
> +	 * can generate a MINOR fault rather than a MISSING fault.
> +	 */
> +	SAFE_PWRITE(1, memfd, &ch, 1, 0);
> +
> +	page = SAFE_MMAP(NULL, page_size, PROT_READ, MAP_SHARED, memfd, 0);
> +}
> +
> +static void reset_pages(void)
> +{
> +	if (page) {
> +		SAFE_MUNMAP(page, page_size);
> +		page = NULL;
> +	}
> +
> +	if (memfd != -1) {
> +		SAFE_CLOSE(memfd);
> +		memfd = -1;
This is not needed, see include/tst_safe_macros.h

#define SAFE_CLOSE(fd) do { \
		safe_close(__FILE__, __LINE__, NULL, (fd)); \
		fd = -1; \
	} while (0)

> +	}
> +
> +	if (uffd != -1) {
> +		SAFE_CLOSE(uffd);
> +		uffd = -1;
And here.

> +	}
> +}
> +
> +static void *handle_thread(void *arg LTP_ATTRIBUTE_UNUSED)
> +{
> +	static struct uffd_msg msg;
> +	struct uffdio_continue uffdio_continue = {};
> +	struct pollfd pollfd;
> +	int nready;
> +	char z = 'Z';
> +
> +	pollfd.fd = uffd;
> +	pollfd.events = POLLIN;
> +	nready = poll(&pollfd, 1, -1);
> +	if (nready == -1)
> +		tst_brk(TBROK | TERRNO, "Error on poll");
> +
> +	SAFE_READ(1, uffd, &msg, sizeof(msg));
> +
> +	if (msg.event != UFFD_EVENT_PAGEFAULT)
> +		tst_brk(TFAIL, "Received unexpected UFFD_EVENT %d", msg.event);
> +
> +	if (!(msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_MINOR)) {
> +		tst_brk(TBROK, "expected MINOR fault, got flags=0x%llx",
> +			(unsigned long long)msg.arg.pagefault.flags);
> +	}
> +
> +	/* Update the shmem page in page cache before resuming the fault. */
> +	SAFE_PWRITE(1, memfd, &z, 1, 0);
> +
> +	uffdio_continue.range.start =
> +		msg.arg.pagefault.address & ~((unsigned long)page_size - 1);
> +	uffdio_continue.range.len = page_size;
> +
> +	SAFE_IOCTL(uffd, UFFDIO_CONTINUE, &uffdio_continue);
> +
> +	SAFE_CLOSE(uffd);
> +	return NULL;
> +}
> +
> +static void run(void)
> +{
> +	pthread_t thr;
> +	struct uffdio_api uffdio_api = {};
> +	struct uffdio_register uffdio_register;
> +
> +	set_pages();
Maybe have it as setup function instead of calling it manually at the beginning of run()?
I'm not sure if it justifies, but remember possible running -i1000.

THe rest LGTM.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp