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 99B75EB64DA for ; Tue, 18 Jul 2023 10:49:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0C8038D0001; Tue, 18 Jul 2023 06:49:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 078066B0074; Tue, 18 Jul 2023 06:49:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EA9588D0001; Tue, 18 Jul 2023 06:49:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id DA4B16B0071 for ; Tue, 18 Jul 2023 06:49:18 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 94A0514034C for ; Tue, 18 Jul 2023 10:49:18 +0000 (UTC) X-FDA: 81024410796.17.0079BDE Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf15.hostedemail.com (Postfix) with ESMTP id B76EAA0012 for ; Tue, 18 Jul 2023 10:49:16 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=none; spf=pass (imf15.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1689677357; 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; bh=snY0hoQBnXH5jJwH47G7MHj9GxkYqabFTbbTVmAeSz0=; b=yZ7qN207ClEHQCtEZw1FQncieHQXWpagvTRfDeW7164iiqgq2xRIpvxdQRvf5dT5BkIhN5 Dy9cq7u/FylKkeZ8xauXGk69NiWzF41LaeyFV48kS2+hzHB3sN1T1DTsBtqF+79kPFu3Pk /atPLg3WKXTSOfwBp7Uv1dyWLTqTsiE= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1689677357; a=rsa-sha256; cv=none; b=thR7X/j4IxbXYew1YByK/ybKfL1akVhwAdvkfosT+iJRFqk/QkEBhmBzynAwjcZGGzUVFa Frvyxu6QJoCzqF5/5gvvPb2fdLKUbgtACe+Wjc1lYKS9GxVGwcdi2HmipUCs1/++O3ihcr h7r3Q0RrL7EtxyF0x6VBhO7Yfcf8xqg= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=none; spf=pass (imf15.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 239432F4; Tue, 18 Jul 2023 03:49:59 -0700 (PDT) Received: from [10.1.34.52] (C02Z41KALVDN.cambridge.arm.com [10.1.34.52]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 24BCD3F6C4; Tue, 18 Jul 2023 03:49:14 -0700 (PDT) Message-ID: <2e5baba4-c8ef-9da4-a2d6-3cf383ed05bd@arm.com> Date: Tue, 18 Jul 2023 11:49:12 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v2 6/8] selftests/mm: Make migration test robust to failure To: David Hildenbrand , Andrew Morton , Shuah Khan , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Mark Brown , John Hubbard , Florent Revest , Peter Xu Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org References: <20230717103152.202078-1-ryan.roberts@arm.com> <20230717103152.202078-7-ryan.roberts@arm.com> <20501a7c-19f6-4154-aebc-49df04c9b043@redhat.com> From: Ryan Roberts In-Reply-To: <20501a7c-19f6-4154-aebc-49df04c9b043@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Stat-Signature: f4kf63dhs93jtae3re5666tbb68b8ueu X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: B76EAA0012 X-Rspam-User: X-HE-Tag: 1689677356-208765 X-HE-Meta: U2FsdGVkX1+BuCpZ1R3rRIXxt1N4os4pNlmhqybyF8m825UVWc/o8cLxcABYfXzc6D3GD+L2Diedy9yMVWcjhie6PcXcJNZdnt67XH+LwXa4qBglV7pIN/0E/OIvPYVQlJ/DpHH23ydkg8X1um5OhKpy21kJRwhP0oNR8DetB78i/x5CRBMjjwtp8Ypy7gYFObUPQkvrXuYh7aelAnBNOYKtdyvzYPewxC1RzZE+Qij2qak2KnWylCYK1Fqc+bksnfvmavY/B7mAzX+KDlTlxHVWhwnpvpE9Gl1LrT5MQnQ5yrtlOHZWoqKCNHsy/RD4XTQEdIK4AW9sClX1dF/eHMFdhQrBDhwbb+odeRZytJ2eOg3AX5yxiXZhmohZtqzi9fv3SOphi0meNlhbI/cnKLlfrikWIF1XSH5IgUXk9xObXMLwRAaMbf5onAzyYSSi2JHJFrV9FoAMwEaGVdfHVG1Lr2YsvRclzHRHUfJkgMGb+uLq6O3kN9aD0HIpuaevwKcHgdmRsaMQjFZ4phC3bb8OFj9WSUGaHKeD2Om9StqQWXaUYdlxyxXn1GeckuHZ4yAyBt57HTVDHIFzPBM+cDsaU9cJUmhfNFwH8FiRRjb8izGDFTCKwkFpCEXAPDGbQUcQLF+A9YKcSDDsP/yQnGGxRL5NgCn02Tb7tHzmLA9prd5csyzw8aQf1SJjLHw126vZscWHtYlAGFa9MkTywyK1uQgracIDpuKEbltgMoa6DOsMhvff50MJF815F7sB5LvoBEJvnjPpiJJ6akbmaeejy9WRxkqpgFVXGQfWAGA8BKst/otP40mvI766HLLNjPS5IFXk/lJ36RAmn7g/izjMasPg6bNT2pF596/3+KEyABs68fKudLKtzo/oiBpZFiZy1290av4fux5JWmEyW426DrY27LjYYoLp9Raae5D8p/e4rG0kHkeIwrF61HY5Wr2v7af9m4tUwWzDCli lXKvYGag 1GB3OJclH8GNcq2V6manm85mRWupp/lXUniOjOC/Ni7VbnTtxmLmlZme4MmXh4UqdHx2EgW2VRXAwTqLkexZURIG7xIW93L+2CsNlpFn2gQSb7IEa8+Rl1CxYMpamkV1Vvvv/6GbReZrZNnX2fWwx/5pw30t3dp1d0waKiVMdWgK0wBA= 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: On 17/07/2023 18:40, David Hildenbrand wrote: > On 17.07.23 12:31, Ryan Roberts wrote: >> The `migration` test currently has a number of robustness problems that >> cause it to hang and leak resources. >> >> Timeout: There are 3 tests, which each previously ran for 60 seconds. >> However, the timeout in mm/settings for a single test binary was set to >> 45 seconds. So when run using run_kselftest.sh, the top level timeout >> would trigger before the test binary was finished. Solve this by meeting >> in the middle; each of the 3 tests now runs for 20 seconds (for a total >> of 60), and the top level timeout is set to 90 seconds. >> >> Leaking child processes: the `shared_anon` test fork()s some children >> but then an ASSERT() fires before the test kills those children. The >> assert causes immediate exit of the parent and leaking of the children. >> Furthermore, if run using the run_kselftest.sh wrapper, the wrapper >> would get stuck waiting for those children to exit, which never happens. >> Solve this by deferring any asserts until after the children are killed. >> The same pattern is used for the threaded tests for uniformity. >> >> With these changes, the test binary now runs to completion on arm64, >> with 2 tests passing and the `shared_anon` test failing. >> >> Signed-off-by: Ryan Roberts >> --- >>   tools/testing/selftests/mm/migration.c | 14 ++++++++++---- >>   tools/testing/selftests/mm/settings    |  2 +- >>   2 files changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/tools/testing/selftests/mm/migration.c >> b/tools/testing/selftests/mm/migration.c >> index 379581567f27..189d7d9070e8 100644 >> --- a/tools/testing/selftests/mm/migration.c >> +++ b/tools/testing/selftests/mm/migration.c >> @@ -15,7 +15,7 @@ >>   #include >>     #define TWOMEG (2<<20) >> -#define RUNTIME (60) >> +#define RUNTIME (20) >>     #define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1))) >>   @@ -118,6 +118,7 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME) >>   { >>       uint64_t *ptr; >>       int i; >> +    int ret; >>         if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0) >>           SKIP(return, "Not enough threads or NUMA nodes available"); >> @@ -131,9 +132,10 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME) >>           if (pthread_create(&self->threads[i], NULL, access_mem, ptr)) >>               perror("Couldn't create thread"); >>   -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0); >> +    ret = migrate(ptr, self->n1, self->n2); >>       for (i = 0; i < self->nthreads - 1; i++) >>           ASSERT_EQ(pthread_cancel(self->threads[i]), 0); >> +    ASSERT_EQ(ret, 0); > > Why is that required? This does not involve fork. It's not required. I was just trying to keep everything aligned to the same pattern. > >>   } >>     /* >> @@ -144,6 +146,7 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME) >>       pid_t pid; >>       uint64_t *ptr; >>       int i; >> +    int ret; >>         if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0) >>           SKIP(return, "Not enough threads or NUMA nodes available"); >> @@ -161,9 +164,10 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME) >>               self->pids[i] = pid; >>       } >>   -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0); >> +    ret = migrate(ptr, self->n1, self->n2); >>       for (i = 0; i < self->nthreads - 1; i++) >>           ASSERT_EQ(kill(self->pids[i], SIGTERM), 0); >> +    ASSERT_EQ(ret, 0); > > > Might be cleaner to also: Or instead of? I agree this is neater, so will undo the moving of the ASSERT() and rely on this prctl. > > diff --git a/tools/testing/selftests/mm/migration.c > b/tools/testing/selftests/mm/migration.c > index 379581567f27..b3f12b9847ec 100644 > --- a/tools/testing/selftests/mm/migration.c > +++ b/tools/testing/selftests/mm/migration.c > @@ -11,6 +11,7 @@ >  #include >  #include >  #include > +#include >  #include >  #include > > @@ -155,10 +156,12 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME) >         memset(ptr, 0xde, TWOMEG); >         for (i = 0; i < self->nthreads - 1; i++) { >                 pid = fork(); > -               if (!pid) > +               if (!pid) { > +                       prctl(PR_SET_PDEATHSIG, SIGHUP); >                         access_mem(ptr); > -               else > +               } else { >                         self->pids[i] = pid; > +               } >         } > >         ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0); > > > Then, whenever the parent dies, all child processes get zapped. > >