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 564AAEB64DA for ; Tue, 18 Jul 2023 11:25:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D17408D0001; Tue, 18 Jul 2023 07:24:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CC7BD6B0074; Tue, 18 Jul 2023 07:24:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B69318D0001; Tue, 18 Jul 2023 07:24:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id A44436B0071 for ; Tue, 18 Jul 2023 07:24:59 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 6B0A8160370 for ; Tue, 18 Jul 2023 11:24:59 +0000 (UTC) X-FDA: 81024500718.23.2E873ED Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf06.hostedemail.com (Postfix) with ESMTP id 02462180018 for ; Tue, 18 Jul 2023 11:24:56 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="FzyI/qy4"; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf06.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1689679497; 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=6uvmITmSWq5XdrG3a/DkmnJG1vY8QF6IRECe80ErATc=; b=8O+c6btlJQQwlvFVSRnPDFcPaa+VTdjkd5YNakdVh14A/fIDE/qOChDlzlICAK9L7TsyRj TSGHjWqO4x4Q/tDQaAWDcQ6nWdM+HIZL3nDvHhdBhlx85yDpJZ+dbr9kfRJ1UbFsniOOKf +ax8SPGmt0Xs6YfMJ2+4Yn6GyqfCsWk= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="FzyI/qy4"; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf06.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1689679497; a=rsa-sha256; cv=none; b=HWL5K8uRhAYksbepgtfkr2EJGRL4BBjI+DO4c6W+AgLeCA+9rtXDI0qlFXG0M+LbfEpW4G xAZKvN+Sh3RBfWOz2KATbrTnC40oTD+s+J2+3K1nzqpjwK2NOthLeijzQTwg3di/8uyD6H W+hFEINVo+afYlDeu3BQZcFKy46XrRU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1689679496; h=from:from: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=6uvmITmSWq5XdrG3a/DkmnJG1vY8QF6IRECe80ErATc=; b=FzyI/qy4PHPNTH1kVbgd4dWYRKOHUh7hEgmZ7FDp3Ku2tusEwYsabUlCnK+cSNdHrJLWg4 bo6BYgWImGX/DgO0fAhO2pyKFENJWRbpvXzoCapVKL3emhO6YcPYq3wNOavkfB1eUhawzf y88HkY2etwk1X3gmzXzQOM19F9g1K6o= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-173-ZJvb4oUhNCGKZN9XLzx__Q-1; Tue, 18 Jul 2023 07:24:55 -0400 X-MC-Unique: ZJvb4oUhNCGKZN9XLzx__Q-1 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-3158f4d72e9so3116091f8f.1 for ; Tue, 18 Jul 2023 04:24:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689679494; x=1692271494; h=content-transfer-encoding:in-reply-to:organization:references:cc:to :from:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=6uvmITmSWq5XdrG3a/DkmnJG1vY8QF6IRECe80ErATc=; b=Z6BYZ4jUMXDcfb31nPtH8uBFjyCfTV7ZQOtOp1TiVM7XCY61JhxsMVKyf6MHAQKdq4 T4Et6c8CmPeiW1jL39mR+TDOiwWg/HHH4cPZ+8ET0q/SepB14X6WdnIjrz8+mIXQxQMl WuTS+dCNyXwFBiobn2e6PP4+SlSX+CnlHvxtUkCDtWd/EVTxZFrDioEp2FXatd8oDZAU +oJFxCD1Cqr6PERAUfm3lNfwZ/XBiNqdE/B8XK7C+ikXBA+7SYFHScPkMdPn9vqP1BcD tNqLLpk6f/+9pS0W1TcGiDDoNiII7C1AQoBGsPlbC9vbq9K3NKpPy5piA0jue1VuxRRH WRLA== X-Gm-Message-State: ABy/qLYkqZE3ydHUUa9Z7vt7HHR5UNo09bWKfD+RA/bbZ9/BRsyM/3a8 vhIdmJCJbV/Oc0jdhqDtN6V6hjrAbAFO6OV/CTdEwgiumMejo5QFpRauMOws9oXLjC4ZwgWP8se azjcQtT1+DpI= X-Received: by 2002:a5d:6603:0:b0:313:ee2e:dae1 with SMTP id n3-20020a5d6603000000b00313ee2edae1mr11776895wru.18.1689679493869; Tue, 18 Jul 2023 04:24:53 -0700 (PDT) X-Google-Smtp-Source: APBJJlHg7VIZgLtfc+4zjH3bLdSY+veUxJSqkHQDeMhRBUXaInjZYiNoZtklKOQkBINC56GiIEyvYw== X-Received: by 2002:a5d:6603:0:b0:313:ee2e:dae1 with SMTP id n3-20020a5d6603000000b00313ee2edae1mr11776882wru.18.1689679493465; Tue, 18 Jul 2023 04:24:53 -0700 (PDT) Received: from ?IPV6:2003:cb:c740:6200:84e1:eabc:8e2:7429? (p200300cbc740620084e1eabc08e27429.dip0.t-ipconnect.de. [2003:cb:c740:6200:84e1:eabc:8e2:7429]) by smtp.gmail.com with ESMTPSA id n13-20020adff08d000000b0031433443265sm2136504wro.53.2023.07.18.04.24.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 18 Jul 2023 04:24:52 -0700 (PDT) Message-ID: <90a8b224-4566-5051-4903-f9ab83b26150@redhat.com> Date: Tue, 18 Jul 2023 13:24:52 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH v2 6/8] selftests/mm: Make migration test robust to failure From: David Hildenbrand To: Ryan Roberts , 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> <2e5baba4-c8ef-9da4-a2d6-3cf383ed05bd@arm.com> <1314fe0e-dd32-bf10-0a33-2b571dad70bd@redhat.com> Organization: Red Hat In-Reply-To: <1314fe0e-dd32-bf10-0a33-2b571dad70bd@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 02462180018 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: qa9osu4bkc8jkgerd8escorwx4m5uidh X-HE-Tag: 1689679496-248657 X-HE-Meta: U2FsdGVkX1+nPgm4yx9Y1cGQO5M44MzOdv2j463ISgVMriy+OOEkimK0hqmFJ0E/fJWj+pcq1YyGZjDRyKQLwXj9e6sI1YxPzGm3hGxYWCIehJ55WOG3D71XrePkSAKkLpnduXyV8YlDgMGkonelCXE//abB6+KnBA8YrBg2pBKUZX1cFdhSw74sHZ1YBSnIgWjFe3bVRlDRNBtKM8YA0NxR+n30gBGSpsVcQ1EZ2EwGtWGyJI+uayHX67lwllA0ztlX4EZpjZZBs+sgB5y4vxavmGgdxuGLEHtK1qUHhr0v8nN+X3/q3MYKwVmUpp8NSOZVASkAep//1WKlo31YZOLfZkZf6SKowpM9koUQqsP2RtkfWjWRF0hDKvIzyePX6PbrEk4Y+M0pEM26R4LnrgJZQyll1kxgEfTrRdKljqiuAf+TYeh2OPTtbjih5+p/7ccCPZJHzP0iE0Vg2Gq8KfUHlyTANFHGmGpbK8qdSCCdoFShSVy+h7Ao91pI9Q4Fwgkhif0yLZptz101Qzeg4X5DsjicYriLb0199fVNmRRecsk1rSsC3g3a0aYaJES17I1FbZ7+bY8LmseONW2vXgTGwhN05sEDCsb1yFuIMgbtSmsqnvQHdxLRnkqj421JMjf1aXQTLyhKcGyQimhRtkI95ANuaKGZ2p0qlcI5X2aYS362cvWPfS9TJVOoQnR1CjlhJwYWgLGwtdfFxU8TxbO5bYY7Ayh/+L7+W3en4V+bDaSignbe3fwPwIbXFu6a4YGe4UEN7dBb6fQ9zm/Aqwcu17tOrCCtOyn/0PSNO0SLht5BmyR4ghtugmFG9Fjr1OzEHZzNiCw9OdOZmCIi2Gu6/Cdvudyz/6vnQrq/C+KxlJ7FfZUwbo2p2DPnZ10fh+YG4aZUcbCshWcFNSPmanMXCGUnJ+1bCSpim44a8q/k/1Nw8CWCmnKYwdWEeoqhOMVElB0AsymSViikTaB Z5/e9tfc PhBJQ2+Vo82BWMQ47oFDwYnWY9NxxsmwT+720uGg1zAt5bSsVmwoBh8FbmDGvwuVIpRWq/bqmtkrDbH5TjY4vMVBhtlf0tVdzcK1K4XVHt+a69oIuYXSTACazw+dtGiB0h+Z818Aa8vS2CdERvSuAgJEgjkmp1vr6/GVM8SxOSKR26grAYlVMH8qUTzMe3L/JPg0bws5ivVRSkeiedlJISgmEJlhME8KjdWF1V+nouXie6Gz7/doP/UPdrzRKT9OfNE15VYnJWD8YaxL7D2A5XqnQa2U04hAzlcZQT1zqOc9bUXfQDQK0mZh3YeUcV/T4tA0wYO1ZhmxmwF31P1tSN8fw12vdnFMPW5Jn 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 18.07.23 13:23, David Hildenbrand wrote: > On 18.07.23 12:49, Ryan Roberts wrote: >> 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. > > I was thinking about possible races when our parent process already > quits before our child managed to set the prctl. prctl() won't do > anything in that case, hmmmm. > > But similarly, existing code might already trigger the migrate() + kill > before the child processes even started to access_mem(). > > Racy :) > Maybe what would work, is checking after the prctl() in the child if the parent is already gone. -- Cheers, David / dhildenb