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 A1FDEC43334 for ; Tue, 5 Jul 2022 17:02:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2C0B26B0071; Tue, 5 Jul 2022 13:02:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 26FBC8E0002; Tue, 5 Jul 2022 13:02:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 138C76B0074; Tue, 5 Jul 2022 13:02:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 03CA86B0071 for ; Tue, 5 Jul 2022 13:02:21 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay11.hostedemail.com (Postfix) with ESMTP id C49D6801A8 for ; Tue, 5 Jul 2022 17:02:20 +0000 (UTC) X-FDA: 79653664440.21.AB6DB0F Received: from mail-yb1-f179.google.com (mail-yb1-f179.google.com [209.85.219.179]) by imf01.hostedemail.com (Postfix) with ESMTP id 666E64000F for ; Tue, 5 Jul 2022 17:02:20 +0000 (UTC) Received: by mail-yb1-f179.google.com with SMTP id i7so22924672ybe.11 for ; Tue, 05 Jul 2022 10:02:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9XOyOTVWWKb/JCrh+gNLsNuvav7Gz0G/bPl9Te68Fwg=; b=MqzIYCjpBGWvSVZ9jcEUHVhWa1oA0I9uVc2TpSnhJztj4Ui3DEMCG3poL217ttajos +tpQoA59rExymTFZy9dWbOVaoj6fjCY9d0eMU0TxKLWY/iTXju6WYmS7MkSED0ivjHuZ 6aOYNAgtPaH9dAsOk37n6gq2pg/nYf7TmOwKlCkQvL8N+vw5RBr6C2abeC8AfsXAgtwe xoP2PaFm3NRhhU5hkeABvO0zJ8tLH1cO/r5NUuXEy9O3evSy53DUJJlWP0+VJud5OY8Z S5e1i3hkexJg6shJpcWodbjYlviSi1kmMCE9uiDDEAioXftZg+D4KTbkafGzpkZRzrtL p+2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9XOyOTVWWKb/JCrh+gNLsNuvav7Gz0G/bPl9Te68Fwg=; b=kRZo9gQXqFIs0sPb3cEK74VvoPumnGj5tbmAf/9eed1tkdWE33kaCpeFGRdBWXz2/Q yfSsSJkpqeKC7eONNjX47rujGYMXtyFqP31JUHiMs/emcDwQd6G6TrBnB2SDJI0grlsO Ma6QS9z4E8e0jpsjJ8hlGhilJQuKUGoVDUnaPF63xw4X43PYi6suwM6wIWj9LkQx6jxg CSISqk2ixHuQwLSdKIzrEhbUUkC6tNvhp9dCb1HgCe3Wly1ISoEk0pU25BdGmSaxLliB CZT3dPo4JVjKa/3WRXEx71VhsJ9cpDSDmh699zRlvcKidbDSUDcD3IXLxqejLjOOQFrE pdfA== X-Gm-Message-State: AJIora/wWc+7F7cX2KcLIPevqLisX5NkcIKwTQCjHI0zdzo1ORRThZFe 1/d/CRn4cw0jcRVmHKDp3COdUvPsrwLVPAGGaffGUQ== X-Google-Smtp-Source: AGRyM1v0cwsarcIi2DM2H1/KpQM/YjciOfWkgM1janrqCq9g8UxnS5Je20Bm1SawEHMDktoKHyaPK0cy2fg/3S7VJns= X-Received: by 2002:a25:888a:0:b0:669:9661:912 with SMTP id d10-20020a25888a000000b0066996610912mr37449131ybl.348.1657040539411; Tue, 05 Jul 2022 10:02:19 -0700 (PDT) MIME-Version: 1.0 References: <20220704173351.19595-1-adam@wowsignal.io> <20220705150015.o3omwkc7c23dbtkh@dev0025.ash9.facebook.com> In-Reply-To: <20220705150015.o3omwkc7c23dbtkh@dev0025.ash9.facebook.com> From: Suren Baghdasaryan Date: Tue, 5 Jul 2022 10:02:08 -0700 Message-ID: Subject: Re: [PATCH v2] selftests/vm: fix errno handling in mrelease_test To: David Vernet Cc: Adam Sindelar , Andrew Morton , linux-mm , Adam Sindelar , kernel-team@fb.com Content-Type: text/plain; charset="UTF-8" ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=MqzIYCjp; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of surenb@google.com designates 209.85.219.179 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1657040540; a=rsa-sha256; cv=none; b=b3KNbMPG2jAatbxP+vzVEQk8daGVEncYu8upYZKCs4v7KDdyVKmNdTzcIOWRDAQFMwKYe8 T7NJgOEcixz0d+Wybe8RB5WOSKTDnNJNafsIRQQSTKjD6Ch9S0jbKdnxBC+T5dXS6fgslg 6ki7W/NslraJSSVFFk2d6fJeCMzz1tM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1657040540; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=9XOyOTVWWKb/JCrh+gNLsNuvav7Gz0G/bPl9Te68Fwg=; b=D4GAhK2/Y5ZSgyclV7p/R9B4QgQC20vB76htcludkDSBAeP98uUepOeIhRhfjImksjY07E Kzg0HhQAlOs5mlzAzRAA/bKvGRJopo9cFG0n+NqxA40/bOk20/dOaioZyktnyS7GIJTIzb 9/JZrKTsh0QIrr3q3hzZChYM5kQVpNo= Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=MqzIYCjp; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of surenb@google.com designates 209.85.219.179 as permitted sender) smtp.mailfrom=surenb@google.com X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: x1rq7j3m6ckpspbndtooftu9t4u1973f X-Rspamd-Queue-Id: 666E64000F X-HE-Tag: 1657040540-972198 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jul 5, 2022 at 9:41 AM David Vernet wrote: > > On Mon, Jul 04, 2022 at 07:33:51PM +0200, Adam Sindelar wrote: > > Thanks for fixing this, Adam. > > > mrelease_test should return KSFT_SKIP when process_mrelease is not > > defined, but due to a perror call consuming the errno, it returns > > KSFT_FAIL. > > > > This patch decides the exit code before calling perror. > > > > We should probably also include a "Fixes" line here (see [0]): > > Fixes: 33776141b812 ("selftests: vm: add process_mrelease tests") > > [0]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes > > > Signed-off-by: Adam Sindelar > > --- > > v1->v2: Fixed second instance in the same file > > > > tools/testing/selftests/vm/mrelease_test.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/tools/testing/selftests/vm/mrelease_test.c b/tools/testing/selftests/vm/mrelease_test.c > > index 96671c2f7d48..e8b17258579b 100644 > > --- a/tools/testing/selftests/vm/mrelease_test.c > > +++ b/tools/testing/selftests/vm/mrelease_test.c > > @@ -100,8 +100,10 @@ int main(void) > > > > /* Test a wrong pidfd */ > > if (!syscall(__NR_process_mrelease, -1, 0) || errno != EBADF) { > > + /* perror overwrites errno, so this line must be first */ > > + res = (errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL); > > perror("process_mrelease with wrong pidfd"); > > - exit(errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL); > > + exit(res); > > } > > > > /* Start the test with 1MB child memory allocation */ > > @@ -156,8 +158,9 @@ int main(void) > > run_negative_tests(pidfd); > > > > if (kill(pid, SIGKILL)) { > > + res = (errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL); > > perror("kill"); > > - exit(errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL); > > + exit(res); > > } > > > > success = (syscall(__NR_process_mrelease, pidfd, 0) == 0); > > -- > > 2.35.1 > > > > This looks good to me, but it looks like there are a couple of other places > where we're doing the wrong thing, i.e. in run_negative_tests() and after > calling waitpid(). Could you please fix those as well? > > Also adding Suren to cc. Thanks for adding me, David! The fixes look good but there are 5 places in all this fix is needed: https://elixir.bootlin.com/linux/v5.19-rc5/source/tools/testing/selftests/vm/mrelease_test.c#L68 https://elixir.bootlin.com/linux/v5.19-rc5/source/tools/testing/selftests/vm/mrelease_test.c#L76 https://elixir.bootlin.com/linux/v5.19-rc5/source/tools/testing/selftests/vm/mrelease_test.c#L103 https://elixir.bootlin.com/linux/v5.19-rc5/source/tools/testing/selftests/vm/mrelease_test.c#L159 https://elixir.bootlin.com/linux/v5.19-rc5/source/tools/testing/selftests/vm/mrelease_test.c#L175 Thanks for catching this, Adam! Suren. > > - David