public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 v3 1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors
@ 2026-02-25 11:14 Sun Jian
  2026-02-26  3:53 ` Jakub Kicinski
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Sun Jian @ 2026-02-25 11:14 UTC (permalink / raw)
  To: Shuah Khan, Jakub Kicinski
  Cc: linux-kselftest, netdev, linux-kernel, Kees Cook,
	David S . Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
	Sun Jian

TEST_F() allocates and registers its struct __test_metadata via mmap()
inside its constructor, and only then assigns the
_##fixture_##test##_object pointer.

XFAIL_ADD() runs in a constructor too and reads
_##fixture_##test##_object to initialize xfail->test. If XFAIL_ADD runs
first, xfail->test can be NULL and the expected failure will be reported
as FAIL.

Use constructor priorities to ensure TEST_F registration runs before
XFAIL_ADD, without adding extra state or runtime lookups.

Fixes: 2709473c9386 ("selftests: kselftest_harness: support using xfail")
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
v3:
  - Move KSELFTEST_PRIO_* definitions above the TEST_SIGNAL() kdoc block.

v2:
  - Drop the lazy string-matching approach.
  - Use constructor priorities to enforce initialization order.
  Link:
    https://lore.kernel.org/all/20260224113859.12345-1-sun.jian.kdev@gmail.com/

v1:
  Link:
    https://lore.kernel.org/all/20260222111846.12345-1-sun.jian.kdev@gmail.com/
---
 tools/testing/selftests/kselftest_harness.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 16a119a4656c..4afaef01c22e 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -76,6 +76,9 @@ static inline void __kselftest_memset_safe(void *s, int c, size_t n)
 		memset(s, c, n);
 }
 
+#define KSELFTEST_PRIO_TEST_F  20000
+#define KSELFTEST_PRIO_XFAIL   20001
+
 #define TEST_TIMEOUT_DEFAULT 30
 
 /* Utilities exposed to the test definitions */
@@ -465,7 +468,7 @@ static inline void __kselftest_memset_safe(void *s, int c, size_t n)
 			fixture_name##_teardown(_metadata, self, variant); \
 	} \
 	static struct __test_metadata *_##fixture_name##_##test_name##_object; \
-	static void __attribute__((constructor)) \
+	static void __attribute__((constructor(KSELFTEST_PRIO_TEST_F))) \
 			_register_##fixture_name##_##test_name(void) \
 	{ \
 		struct __test_metadata *object = mmap(NULL, sizeof(*object), \
@@ -880,7 +883,7 @@ struct __test_xfail {
 		.fixture = &_##fixture_name##_fixture_object, \
 		.variant = &_##fixture_name##_##variant_name##_object, \
 	}; \
-	static void __attribute__((constructor)) \
+	static void __attribute__((constructor(KSELFTEST_PRIO_XFAIL))) \
 		_register_##fixture_name##_##variant_name##_##test_name##_xfail(void) \
 	{ \
 		_##fixture_name##_##variant_name##_##test_name##_xfail.test = \

base-commit: 7dff99b354601dd01829e1511711846e04340a69
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 v3 1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors
  2026-02-25 11:14 [PATCH v3 v3 1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors Sun Jian
@ 2026-02-26  3:53 ` Jakub Kicinski
  2026-03-05  8:46 ` [v3,v3,1/2] " Paolo Abeni
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-02-26  3:53 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Sun Jian, linux-kselftest, netdev, linux-kernel, Kees Cook,
	David S . Miller, Eric Dumazet, Paolo Abeni, Simon Horman

On Wed, 25 Feb 2026 19:14:50 +0800 Sun Jian wrote:
> TEST_F() allocates and registers its struct __test_metadata via mmap()
> inside its constructor, and only then assigns the
> _##fixture_##test##_object pointer.
> 
> XFAIL_ADD() runs in a constructor too and reads
> _##fixture_##test##_object to initialize xfail->test. If XFAIL_ADD runs
> first, xfail->test can be NULL and the expected failure will be reported
> as FAIL.
> 
> Use constructor priorities to ensure TEST_F registration runs before
> XFAIL_ADD, without adding extra state or runtime lookups.
> 
> Fixes: 2709473c9386 ("selftests: kselftest_harness: support using xfail")
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>

Hi Shuah!

Any preference which one of us takes this series?
No real preference here, just slight preference to ship it as 
a fix to 7.0 rather than via -next.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [v3,v3,1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors
  2026-02-25 11:14 [PATCH v3 v3 1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors Sun Jian
  2026-02-26  3:53 ` Jakub Kicinski
@ 2026-03-05  8:46 ` Paolo Abeni
  2026-03-05  9:09 ` [PATCH v3 v3 1/2] " Thomas Weißschuh
  2026-03-05 16:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 6+ messages in thread
From: Paolo Abeni @ 2026-03-05  8:46 UTC (permalink / raw)
  To: sun.jian.kdev
  Cc: kuba, shuah, horms, netdev, kees, linux-kselftest, davem,
	linux-kernel, edumazet, pabeni

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
selftests/harness: order TEST_F and XFAIL_ADD constructors

This commit fixes a constructor ordering race where XFAIL_ADD() could run
before TEST_F() and read a NULL _##fixture_##test##_object pointer. The
fix uses constructor priorities to ensure TEST_F runs first.

> TEST_F() allocates and registers its struct __test_metadata via mmap()
> inside its constructor, and only then assigns the
> _##fixture_##test##_object pointer.
>
> XFAIL_ADD() runs in a constructor too and reads
> _##fixture_##test##_object to initialize xfail->test. If XFAIL_ADD runs
> first, xfail->test can be NULL and the expected failure will be reported
> as FAIL.
>
> Use constructor priorities to ensure TEST_F registration runs before
> XFAIL_ADD, without adding extra state or runtime lookups.
>
> Fixes: 2709473c9386 ("selftests: kselftest_harness: support using xfail")

Should the Fixes: tag point to commit 24cf65a62266 instead?

The existing tag points to 2709473c9386 ("selftests: kselftest_harness:
support using xfail"), but that commit used static initialization
(.test = &object) which had no race condition.

The race was actually introduced by commit 24cf65a62266
("selftests/harness: Share _metadata between forked processes"), which
changed TEST_F to use constructor-based allocation and also changed
XFAIL_ADD to read the pointer value in its constructor. This created the
race where XFAIL_ADD could run before TEST_F and read NULL.

> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>

> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index 16a119a4656c..4afaef01c22e 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -76,6 +76,9 @@ static inline void __kselftest_memset_safe(void *s, int c, size_t n)
>  		memset(s, c, n);
>  }
>
> +#define KSELFTEST_PRIO_TEST_F  20000
> +#define KSELFTEST_PRIO_XFAIL   20001
> +
>  #define TEST_TIMEOUT_DEFAULT 30
>
>  /* Utilities exposed to the test definitions */
> @@ -465,7 +468,7 @@ static inline void __kselftest_memset_safe(void *s, int c, size_t n)
>  			fixture_name##_teardown(_metadata, self, variant); \
>  	} \
>  	static struct __test_metadata *_##fixture_name##_##test_name##_object; \
> -	static void __attribute__((constructor)) \
> +	static void __attribute__((constructor(KSELFTEST_PRIO_TEST_F))) \
>  			_register_##fixture_name##_##test_name(void) \
>  	{ \
>  		struct __test_metadata *object = mmap(NULL, sizeof(*object), \
> @@ -880,7 +883,7 @@ struct __test_xfail {
>  		.fixture = &_##fixture_name##_fixture_object, \
>  		.variant = &_##fixture_name##_##variant_name##_object, \
>  	}; \
> -	static void __attribute__((constructor)) \
> +	static void __attribute__((constructor(KSELFTEST_PRIO_XFAIL))) \
>  		_register_##fixture_name##_##variant_name##_##test_name##_xfail(void) \
>  	{ \
>  		_##fixture_name##_##variant_name##_##test_name##_xfail.test = \


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 v3 1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors
  2026-02-25 11:14 [PATCH v3 v3 1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors Sun Jian
  2026-02-26  3:53 ` Jakub Kicinski
  2026-03-05  8:46 ` [v3,v3,1/2] " Paolo Abeni
@ 2026-03-05  9:09 ` Thomas Weißschuh
  2026-03-05 16:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 6+ messages in thread
From: Thomas Weißschuh @ 2026-03-05  9:09 UTC (permalink / raw)
  To: Sun Jian
  Cc: Shuah Khan, Jakub Kicinski, linux-kselftest, netdev, linux-kernel,
	Kees Cook, David S . Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman

On Wed, Feb 25, 2026 at 07:14:50PM +0800, Sun Jian wrote:
> TEST_F() allocates and registers its struct __test_metadata via mmap()
> inside its constructor, and only then assigns the
> _##fixture_##test##_object pointer.
> 
> XFAIL_ADD() runs in a constructor too and reads
> _##fixture_##test##_object to initialize xfail->test. If XFAIL_ADD runs
> first, xfail->test can be NULL and the expected failure will be reported
> as FAIL.
> 
> Use constructor priorities to ensure TEST_F registration runs before
> XFAIL_ADD, without adding extra state or runtime lookups.
> 
> Fixes: 2709473c9386 ("selftests: kselftest_harness: support using xfail")
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
> v3:
>   - Move KSELFTEST_PRIO_* definitions above the TEST_SIGNAL() kdoc block.
> 
> v2:
>   - Drop the lazy string-matching approach.
>   - Use constructor priorities to enforce initialization order.
>   Link:
>     https://lore.kernel.org/all/20260224113859.12345-1-sun.jian.kdev@gmail.com/
> 
> v1:
>   Link:
>     https://lore.kernel.org/all/20260222111846.12345-1-sun.jian.kdev@gmail.com/
> ---
>  tools/testing/selftests/kselftest_harness.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

FYI there is a selftest for the harness in
tools/testing/selftests/kselftest_harness/harness-selftest.c .
You could update this with your usecase to make sure, that this functionality
keeps working.

(...)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 v3 1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors
  2026-02-25 11:14 [PATCH v3 v3 1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors Sun Jian
                   ` (2 preceding siblings ...)
  2026-03-05  9:09 ` [PATCH v3 v3 1/2] " Thomas Weißschuh
@ 2026-03-05 16:00 ` patchwork-bot+netdevbpf
  2026-03-06  2:47   ` sun jian
  3 siblings, 1 reply; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-03-05 16:00 UTC (permalink / raw)
  To: Sun Jian
  Cc: shuah, kuba, linux-kselftest, netdev, linux-kernel, kees, davem,
	edumazet, pabeni, horms

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 25 Feb 2026 19:14:50 +0800 you wrote:
> TEST_F() allocates and registers its struct __test_metadata via mmap()
> inside its constructor, and only then assigns the
> _##fixture_##test##_object pointer.
> 
> XFAIL_ADD() runs in a constructor too and reads
> _##fixture_##test##_object to initialize xfail->test. If XFAIL_ADD runs
> first, xfail->test can be NULL and the expected failure will be reported
> as FAIL.
> 
> [...]

Here is the summary with links:
  - [v3,v3,1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors
    https://git.kernel.org/netdev/net/c/6be268151426
  - [v3,v3,2/2] selftests: net: tun: don't abort XFAIL cases
    https://git.kernel.org/netdev/net/c/c952291593e5

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 v3 1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors
  2026-03-05 16:00 ` patchwork-bot+netdevbpf
@ 2026-03-06  2:47   ` sun jian
  0 siblings, 0 replies; 6+ messages in thread
From: sun jian @ 2026-03-06  2:47 UTC (permalink / raw)
  To: patchwork-bot+netdevbpf
  Cc: shuah, kuba, linux-kselftest, netdev, linux-kernel, kees, davem,
	edumazet, pabeni, horms

Hi Paolo, Thomas,
Thanks for the reviews.

Paolo: Agreed on the Fixes tag. I won't resend since the series has already
been applied to netdev/net.git.

Thomas: Will send a follow-up covering this XFAIL_ADD/TEST_F() ctor
ordering case.

Regards,
sun jian

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-06  2:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-25 11:14 [PATCH v3 v3 1/2] selftests/harness: order TEST_F and XFAIL_ADD constructors Sun Jian
2026-02-26  3:53 ` Jakub Kicinski
2026-03-05  8:46 ` [v3,v3,1/2] " Paolo Abeni
2026-03-05  9:09 ` [PATCH v3 v3 1/2] " Thomas Weißschuh
2026-03-05 16:00 ` patchwork-bot+netdevbpf
2026-03-06  2:47   ` sun jian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox