Re: [PATCH v3] selftests/mm: Fix ARM related issue with fork after pthread_create

[Muhammad Usama Anjum]

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@xxxxxxxxxx>
> [edliaw: Refactored to use atomic_bool]
> Signed-off-by: Edward Liaw <edliaw@xxxxxxxxxx>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@xxxxxxxxxxxxx>

> ---
> 
> 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