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 451C6EE49A3 for ; Tue, 22 Aug 2023 16:59:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8561428004B; Tue, 22 Aug 2023 12:59:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8060B280046; Tue, 22 Aug 2023 12:59:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6CDC528004B; Tue, 22 Aug 2023 12:59:04 -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 5EE4B280046 for ; Tue, 22 Aug 2023 12:59:04 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 3299E8033C for ; Tue, 22 Aug 2023 16:59:04 +0000 (UTC) X-FDA: 81152350608.14.A880E69 Received: from mail-il1-f173.google.com (mail-il1-f173.google.com [209.85.166.173]) by imf09.hostedemail.com (Postfix) with ESMTP id 65520140034 for ; Tue, 22 Aug 2023 16:59:02 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=DjHjLvwE; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf09.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.173 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1692723542; 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=N28LuQnV9GildzcT8LKkphpWOthpCuwzebagXmXjarY=; b=4W4jylzsTVX6sWSOktf5ax+7tfukHCS5L0dKss7qwAKIRONuWNdmbIKuqBLScecMK46H1e 7eAEaM0sQwiHCP1HK3UaC9UF5PUuB0iBqx+hnqnMw9vx04CQf1wCPBKFWV8RR1ApRF2Oeo 0wRJs05jJMs863HNq6aeUp2WE6JEJjQ= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=DjHjLvwE; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf09.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.173 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692723542; a=rsa-sha256; cv=none; b=30p7exp3D9LUT/5bgfpcN/WT/OEfsuFvh/yFiEClyfGB9D+wo5p81YQ1u1gpZhKlzqBzNw 5pDioA96ZLUIHC1OSjPadqneTlewLvlMv7GHDEUl9Ghg7c84YTt07LyYOj2SDCfRuobS1N JhRmKFpJ/rxmy5uED/Vrkr8+FpsP8A0= Received: by mail-il1-f173.google.com with SMTP id e9e14a558f8ab-34bbc5b5381so9412375ab.0 for ; Tue, 22 Aug 2023 09:59:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1692723541; x=1693328341; 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=N28LuQnV9GildzcT8LKkphpWOthpCuwzebagXmXjarY=; b=DjHjLvwER9a8EUgsdxU3QPMVGvEkpl2obK0Y10/iLsjkRQ0AdxRs+Bm2HDjYWWDAiF wDwYxDVpWbb0lkfDGOXdcjxv3cw4TMC04lO0jOFFeYoyd65nIIJ5Yw20KDdXzg4nYXRs E4lX/S8GUQR+6rWczJ9zAkUUN7VktM+SK3I7XnISpvmM50h924QPSxl+M9tPnKG/L1oW bgu7AyScSSetEKgGkiYyy1KYyGDWbZLozFPZ9mOn/ygOhbclYRxkPxP3DKK5f3IU8R07 /AdBeUaP/KyuNJlqXwWAV4wTd01ScacY4i/XtHLafd6eXNKcCWGTI95Ny5jKfrnwMFXo d2pQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692723541; x=1693328341; 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=N28LuQnV9GildzcT8LKkphpWOthpCuwzebagXmXjarY=; b=eaDa1ONla4vai3WmxHxs8Y20eFkUV55OmZa1KsY62EuZ0+8f06KlcX25lMFWSpV+/G 1zs1qJWps7kT4FYJUpGk9xLCJB8ybAo3L9Lerf0yUMjPQRQMaoUvCcTZsY0CzL4DRfQT W2wXYBzKvaLBP5h8RsZOn9359QDkoFF0S3IghHI1DLlnPx6eS0dwscPBTVyG2/uKUy4x 4OSRksZbk9qWSSKN7VBWVMT2sMUsQyAcWx6ie1ySRk9HDYIb7zs+gNu4HPiN9spgVSqm VxRchnPmiatU3aQs+AiogqgOTwAi+/urA5zISe+6z09nBnilCGoBGdzmCjAOAfxtMlso ZxgQ== X-Gm-Message-State: AOJu0YwLcU474l/Wd2jTrOucR4ysVtsejlgfSjdNh4SbxLJZe2lM6yGC 9RcBLMLt+1uIuNNSCCUK6vzTc5spb/0imJcmsqw= X-Google-Smtp-Source: AGHT+IHt26+UQXrWFQH+3S8jn1srFoxmOruOmztWrMHgGMGI6XKgZdP5HwziGldvSChthRr3P/HMv/uzSc4RKexOUPU= X-Received: by 2002:a05:6e02:1d97:b0:348:b086:2c4b with SMTP id h23-20020a056e021d9700b00348b0862c4bmr184020ila.9.1692723541341; Tue, 22 Aug 2023 09:59:01 -0700 (PDT) MIME-Version: 1.0 References: <20230821160534.3414911-1-andre.przywara@arm.com> <20230821160534.3414911-3-andre.przywara@arm.com> In-Reply-To: From: Nhat Pham Date: Tue, 22 Aug 2023 09:58:50 -0700 Message-ID: Subject: Re: [PATCH v2 2/2] selftests: cachestat: catch failing fsync test on tmpfs To: Andre Przywara Cc: Shuah Khan , Johannes Weiner , linux-kselftest@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 65520140034 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: biy86e7acoap8j7eprkqb9irh6oe6kzc X-HE-Tag: 1692723542-873769 X-HE-Meta: U2FsdGVkX1+FFMxQ0s3/TMb1cZoFep8K+PZMIRWsJ25N4VoSiDTcHg+dYPlN1u6q2HGNb2KpUa14xSibbSAEBo5L8ZY2HjSPJB0TejNoEl7Sw0r3Fu8H9aoBMcmAmd2Fl13Dmq6BOjl7qq/lzo/AFsOmtAYRFMpg+xSB30ItqQFxwfmiVPDnL4azkWO5jFU9SYPo700wsu8q+lbn2xzOpP1MkNePQPeB8M31sc3z2dhJr9D6tDV1RYw/EJ4dxbJ/YZqpujNS3MAse8RH/rhOymV2PMAIp2lPZdBD1164sO0P56TrCLNUZGN9OR1dLSg0e4uTNk9velTEVHLf3LgjyBw8GaOVx27lmrivCNY9K6Gm8RpGt25Hr6NQe95PxUl10g3TCCIqpi12RG4ZSGvpddzE9zJuOIEll1A9mzoBjDMIZPcUhdMoPHXp1PKAf7WeuMz1n8mIaFAetgsm8mV3QLEidnyZBbk4BoBc8+mjj/aagZ8YprBwqniD4QmRqQEADCBg4IlCKgynESq/mcDV+Mn4T7pGqX0Pg8s2Cun/AM8YV7bZKOf7olvXnW1qE8D3utAq9IogvqijmlrbaXpKXMWLBfwGsPh4/GZ9mKhwVui9DcCdRiT7fuKQPsYKcazwwucUFf7dBg8qPEZcuBdiBM2iEplZtNwr2VkNXAwOHq3wcfxOw38Nk5jGSf8QGND4gQXUl0Ngi0PiMUO49odDb9tQ+3jWacamKSAKjKkLWNhC3mvcgxkz7GzG5oNrb0X8Ggec157LAgc9hh/KekLxIChbaG79/UyzRNiva6M32T8dMS8I0nqixu5Hvm05P8IjHIyCaX/9mqbwXbkXEwSm1f6UODFaDXHdVOXXJIbL/40076+BiiAQDEnj5dEELhZw4MnGX+4Dx0d+F4rKS/8XCBFmgOKD8D+7bNR4Tni52TU5i04aXXsaV0Lo8HDhfZG+/nsf0+LDLpTRiNxQKzc HpbM/cic 0pBySnA1auP5KsbBNtOZOCa1Ra8XKOVUl69V3+pv8lkZIUS3LPcXbkwoR9mSLWHZBKxhtqP9OU/dx9OS8BvjeZ9D4TGvIWOnYbdUdDH1CQjvko0aRpaFAs7V+VkxJB2hp4eoxpq+RUUWqKjUhFAwfNU2LfqswNjexMr3kBvTzVFkHdUb4WNctSsf8taVT4sZbwNVzCJL/etsXtAgbdjShlUfpx/0hweGrZHWdjzLWIk/VZYQieOBNwQ3nHXyGHBaOmmw+x/RBaLPT2s5x7mBMPyKae18bPBVUjby56dMlDVlqZh3FiWdld64ykr+SQi61LmJwxBE7MpGETdOM1e8bUOJF2JuKRCLaFxGKGIGkhss2GYcQyb4q317tskSy16PsAq6pYRVYZTA8J+Q7vkr1qDHfJO1pi1yS54NFm+3nact0vnUdtTa3/8oPmX5AhzKJ8DuX/0miPp1FsmU= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000073, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Aug 22, 2023 at 8:55=E2=80=AFAM Nhat Pham wrote= : > > On Mon, Aug 21, 2023 at 9:05=E2=80=AFAM Andre Przywara wrote: > > > > The cachestat kselftest runs a test on a normal file, which is created > > temporarily in the current directory. Among the tests it runs there is = a > > call to fsync(), which is expected to clean all dirty pages used by the > > file. > > However the tmpfs filesystem implements fsync() as noop_fsync(), so the > > call will not even attempt to clean anything when this test file happen= s > > to live on a tmpfs instance. This happens in an initramfs, or when the > > current directory is in /dev/shm or sometimes /tmp. > > > > To avoid this test failing wrongly, use statfs() to check which filesys= tem > > the test file lives on. If that is "tmpfs", we skip the fsync() test. > > > > Since the fsync test is only one part of the "normal file" test, we now > > execute this twice, skipping the fsync part on the first call. > > This way only the second test, including the fsync part, would be skipp= ed. > > > > Signed-off-by: Andre Przywara > > --- > > .../selftests/cachestat/test_cachestat.c | 62 ++++++++++++++----- > > 1 file changed, 47 insertions(+), 15 deletions(-) > > > > diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools= /testing/selftests/cachestat/test_cachestat.c > > index 8f8f46c24846d..4804c7dc7b312 100644 > > --- a/tools/testing/selftests/cachestat/test_cachestat.c > > +++ b/tools/testing/selftests/cachestat/test_cachestat.c > > @@ -4,10 +4,12 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -15,7 +17,7 @@ > > > > #include "../kselftest.h" > > > > -#define NR_TESTS 8 > > +#define NR_TESTS 9 > > > > static const char * const dev_files[] =3D { > > "/dev/zero", "/dev/null", "/dev/urandom", > > @@ -91,6 +93,20 @@ bool write_exactly(int fd, size_t filesize) > > return ret; > > } > > > > +/* > > + * fsync() is implemented via noop_fsync() on tmpfs. This makes the fs= ync() > > + * test fail below, so we need to check for test file living on a tmpf= s. > > + */ > > +static bool is_on_tmpfs(int fd) > > +{ > > + struct statfs statfs_buf; > > + > > + if (fstatfs(fd, &statfs_buf)) > > + return false; > > + > > + return statfs_buf.f_type =3D=3D TMPFS_MAGIC; > > +} > > + > > /* > > * Open/create the file at filename, (optionally) write random data to= it > > * (exactly num_pages), then test the cachestat syscall on this file. > > @@ -98,13 +114,13 @@ bool write_exactly(int fd, size_t filesize) > > * If test_fsync =3D=3D true, fsync the file, then check the number of= dirty > > * pages. > > */ > > -bool test_cachestat(const char *filename, bool write_random, bool crea= te, > > - bool test_fsync, unsigned long num_pages, int open_flag= s, > > - mode_t open_mode) > > +static int test_cachestat(const char *filename, bool write_random, boo= l create, > > + bool test_fsync, unsigned long num_pages, > > + int open_flags, mode_t open_mode) > > { > Hmm would the semantic change a bit here? > > Previously, this function returned true if passed. > But it seems like KSFT_PASS is defined as 0: > https://github.com/torvalds/linux/blob/9e38be7/tools/testing/selftests/ks= elftest.h#L74 > > So maybe we have to change from: > > if (test_()) > > > to: > > if (!test_)()) > > or, explicitly as: > > if (test_() =3D=3D KSFT_PASS) Ah never mind, ignore this. I didn't see your change down in the main function. Everything LGTM! Acked-by: Nhat Pham > > > > size_t PS =3D sysconf(_SC_PAGESIZE); > > int filesize =3D num_pages * PS; > > - bool ret =3D true; > > + int ret =3D KSFT_PASS; > > long syscall_ret; > > struct cachestat cs; > > struct cachestat_range cs_range =3D { 0, filesize }; > > @@ -113,7 +129,7 @@ bool test_cachestat(const char *filename, bool writ= e_random, bool create, > > > > if (fd =3D=3D -1) { > > ksft_print_msg("Unable to create/open file.\n"); > > - ret =3D false; > > + ret =3D KSFT_FAIL; > > goto out; > > } else { > > ksft_print_msg("Create/open %s\n", filename); > > @@ -122,7 +138,7 @@ bool test_cachestat(const char *filename, bool writ= e_random, bool create, > > if (write_random) { > > if (!write_exactly(fd, filesize)) { > > ksft_print_msg("Unable to access urandom.\n"); > > - ret =3D false; > > + ret =3D KSFT_FAIL; > > goto out1; > > } > > } > > @@ -133,7 +149,7 @@ bool test_cachestat(const char *filename, bool writ= e_random, bool create, > > > > if (syscall_ret) { > > ksft_print_msg("Cachestat returned non-zero.\n"); > > - ret =3D false; > > + ret =3D KSFT_FAIL; > > goto out1; > > > > } else { > > @@ -143,15 +159,17 @@ bool test_cachestat(const char *filename, bool wr= ite_random, bool create, > > if (cs.nr_cache + cs.nr_evicted !=3D num_pages)= { > > ksft_print_msg( > > "Total number of cached and evi= cted pages is off.\n"); > > - ret =3D false; > > + ret =3D KSFT_FAIL; > > } > > } > > } > > > > if (test_fsync) { > > - if (fsync(fd)) { > > + if (is_on_tmpfs(fd)) { > > + ret =3D KSFT_SKIP; > > + } else if (fsync(fd)) { > > ksft_print_msg("fsync fails.\n"); > > - ret =3D false; > > + ret =3D KSFT_FAIL; > > } else { > > syscall_ret =3D syscall(__NR_cachestat, fd, &cs= _range, &cs, 0); > > > > @@ -162,13 +180,13 @@ bool test_cachestat(const char *filename, bool wr= ite_random, bool create, > > print_cachestat(&cs); > > > > if (cs.nr_dirty) { > > - ret =3D false; > > + ret =3D KSFT_FAIL; > > ksft_print_msg( > > "Number of dirty should= be zero after fsync.\n"); > > } > > } else { > > ksft_print_msg("Cachestat (after fsync)= returned non-zero.\n"); > > - ret =3D false; > > + ret =3D KSFT_FAIL; > > goto out1; > > } > > } > > @@ -259,7 +277,7 @@ int main(void) > > const char *dev_filename =3D dev_files[i]; > > > > if (test_cachestat(dev_filename, false, false, false, > > - 4, O_RDONLY, 0400)) > > + 4, O_RDONLY, 0400) =3D=3D KSFT_PASS) > > ksft_test_result_pass("cachestat works with %s\= n", dev_filename); > > else { > > ksft_test_result_fail("cachestat fails with %s\= n", dev_filename); > > @@ -268,13 +286,27 @@ int main(void) > > } > > > > if (test_cachestat("tmpfilecachestat", true, true, > > - true, 4, O_CREAT | O_RDWR, 0400 | 0600)) > > + false, 4, O_CREAT | O_RDWR, 0600) =3D=3D KSFT_PASS) > > ksft_test_result_pass("cachestat works with a normal fi= le\n"); > > else { > > ksft_test_result_fail("cachestat fails with normal file= \n"); > > ret =3D 1; > > } > > > > + switch (test_cachestat("tmpfilecachestat", true, true, > > + true, 4, O_CREAT | O_RDWR, 0600)) { > > + case KSFT_FAIL: > > + ksft_test_result_fail("cachestat fsync fails with norma= l file\n"); > > + ret =3D KSFT_FAIL; > > + break; > > + case KSFT_PASS: > > + ksft_test_result_pass("cachestat fsync works with a nor= mal file\n"); > > + break; > > + case KSFT_SKIP: > > + ksft_test_result_skip("tmpfilecachestat is on tmpfs\n")= ; > > + break; > > + } > > + > > if (test_cachestat_shmem()) > > ksft_test_result_pass("cachestat works with a shmem fil= e\n"); > > else { > > -- > > 2.25.1 > >