Received-SPF: pass (google.com: domain of linux-kernel+bounces-125368-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <c54bae70-3655-4188-aad7-358bb7897c3c@collabora.com>
Date: Sat, 30 Mar 2024 00:54:20 +0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>,
 linux-kselftest@vger.kernel.org, kernel-team@android.com,
 Lokesh Gidra <lokeshgidra@google.com>, bpf@vger.kernel.org,
 netdev@vger.kernel.org, linux-mm@kvack.org, llvm@lists.linux.dev
Subject: Re: [PATCH v3] selftests/mm: Fix ARM related issue with fork after
 pthread_create
To: Edward Liaw <edliaw@google.com>, linux-kernel@vger.kernel.org,
 Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>,
 John Fastabend <john.fastabend@gmail.com>,
 Andrii Nakryiko <andrii@kernel.org>, Martin KaFai Lau
 <martin.lau@linux.dev>, Eduard Zingerman <eddyz87@gmail.com>,
 Song Liu <song@kernel.org>, Yonghong Song <yonghong.song@linux.dev>,
 KP Singh <kpsingh@kernel.org>, Stanislav Fomichev <sdf@google.com>,
 Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
 "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>,
 Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
 Andrew Morton <akpm@linux-foundation.org>, Shuah Khan <shuah@kernel.org>,
 Nathan Chancellor <nathan@kernel.org>,
 Nick Desaulniers <ndesaulniers@google.com>, Bill Wendling
 <morbo@google.com>, Justin Stitt <justinstitt@google.com>,
 Jann Horn <jannh@google.com>
References: <20240325194100.775052-1-edliaw@google.com>
Content-Language: en-US
From: Muhammad Usama Anjum <usama.anjum@collabora.com>
In-Reply-To: <20240325194100.775052-1-edliaw@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

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