linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] selftests/mm: Fix ARM related issue with fork after pthread_create
@ 2024-03-25 19:40 Edward Liaw
  2024-03-25 20:04 ` Andrew Morton
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Edward Liaw @ 2024-03-25 19:40 UTC (permalink / raw)
  To: linux-kernel, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Morton,
	Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Jann Horn
  Cc: linux-kselftest, kernel-team, Edward Liaw, Lokesh Gidra, bpf,
	netdev, linux-mm, llvm

Following issue was observed while running the uffd-unit-tests selftest
on ARM devices. On x86_64 no issues were detected:

pthread_create followed by fork caused deadlock in certain cases
wherein fork required some work to be completed by the created thread.
Used synchronization to ensure that created thread's start function has
started before invoking fork.

Signed-off-by: Lokesh Gidra <lokeshgidra@google.com>
[edliaw: Refactored to use atomic_bool]
Signed-off-by: Edward Liaw <edliaw@google.com>
---

v2: restored accidentally removed uffd_test_case_ops when merging
v3: fixed commit subject to use selftests/mm prefix

 tools/testing/selftests/mm/uffd-common.c     |  3 +++
 tools/testing/selftests/mm/uffd-common.h     |  2 ++
 tools/testing/selftests/mm/uffd-unit-tests.c | 10 ++++++++++
 3 files changed, 15 insertions(+)

diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/selftests/mm/uffd-common.c
index b0ac0ec2356d..7ad6ba660c7d 100644
--- a/tools/testing/selftests/mm/uffd-common.c
+++ b/tools/testing/selftests/mm/uffd-common.c
@@ -18,6 +18,7 @@ bool test_uffdio_wp = true;
 unsigned long long *count_verify;
 uffd_test_ops_t *uffd_test_ops;
 uffd_test_case_ops_t *uffd_test_case_ops;
+atomic_bool ready_for_fork;

 static int uffd_mem_fd_create(off_t mem_size, bool hugetlb)
 {
@@ -518,6 +519,8 @@ void *uffd_poll_thread(void *arg)
 	pollfd[1].fd = pipefd[cpu*2];
 	pollfd[1].events = POLLIN;

+	ready_for_fork = true;
+
 	for (;;) {
 		ret = poll(pollfd, 2, -1);
 		if (ret <= 0) {
diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h
index cb055282c89c..cc5629c3d2aa 100644
--- a/tools/testing/selftests/mm/uffd-common.h
+++ b/tools/testing/selftests/mm/uffd-common.h
@@ -32,6 +32,7 @@
 #include <inttypes.h>
 #include <stdint.h>
 #include <sys/random.h>
+#include <stdatomic.h>

 #include "../kselftest.h"
 #include "vm_util.h"
@@ -103,6 +104,7 @@ extern bool map_shared;
 extern bool test_uffdio_wp;
 extern unsigned long long *count_verify;
 extern volatile bool test_uffdio_copy_eexist;
+extern atomic_bool ready_for_fork;

 extern uffd_test_ops_t anon_uffd_test_ops;
 extern uffd_test_ops_t shmem_uffd_test_ops;
diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
index 2b9f8cc52639..4a48dc617c6b 100644
--- a/tools/testing/selftests/mm/uffd-unit-tests.c
+++ b/tools/testing/selftests/mm/uffd-unit-tests.c
@@ -775,6 +775,8 @@ static void uffd_sigbus_test_common(bool wp)
 	char c;
 	struct uffd_args args = { 0 };

+	ready_for_fork = false;
+
 	fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);

 	if (uffd_register(uffd, area_dst, nr_pages * page_size,
@@ -790,6 +792,9 @@ static void uffd_sigbus_test_common(bool wp)
 	if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
 		err("uffd_poll_thread create");

+	while (!ready_for_fork)
+		; /* Wait for the poll_thread to start executing before forking */
+
 	pid = fork();
 	if (pid < 0)
 		err("fork");
@@ -829,6 +834,8 @@ static void uffd_events_test_common(bool wp)
 	char c;
 	struct uffd_args args = { 0 };

+	ready_for_fork = false;
+
 	fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
 	if (uffd_register(uffd, area_dst, nr_pages * page_size,
 			  true, wp, false))
@@ -838,6 +845,9 @@ static void uffd_events_test_common(bool wp)
 	if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
 		err("uffd_poll_thread create");

+	while (!ready_for_fork)
+		; /* Wait for the poll_thread to start executing before forking */
+
 	pid = fork();
 	if (pid < 0)
 		err("fork");
--
2.44.0.396.g6e790dbe36-goog


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

* Re: [PATCH v3] selftests/mm: Fix ARM related issue with fork after pthread_create
  2024-03-25 19:40 [PATCH v3] selftests/mm: Fix ARM related issue with fork after pthread_create Edward Liaw
@ 2024-03-25 20:04 ` Andrew Morton
  2024-03-29  0:31 ` patchwork-bot+netdevbpf
  2024-03-29 19:54 ` Muhammad Usama Anjum
  2 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2024-03-25 20:04 UTC (permalink / raw)
  To: Edward Liaw
  Cc: linux-kernel, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	Jann Horn, linux-kselftest, kernel-team, Lokesh Gidra, bpf,
	netdev, linux-mm, llvm, Peter Xu

On Mon, 25 Mar 2024 19:40:52 +0000 Edward Liaw <edliaw@google.com> wrote:

> Following issue was observed while running the uffd-unit-tests selftest
> on ARM devices. On x86_64 no issues were detected:
> 
> pthread_create followed by fork caused deadlock in certain cases
> wherein fork required some work to be completed by the created thread.
> Used synchronization to ensure that created thread's start function has
> started before invoking fork.

hm, you cc'ed the whole world apart from peterx.  Fixed.

> Signed-off-by: Lokesh Gidra <lokeshgidra@google.com>
> [edliaw: Refactored to use atomic_bool]
> Signed-off-by: Edward Liaw <edliaw@google.com>

I'll add cc:stable.  For which a Fixes: is desirable.  I used
760aee0b71e3 ("selftests/mm: add tests for RO pinning vs fork()"),
please check that.


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

* Re: [PATCH v3] selftests/mm: Fix ARM related issue with fork after pthread_create
  2024-03-25 19:40 [PATCH v3] selftests/mm: Fix ARM related issue with fork after pthread_create Edward Liaw
  2024-03-25 20:04 ` Andrew Morton
@ 2024-03-29  0:31 ` patchwork-bot+netdevbpf
  2024-03-29 19:54 ` Muhammad Usama Anjum
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-03-29  0:31 UTC (permalink / raw)
  To: Edward Liaw
  Cc: linux-kernel, ast, daniel, john.fastabend, andrii, martin.lau,
	eddyz87, song, yonghong.song, kpsingh, sdf, haoluo, jolsa, davem,
	edumazet, kuba, pabeni, akpm, shuah, nathan, ndesaulniers, morbo,
	justinstitt, jannh, linux-kselftest, kernel-team, lokeshgidra,
	bpf, netdev, linux-mm, llvm

Hello:

This patch was applied to netdev/net.git (main)
by Andrew Morton <akpm@linux-foundation.org>:

On Mon, 25 Mar 2024 19:40:52 +0000 you wrote:
> Following issue was observed while running the uffd-unit-tests selftest
> on ARM devices. On x86_64 no issues were detected:
> 
> pthread_create followed by fork caused deadlock in certain cases
> wherein fork required some work to be completed by the created thread.
> Used synchronization to ensure that created thread's start function has
> started before invoking fork.
> 
> [...]

Here is the summary with links:
  - [v3] selftests/mm: Fix ARM related issue with fork after pthread_create
    https://git.kernel.org/netdev/net/c/8c864371b2a1

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] 4+ messages in thread

* Re: [PATCH v3] selftests/mm: Fix ARM related issue with fork after pthread_create
  2024-03-25 19:40 [PATCH v3] selftests/mm: Fix ARM related issue with fork after pthread_create Edward Liaw
  2024-03-25 20:04 ` Andrew Morton
  2024-03-29  0:31 ` patchwork-bot+netdevbpf
@ 2024-03-29 19:54 ` Muhammad Usama Anjum
  2 siblings, 0 replies; 4+ messages in thread
From: Muhammad Usama Anjum @ 2024-03-29 19:54 UTC (permalink / raw)
  To: Edward Liaw, linux-kernel, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Morton,
	Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Jann Horn
  Cc: Muhammad Usama Anjum, linux-kselftest, kernel-team, Lokesh Gidra,
	bpf, netdev, linux-mm, llvm

On 3/26/24 12:40 AM, Edward Liaw wrote:
> Following issue was observed while running the uffd-unit-tests selftest
> on ARM devices. On x86_64 no issues were detected:
> 
> pthread_create followed by fork caused deadlock in certain cases
> wherein fork required some work to be completed by the created thread.
> Used synchronization to ensure that created thread's start function has
> started before invoking fork.
> 
> Signed-off-by: Lokesh Gidra <lokeshgidra@google.com>
> [edliaw: Refactored to use atomic_bool]
> Signed-off-by: Edward Liaw <edliaw@google.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

> ---
> 
> v2: restored accidentally removed uffd_test_case_ops when merging
> v3: fixed commit subject to use selftests/mm prefix
> 
>  tools/testing/selftests/mm/uffd-common.c     |  3 +++
>  tools/testing/selftests/mm/uffd-common.h     |  2 ++
>  tools/testing/selftests/mm/uffd-unit-tests.c | 10 ++++++++++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/selftests/mm/uffd-common.c
> index b0ac0ec2356d..7ad6ba660c7d 100644
> --- a/tools/testing/selftests/mm/uffd-common.c
> +++ b/tools/testing/selftests/mm/uffd-common.c
> @@ -18,6 +18,7 @@ bool test_uffdio_wp = true;
>  unsigned long long *count_verify;
>  uffd_test_ops_t *uffd_test_ops;
>  uffd_test_case_ops_t *uffd_test_case_ops;
> +atomic_bool ready_for_fork;
> 
>  static int uffd_mem_fd_create(off_t mem_size, bool hugetlb)
>  {
> @@ -518,6 +519,8 @@ void *uffd_poll_thread(void *arg)
>  	pollfd[1].fd = pipefd[cpu*2];
>  	pollfd[1].events = POLLIN;
> 
> +	ready_for_fork = true;
> +
>  	for (;;) {
>  		ret = poll(pollfd, 2, -1);
>  		if (ret <= 0) {
> diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h
> index cb055282c89c..cc5629c3d2aa 100644
> --- a/tools/testing/selftests/mm/uffd-common.h
> +++ b/tools/testing/selftests/mm/uffd-common.h
> @@ -32,6 +32,7 @@
>  #include <inttypes.h>
>  #include <stdint.h>
>  #include <sys/random.h>
> +#include <stdatomic.h>
> 
>  #include "../kselftest.h"
>  #include "vm_util.h"
> @@ -103,6 +104,7 @@ extern bool map_shared;
>  extern bool test_uffdio_wp;
>  extern unsigned long long *count_verify;
>  extern volatile bool test_uffdio_copy_eexist;
> +extern atomic_bool ready_for_fork;
> 
>  extern uffd_test_ops_t anon_uffd_test_ops;
>  extern uffd_test_ops_t shmem_uffd_test_ops;
> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> index 2b9f8cc52639..4a48dc617c6b 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -775,6 +775,8 @@ static void uffd_sigbus_test_common(bool wp)
>  	char c;
>  	struct uffd_args args = { 0 };
> 
> +	ready_for_fork = false;
> +
>  	fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
> 
>  	if (uffd_register(uffd, area_dst, nr_pages * page_size,
> @@ -790,6 +792,9 @@ static void uffd_sigbus_test_common(bool wp)
>  	if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
>  		err("uffd_poll_thread create");
> 
> +	while (!ready_for_fork)
> +		; /* Wait for the poll_thread to start executing before forking */
> +
>  	pid = fork();
>  	if (pid < 0)
>  		err("fork");
> @@ -829,6 +834,8 @@ static void uffd_events_test_common(bool wp)
>  	char c;
>  	struct uffd_args args = { 0 };
> 
> +	ready_for_fork = false;
> +
>  	fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
>  	if (uffd_register(uffd, area_dst, nr_pages * page_size,
>  			  true, wp, false))
> @@ -838,6 +845,9 @@ static void uffd_events_test_common(bool wp)
>  	if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
>  		err("uffd_poll_thread create");
> 
> +	while (!ready_for_fork)
> +		; /* Wait for the poll_thread to start executing before forking */
> +
>  	pid = fork();
>  	if (pid < 0)
>  		err("fork");
> --
> 2.44.0.396.g6e790dbe36-goog
> 
> 

-- 
BR,
Muhammad Usama Anjum

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

end of thread, other threads:[~2024-03-29 19:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-25 19:40 [PATCH v3] selftests/mm: Fix ARM related issue with fork after pthread_create Edward Liaw
2024-03-25 20:04 ` Andrew Morton
2024-03-29  0:31 ` patchwork-bot+netdevbpf
2024-03-29 19:54 ` Muhammad Usama Anjum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).