From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5DB52D3DEA2 for ; Fri, 18 Oct 2024 17:18:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D36AE6B0092; Fri, 18 Oct 2024 13:18:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CE2D46B0093; Fri, 18 Oct 2024 13:18:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BA9806B0095; Fri, 18 Oct 2024 13:18:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 98FD96B0092 for ; Fri, 18 Oct 2024 13:18:17 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 899491C4183 for ; Fri, 18 Oct 2024 17:18:03 +0000 (UTC) X-FDA: 82687381098.21.64E1E98 Received: from mail-ej1-f45.google.com (mail-ej1-f45.google.com [209.85.218.45]) by imf29.hostedemail.com (Postfix) with ESMTP id 5B8A5120013 for ; Fri, 18 Oct 2024 17:18:00 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ued0EiE2; spf=pass (imf29.hostedemail.com: domain of edliaw@google.com designates 209.85.218.45 as permitted sender) smtp.mailfrom=edliaw@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729271846; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=9d3KGHo0twHh3KK7tsU2WqukBG2HV/y2vfRLQQjxQEs=; b=mM/TokiOMntEDhLRoA2F0XDo7421/TisX2r++6GE9rFvG9KVZRVmvCtjckKKxtUZoHabDI mcjoV3sGnrV8ouDhS2gXy373gcbmGJMVFTfv4TV47axCeLdANVQFtvIyT2MBVFZsKq6oJt sHImZ7rh9fj1F5sCzLrl0iD8jecpDwo= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ued0EiE2; spf=pass (imf29.hostedemail.com: domain of edliaw@google.com designates 209.85.218.45 as permitted sender) smtp.mailfrom=edliaw@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729271846; a=rsa-sha256; cv=none; b=etlhPOu7r/y4EPqdLRYD5Wb4DUI18Wh93uG9QD6DnJJJG4Y54ddYlTVLH8eHVjlPIsMoiY k2e4gVf5vBdtPqeRglCwAfuAm3srwmZAHVxVY1YeQ1VPBzGIXRtJMT+rDEU6iJI4kUZnDC GsL/coZ4nC65F6dt/OfAkm58A1L7bUE= Received: by mail-ej1-f45.google.com with SMTP id a640c23a62f3a-a99f646ff1bso288402866b.2 for ; Fri, 18 Oct 2024 10:18:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1729271894; x=1729876694; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=9d3KGHo0twHh3KK7tsU2WqukBG2HV/y2vfRLQQjxQEs=; b=ued0EiE27Stev4Ad9LNbCGRn1a4l5MXeD6Pzlq2BKYvWE3ENZtTfCaX64Y54J+yTFS /6JPtQWEDdbzGPxjqPzZlXxywBJ/Ay0OX3ly5uYvxH6cpNhsfAYz/qS4FRFKrBy8yy+Y Ohx/4EgqJnvr3I1F/iLMu53863kjIG3C2VQP52Nase8wsXBn6x26/lPKpSEFwSVtG/on qygouR0vQqYb4oKRqFiPQzbdVVNfs6jgXBCkNcoxTPjhCCVc0+CrRfk48rUe4Mw33wef bDBB53bhhi1s/23DD+DGskVCjH3f2ACk6GHmQbaOFN6jA6o2DN9Q/AG/UclSI71fcaAJ uvKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729271894; x=1729876694; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=9d3KGHo0twHh3KK7tsU2WqukBG2HV/y2vfRLQQjxQEs=; b=hd2raFwiebqXHwLWOL4HAFmOvfXcOlb1G9B5hDWciGswZ3XHXng3yOBwO5jEMB48u4 Z/h+UuYnINFHCUm145pUyUi3uW1aiKP5I+hcyBtE5xPkghku9WeBZdEfqGjeJusfQWCl cmGtJCzbUYAcMGTKPNW2s9igm3GQTRWO5LTpxsjIe/aA8NnYmuC5ET2h7m7+V9CRQzne UnuzAIucGAkOXsS3kLEtp9MKZ+Gyz4PJIjmUCrYf2e+4pwN0ItOiIGx4kUaS0C+DkcwI VvK0GaFEInAct3WfMETWBuzTmOHkF2oAoBJyBTlEy/eK+OV4mOJAQqR64EXN5ByOtoSx zCzA== X-Forwarded-Encrypted: i=1; AJvYcCVTb3zEXytktHMywM8ZcYjRNIJWPgWZ1mC1SpLXnjExM5KYQECxM+Me9iBa9BSU+WUUUY4kSTdxbA==@kvack.org X-Gm-Message-State: AOJu0YyNQcRryri9HdHtoFXqH2ZK2JjGSq60+AishACcWM1KcI9ALcsw cs8cfxu9AXH/ynmLFFe4/tetcRJSWoMsFYHbUf3/enEjU01GafZsQUKIoM5sgDpNYO3l80FiMC+ 5CeoKTpmubatjFy9/Qw1EkjQ8tZmaJPIjS7Ty X-Google-Smtp-Source: AGHT+IE8nMV54DuZxqiA4l4fUopX08q7zMqWhyjajbG7WRK1oV7qQeWn3am9rp4Ty4elaO+wHaDx9nCBfdnDv3AYfc8= X-Received: by 2002:a17:907:97c1:b0:a9a:3dc0:8911 with SMTP id a640c23a62f3a-a9a69a7621amr276212566b.16.1729271893553; Fri, 18 Oct 2024 10:18:13 -0700 (PDT) MIME-Version: 1.0 References: <20241003211716.371786-1-edliaw@google.com> <20241003211716.371786-2-edliaw@google.com> In-Reply-To: From: Edward Liaw Date: Fri, 18 Oct 2024 10:17:46 -0700 Message-ID: Subject: Re: [PATCH 1/2] selftests/mm: replace atomic_bool with pthread_barrier_t To: Ryan Roberts Cc: linux-kselftest@vger.kernel.org, Andrew Morton , Shuah Khan , Lokesh Gidra , Peter Xu , linux-kernel@vger.kernel.org, kernel-team@android.com, linux-mm@kvack.org, Aishwarya TCV Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: diqa4ps7tbydi9ez6n5mqr1g4ttbkise X-Rspamd-Queue-Id: 5B8A5120013 X-Rspamd-Server: rspam11 X-HE-Tag: 1729271880-599869 X-HE-Meta: U2FsdGVkX19s2Qp/FUAlE0rxvvQLSOe2xXXs9fWz2FfRTGen0LsTHBnDeSMWTm98J79JeO9sgkdZxkXQOFUQV7ZNmm/cjC2Fox7Hvd5KXyg+WrHcZAuexBB119W8ujmmYOOZ3slLGqBJEC1/J+m+HKHGCU+KUaxzL6h3aubfHEubn5kJOlVFKVepXhgsS4R5aKNjyWmSBmlGwWdSoKk9p1onX6mrunipEb3hyFPuh1qhc12dTkQiJOTp6q+cd+FZJqZbdF38fYPAYtOI0wtld9YU809oRIS/lhNjmlc/z1Q2I4bsYeoCdKDUnUWn6wFkJEcocJUSsFN6Rnleb6qWgqsHHvV3bDxASrGYKYMW+gWWwH9qGPnc7wTe+7+dDtX0P43A2GWpMce45vulBXVZ4ihh8EFhTFcU/aLIQqdXHC6oHeseDcxK6i7vT1tx2MOG9ObsjSfrgixLSULK5aR0wYkOlKWtEJFnm0vDd8/UPSxyUI48YbJyCjO8XnxPN3WdQycRH5nHtCLIC/tBx7Nl3ySjUg36jwt0QUa7XwbvmNYFkkYi9WILkxgBr/p9MeHw48j8XmXjM4mqIhhjtFL2NzLYRmvEmZeDjdlr7BZCts7e2j6gjrExBgkv+0G4DD2U8NJQean1uvxKZTYejjT8aE1WPbBD6/Z0SfHQaLu/EtfJnmeD+8+Zz4LPmqftH7gjSwAB/rzwAWqAePyK5OopVNJpWZYUtW+ygKAAozl84bnxkTQDE/8GbIYGKnxcjpBzjLbc6JMePJkLUzmFjF5cM0nFcwdCeEO0KmUvyQ06E8lp2Og4U8WyHlmtmX6sFl8zUpWpNkEU4O7vmNga99xWS8T4S//dL2KPHIp82ub9UTU7Dt2EpgjSgsBKu/zzC/VE0/DLV9iMVhOiqZJoAoNa+N2QdUYbv/s5yM3HexXaBfpMstJ/8cH3pMZ/7888/ly0n8mIjTV/WaYJrLTaR8q +AAyRZcd 8i/1I23DgeH8+uK+iOjy7aWcQZ5o2z0G/Mdz4DO75IPPcNDHYYAap9PJ7USLfMgM3SQ3u9lYguLibJeBjbQL4IRVG1vkVOXxhuHjTWZM87UEkKeKbXbO8XFpgtfOVMeq/1LjNdRpN2HAex96/Qnk87dXcrHQ1OTU9sslyqFuacN9mWHEva7+Aafm6GksjoEE7g5ZhYznoQZ+05gaYAoI0akHxIFqTdK5UgNO9h8uAzePm8TggU1dqzY9URgVURUCi3kTYq7N4mJXC5RBJ1JnzZ6gckrWQukL44F1D4DiYDJ9Cspoao8y1UDtU9DHa0+iLnHfo X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Oct 18, 2024 at 7:37=E2=80=AFAM Ryan Roberts = wrote: > > On 03/10/2024 22:17, Edward Liaw wrote: > > Swaps synchronization primitive with pthread_barrier, so that > > stdatomic.h does not need to be included. > > > > The synchronization is needed on Android ARM64; we see a deadlock with > > pthread_create when the parent thread races forward before the child ha= s > > a chance to start doing work. > > > > Fixes: 8c864371b2a1 ("selftests/mm: fix ARM related issue with fork aft= er > > pthread_create") > > Signed-off-by: Edward Liaw > > --- > > tools/testing/selftests/mm/uffd-common.c | 5 +++-- > > tools/testing/selftests/mm/uffd-common.h | 3 +-- > > tools/testing/selftests/mm/uffd-unit-tests.c | 14 ++++++++------ > > 3 files changed, 12 insertions(+), 10 deletions(-) > > > > diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/s= elftests/mm/uffd-common.c > > index 717539eddf98..852e7281026e 100644 > > --- a/tools/testing/selftests/mm/uffd-common.c > > +++ b/tools/testing/selftests/mm/uffd-common.c > > @@ -18,7 +18,7 @@ bool test_uffdio_wp =3D 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; > > +pthread_barrier_t ready_for_fork; > > > > static int uffd_mem_fd_create(off_t mem_size, bool hugetlb) > > { > > @@ -519,7 +519,8 @@ void *uffd_poll_thread(void *arg) > > pollfd[1].fd =3D pipefd[cpu*2]; > > pollfd[1].events =3D POLLIN; > > > > - ready_for_fork =3D true; > > + /* Ready for parent thread to fork */ > > + pthread_barrier_wait(&ready_for_fork); > > Hi Edward, > > This change is causing uffd-unit-tests to hang on arm64 when running the = "move on anon" test. It's happening because this wait is never returning fo= r this case. And that happens because ready_for_fork was never initialized = for this test. It looks like there are other places where a thread is creat= ed for uffd_poll_thread() where ready_for_fork is not initialized too. > > I added this change and it solves the problem, although it's pretty hacky= . > > This is blocking our arm64 testing on linux-next so would appreciate eith= er a quick fix or removing the change until a fix is ready. Ah, I should not have changed it to a pthread_barrier, since that depends on the parent creating the barrier; I'm sending a revert and subsequent fix. > > Thanks, > Ryan > > ----8<----- > diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/sel= ftests/mm/uffd-common.c > index 852e7281026e..a05eb705be02 100644 > --- a/tools/testing/selftests/mm/uffd-common.c > +++ b/tools/testing/selftests/mm/uffd-common.c > @@ -19,6 +19,7 @@ unsigned long long *count_verify; > uffd_test_ops_t *uffd_test_ops; > uffd_test_case_ops_t *uffd_test_case_ops; > pthread_barrier_t ready_for_fork; > +bool wait_ready_for_fork; > > static int uffd_mem_fd_create(off_t mem_size, bool hugetlb) > { > @@ -520,7 +521,8 @@ void *uffd_poll_thread(void *arg) > pollfd[1].events =3D POLLIN; > > /* Ready for parent thread to fork */ > - pthread_barrier_wait(&ready_for_fork); > + if (wait_ready_for_fork) > + pthread_barrier_wait(&ready_for_fork); > > for (;;) { > ret =3D poll(pollfd, 2, -1); > diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/sel= ftests/mm/uffd-common.h > index 3e6228d8e0dc..e94329a39b34 100644 > --- a/tools/testing/selftests/mm/uffd-common.h > +++ b/tools/testing/selftests/mm/uffd-common.h > @@ -105,6 +105,7 @@ extern bool test_uffdio_wp; > extern unsigned long long *count_verify; > extern volatile bool test_uffdio_copy_eexist; > extern pthread_barrier_t ready_for_fork; > +extern bool wait_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 3db2296ac631..d1fc4cd6a948 100644 > --- a/tools/testing/selftests/mm/uffd-unit-tests.c > +++ b/tools/testing/selftests/mm/uffd-unit-tests.c > @@ -775,6 +775,7 @@ static void uffd_sigbus_test_common(bool wp) > struct uffd_args args =3D { 0 }; > > pthread_barrier_init(&ready_for_fork, NULL, 2); > + wait_ready_for_fork =3D true; > > fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK); > > @@ -794,6 +795,7 @@ static void uffd_sigbus_test_common(bool wp) > /* Wait for child thread to start before forking */ > pthread_barrier_wait(&ready_for_fork); > pthread_barrier_destroy(&ready_for_fork); > + wait_ready_for_fork =3D false; > > pid =3D fork(); > if (pid < 0) > @@ -835,6 +837,7 @@ static void uffd_events_test_common(bool wp) > struct uffd_args args =3D { 0 }; > > pthread_barrier_init(&ready_for_fork, NULL, 2); > + wait_ready_for_fork =3D true; > > fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK); > if (uffd_register(uffd, area_dst, nr_pages * page_size, > @@ -848,6 +851,7 @@ static void uffd_events_test_common(bool wp) > /* Wait for child thread to start before forking */ > pthread_barrier_wait(&ready_for_fork); > pthread_barrier_destroy(&ready_for_fork); > + wait_ready_for_fork =3D false; > > pid =3D fork(); > if (pid < 0) > ----8<----- > > > > > > for (;;) { > > ret =3D poll(pollfd, 2, -1); > > diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/s= elftests/mm/uffd-common.h > > index a70ae10b5f62..3e6228d8e0dc 100644 > > --- a/tools/testing/selftests/mm/uffd-common.h > > +++ b/tools/testing/selftests/mm/uffd-common.h > > @@ -33,7 +33,6 @@ > > #include > > #include > > #include > > -#include > > > > #include "../kselftest.h" > > #include "vm_util.h" > > @@ -105,7 +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 pthread_barrier_t 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/testi= ng/selftests/mm/uffd-unit-tests.c > > index b3d21eed203d..3db2296ac631 100644 > > --- a/tools/testing/selftests/mm/uffd-unit-tests.c > > +++ b/tools/testing/selftests/mm/uffd-unit-tests.c > > @@ -774,7 +774,7 @@ static void uffd_sigbus_test_common(bool wp) > > char c; > > struct uffd_args args =3D { 0 }; > > > > - ready_for_fork =3D false; > > + pthread_barrier_init(&ready_for_fork, NULL, 2); > > > > fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK); > > > > @@ -791,8 +791,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 f= orking */ > > + /* Wait for child thread to start before forking */ > > + pthread_barrier_wait(&ready_for_fork); > > + pthread_barrier_destroy(&ready_for_fork); > > > > pid =3D fork(); > > if (pid < 0) > > @@ -833,7 +834,7 @@ static void uffd_events_test_common(bool wp) > > char c; > > struct uffd_args args =3D { 0 }; > > > > - ready_for_fork =3D false; > > + pthread_barrier_init(&ready_for_fork, NULL, 2); > > > > fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK); > > if (uffd_register(uffd, area_dst, nr_pages * page_size, > > @@ -844,8 +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 f= orking */ > > + /* Wait for child thread to start before forking */ > > + pthread_barrier_wait(&ready_for_fork); > > + pthread_barrier_destroy(&ready_for_fork); > > > > pid =3D fork(); > > if (pid < 0) >