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
