[LTP] [PATCH v2] userfaultfd: Minor fixes

[LTP] [PATCH v2] userfaultfd: Minor fixes

[Ricardo Branco]

- Use POSIX semantics for thread function
- Set cleanup to call reset_pages

Signed-off-by: Ricardo Branco <rbranco@suse.de>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
 .../kernel/syscalls/userfaultfd/userfaultfd01.c      | 10 +++++++---
 .../kernel/syscalls/userfaultfd/userfaultfd02.c      | 10 +++++++---
 .../kernel/syscalls/userfaultfd/userfaultfd03.c      | 12 ++++++++----
 .../kernel/syscalls/userfaultfd/userfaultfd04.c      |  8 ++++++--
 .../kernel/syscalls/userfaultfd/userfaultfd05.c      | 10 +++++++---
 .../kernel/syscalls/userfaultfd/userfaultfd06.c      |  2 +-
 6 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
index 7368d3863..b481e5a15 100644
--- a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
+++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
@@ -53,11 +53,14 @@ static void set_pages(void)
 
 static void reset_pages(void)
 {
-	SAFE_MUNMAP(page, page_size);
-	SAFE_MUNMAP(copy_page, page_size);
+	if (page) {
+		SAFE_MUNMAP(page, page_size);
+		SAFE_MUNMAP(copy_page, page_size);
+		page = NULL;
+	}
 }
 
-static void *handle_thread(void)
+static void *handle_thread(void *arg LTP_ATTRIBUTE_UNUSED)
 {
 	static struct uffd_msg msg;
 	struct uffdio_copy uffdio_copy = {};
@@ -129,4 +132,5 @@ static struct tst_test test = {
 	.setup = setup,
 	.test = run,
 	.tcnt = ARRAY_SIZE(tcases),
+	.cleanup = reset_pages,
 };
diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd02.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd02.c
index 2fd5ba5d8..bedb5d080 100644
--- a/testcases/kernel/syscalls/userfaultfd/userfaultfd02.c
+++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd02.c
@@ -32,11 +32,14 @@ static void set_pages(void)
 
 static void reset_pages(void)
 {
-	SAFE_MUNMAP(page, page_size);
-	SAFE_MUNMAP(move_page, page_size);
+	if (page) {
+		SAFE_MUNMAP(page, page_size);
+		SAFE_MUNMAP(move_page, page_size);
+		page = NULL;
+	}
 }
 
-static void *handle_thread(void)
+static void *handle_thread(void *arg LTP_ATTRIBUTE_UNUSED)
 {
 	static struct uffd_msg msg;
 	struct uffdio_move uffdio_move = {};
@@ -94,4 +97,5 @@ static void run(void)
 static struct tst_test test = {
 	.test_all = run,
 	.min_kver = "6.8",
+	.cleanup = reset_pages,
 };
diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd03.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd03.c
index b65f39eca..463dfba0b 100644
--- a/testcases/kernel/syscalls/userfaultfd/userfaultfd03.c
+++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd03.c
@@ -57,11 +57,14 @@ static void set_pages(void)
 
 static void reset_pages(void)
 {
-	SAFE_MUNMAP(page, page_size);
-	SAFE_MUNMAP(copy_page, page_size);
+	if (page) {
+		SAFE_MUNMAP(page, page_size);
+		SAFE_MUNMAP(copy_page, page_size);
+		page = NULL;
+	}
 }
 
-static void *handle_thread(void)
+static void *handle_thread(void *arg LTP_ATTRIBUTE_UNUSED)
 {
 	static struct uffd_msg msg;
 	struct uffdio_copy uffdio_copy = {};
@@ -132,5 +135,6 @@ static struct tst_test test = {
 	.needs_kconfigs = (const char *[]) {
 		"CONFIG_USERFAULTFD=y",
 		NULL
-	}
+	},
+	.cleanup = reset_pages,
 };
diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd04.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd04.c
index 4eb811e45..3273d20cb 100644
--- a/testcases/kernel/syscalls/userfaultfd/userfaultfd04.c
+++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd04.c
@@ -30,10 +30,13 @@ static void set_pages(void)
 
 static void reset_pages(void)
 {
-	SAFE_MUNMAP(page, page_size);
+	if (page) {
+		SAFE_MUNMAP(page, page_size);
+		page = NULL;
+	}
 }
 
-static void *handle_thread(void)
+static void *handle_thread(void *arg LTP_ATTRIBUTE_UNUSED)
 {
 	static struct uffd_msg msg;
 	struct uffdio_zeropage uffdio_zeropage = {};
@@ -98,4 +101,5 @@ static void run(void)
 
 static struct tst_test test = {
 	.test_all = run,
+	.cleanup = reset_pages,
 };
diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd05.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd05.c
index e25a227cf..4434f9b9f 100644
--- a/testcases/kernel/syscalls/userfaultfd/userfaultfd05.c
+++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd05.c
@@ -33,10 +33,13 @@ static void set_pages(void)
 
 static void reset_pages(void)
 {
-	SAFE_MUNMAP(page, page_size);
+	if (page) {
+		SAFE_MUNMAP(page, page_size);
+		page = NULL;
+	}
 }
 
-static void *handle_thread(void)
+static void *handle_thread(void *arg LTP_ATTRIBUTE_UNUSED)
 {
 	static struct uffd_msg msg;
 	struct uffdio_writeprotect uffdio_writeprotect = {};
@@ -127,5 +130,6 @@ static struct tst_test test = {
 	.needs_kconfigs = (const char *[]) {
 		"CONFIG_HAVE_ARCH_USERFAULTFD_WP=y",
 		NULL
-	}
+	},
+	.cleanup = reset_pages,
 };
diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd06.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd06.c
index 5b1252c35..238556ba4 100644
--- a/testcases/kernel/syscalls/userfaultfd/userfaultfd06.c
+++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd06.c
@@ -58,7 +58,7 @@ static void reset_pages(void)
 	}
 }
 
-static void *handle_thread(void)
+static void *handle_thread(void *arg LTP_ATTRIBUTE_UNUSED)
 {
 	static struct uffd_msg msg;
 	struct uffdio_poison uffdio_poison = {};
-- 
2.53.0


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

Re: [LTP] [PATCH v2] userfaultfd: Minor fixes

[Andrea Cervesato via ltp]

Hi ricardo,

> +	if (page) {
> +		SAFE_MUNMAP(page, page_size);
> +		SAFE_MUNMAP(copy_page, page_size);
> +		page = NULL;
> +	}

This is gonna TBROK when copy_page is NULL. The same apply to all other
tests which are affected by the same bug.

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

Re: [LTP] [PATCH v2] userfaultfd: Minor fixes

[Ricardo Branco]

On 3/26/26 9:38 AM, Andrea Cervesato wrote:
> Hi ricardo,
>
>> +	if (page) {
>> +		SAFE_MUNMAP(page, page_size);
>> +		SAFE_MUNMAP(copy_page, page_size);
>> +		page = NULL;
>> +	}
> This is gonna TBROK when copy_page is NULL. The same apply to all other
> tests which are affected by the same bug.

Fixed in v3.

BTW, we have a minor difference in tests:

$ diff userfaultfd03.c userfaultfd06.c

<         tst_brk(TBROK | TERRNO, "Received unexpected UFFD_EVENT %d", 
msg.event);
---
 >         tst_brk(TFAIL, "Received unexpected UFFD_EVENT %d", msg.event);

I think we should have TFAIL, WDYT?

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

Re: [LTP] [PATCH v2] userfaultfd: Minor fixes

[Petr Vorel]

> On 3/26/26 9:38 AM, Andrea Cervesato wrote:
> > Hi ricardo,

> > > +	if (page) {
> > > +		SAFE_MUNMAP(page, page_size);
> > > +		SAFE_MUNMAP(copy_page, page_size);
> > > +		page = NULL;
> > > +	}
> > This is gonna TBROK when copy_page is NULL. The same apply to all other
> > tests which are affected by the same bug.

> Fixed in v3.

> BTW, we have a minor difference in tests:

> $ diff userfaultfd03.c userfaultfd06.c

> <         tst_brk(TBROK | TERRNO, "Received unexpected UFFD_EVENT %d",
> msg.event);
> ---
> >         tst_brk(TFAIL, "Received unexpected UFFD_EVENT %d", msg.event);

> I think we should have TFAIL, WDYT?

Yes, IMHO TFAIL.

Regards other cleanup: I again double checked if some test could be merged with
others but haven't found any.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2] userfaultfd: Minor fixes

[Andrea Cervesato via ltp]

> Yes, IMHO TFAIL.
> 

I guess we need to agree on the solution here, otherwise Ricard will be
puzzled :-)

I see TBROK as the right solution, since TFAIL should be used on the subject of
our tests. Also, if we receive the wrong event there's no purpose to continue
with the test because that means we have a kernel bug.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

Re: [LTP] [PATCH v2] userfaultfd: Minor fixes

[Petr Vorel]

> > Yes, IMHO TFAIL.

> I guess we need to agree on the solution here, otherwise Ricard will be
> puzzled :-)

Yeah, I'm sorry for the noise.

> I see TBROK as the right solution, since TFAIL should be used on the subject of
> our tests. Also, if we receive the wrong event there's no purpose to continue
> with the test because that means we have a kernel bug.

OK, tst_brk(TBROK | TERRNO is probably better (and we definitely want TERRNO).

I overlooked Ricardo suggests tst_brk(TFAIL) not tst_res(TFAIL).
The only concern I have is that we skip testing on userfaultfd01.c due tst_brk()
but it's probably quite rare.

Also, this is the problem when test have a lot of duplicity => tests diverge
sometimes in subtle details. But, similarly to fanotify tests, userfaultfd tests
are quite different that it's questionable if factoring out small parts improves
things or complicate.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2] userfaultfd: Minor fixes

[Andrea Cervesato via ltp]

Hi Ricardo,

> 
> On 3/26/26 9:38 AM, Andrea Cervesato wrote:
> > Hi ricardo,
> >
> >> +	if (page) {
> >> +		SAFE_MUNMAP(page, page_size);
> >> +		SAFE_MUNMAP(copy_page, page_size);
> >> +		page = NULL;
> >> +	}
> > This is gonna TBROK when copy_page is NULL. The same apply to all other
> > tests which are affected by the same bug.
> 
> Fixed in v3.
> 
> BTW, we have a minor difference in tests:
> 
> $ diff userfaultfd03.c userfaultfd06.c
> 
> <         tst_brk(TBROK | TERRNO, "Received unexpected UFFD_EVENT %d", 
> msg.event);
> ---
>  >         tst_brk(TFAIL, "Received unexpected UFFD_EVENT %d", msg.event);
> 
> I think we should have TFAIL, WDYT?

It depends on the subject of our test and both tests are actually testing
other features than the specific uffd_msg attributes when an event is
recevied. For this reason I would keep TBROK in both of them.

I see that also userfaultfd05 has the same issue. I don't remember the full
conversation before merging the patches, but if none mentioned a reason in
there, the tests will need to be adapted.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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