Message ID | 5f80f3dae60f77c6b746578113e56c8fa6454143.1687344643.git.falcon@tinylab.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp4362614vqr; Wed, 21 Jun 2023 06:25:45 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7sizVkg6GVJf8QoZQhgV+bddL7auiRGHzxe2eHAcYLM5aMrNGlINRtzlXV2v5tZqvXhyIG X-Received: by 2002:a9d:7a83:0:b0:6b4:aaf0:7449 with SMTP id l3-20020a9d7a83000000b006b4aaf07449mr8752580otn.3.1687353945289; Wed, 21 Jun 2023 06:25:45 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687353945; cv=none; d=google.com; s=arc-20160816; b=nnOuAyCro68ZzpCKTtU0oF9K9MJ5TFW67bukGx8YGeOp8StTQ52bwuHXUVG8i2+mXx zt/mtkX7EZAaeZu17owqqU07HlIcXqtbKq19OhA7wT4QuToohrANYToJGNf13V4m534K F3uVWiXfgbNp3/EzP3G66gcK6VfIcSrNd+7TLRRq4VxK8gu/Qr/nSFOQDoTzppX62ubI WnQPZxHYmg0rNtxfJTWgkLH2wUOpy9Q7G20VNKTzcFCaFCH3kTUk9q8WR1xk/+e13Go5 1JaLvHAI6L2tYx/t6h1fiL5zLNslQtNZnWFLUSL1BYV27hXZIjPtKnzqkL/s+MISieiB Ajgw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:feedback-id:content-transfer-encoding :mime-version:references:in-reply-to:message-id:date:subject:cc:to :from; bh=dtdMmwUqOkXMJsivwEbVxmjyaY7/99ZHofSphWqFuTg=; b=T6IYMOgTg3bOBLdqudvr1WwiF5hRh8eXO2bWLJf9EV8V9fOq4OyBCYO/TPLda66vao kh6JgsUK9JJCg8yrSchkBxgLen/rb+mahrtjxS+Msnwq8KwGXJ5r4QjNrNZGXAB1+YGM Olv7DmVsM0Xo0i8Zn+EfNldsnRZtRfSGoIqGrH5hl/odA6deGf4fM1vwzEl94LECUlWk lv84TaolmTCv1gRPvVCLD9oBFaZQrq7lV7yPL3dmZKF3YuVqPQCWhHU9i2Py6o5T8Q8z SH669IoCxmnY14bYNmP/f+omYhLf07vix6pZgEIy5n40+EFwxcL+FF7B3BTuX2vu5aC8 TKhQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id i68-20020a639d47000000b005347c4d09ddsi4183661pgd.678.2023.06.21.06.25.32; Wed, 21 Jun 2023 06:25:45 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230013AbjFUM7n (ORCPT <rfc822;maxin.john@gmail.com> + 99 others); Wed, 21 Jun 2023 08:59:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37146 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231156AbjFUM7l (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 21 Jun 2023 08:59:41 -0400 Received: from bg4.exmail.qq.com (bg4.exmail.qq.com [43.155.65.254]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F326FAC; Wed, 21 Jun 2023 05:59:38 -0700 (PDT) X-QQ-mid: bizesmtp73t1687352369te3lj68z Received: from linux-lab-host.localdomain ( [116.30.126.60]) by bizesmtp.qq.com (ESMTP) with id ; Wed, 21 Jun 2023 20:59:28 +0800 (CST) X-QQ-SSF: 01200000000000D0V000000A0000000 X-QQ-FEAT: xwvWJGGFd7Pp39HRCTQYRbgJy3bbvGHQ7WlZR48+2RLviBp/41ZiQ96oKRZr/ SilZzV0NenGnJQOAoEBB0FsmiBH7+SiFV9oelPczpDc5Rv6vON7sAY8uZqZwSd+t40quVga UlD5ndGNbGYHcYN65235Szzzn+W3uQJbRs+cM5+xVrvNtkhz8C96FkkUB4g30N+3iiIrT18 IQ6Ef+55Bvl/Wb4QA6h+fvhx0tjuE67lIQ90/3id1+HCtIisliMmEsk8Ug9bqvYo0ghVbrP B7xN3wcVi695bHsw5s8IapJuwl+GKCEXCNiaucghBROurHs1Z2GS5Y9guTFLTImqjkXOMkJ m22jHKU242jcgYr6CLmUguUae2HlOWvJPWimeF9ZtAi9dq0bot3TJaA2a7Ar64D7EGcWIyU X-QQ-GoodBg: 0 X-BIZMAIL-ID: 18004566144104333474 From: Zhangjin Wu <falcon@tinylab.org> To: w@1wt.eu Cc: thomas@t-8ch.de, arnd@arndb.de, falcon@tinylab.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v1 05/17] selftests/nolibc: stat_timestamps: remove procfs dependency Date: Wed, 21 Jun 2023 20:58:58 +0800 Message-Id: <5f80f3dae60f77c6b746578113e56c8fa6454143.1687344643.git.falcon@tinylab.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <cover.1687344643.git.falcon@tinylab.org> References: <cover.1687344643.git.falcon@tinylab.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:tinylab.org:qybglogicsvrsz:qybglogicsvrsz3a-3 X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769318850626339350?= X-GMAIL-MSGID: =?utf-8?q?1769318850626339350?= |
Series |
selftests/nolibc: allow run with minimal kernel config
|
|
Commit Message
Zhangjin Wu
June 21, 2023, 12:58 p.m. UTC
Since it is not really necessary to use /proc/self here, instead of
adding a condition check, we use the always existing '/' path instead of
/proc/self, this eventually let it work without procfs.
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/testing/selftests/nolibc/nolibc-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
Hi, Thomas I'm preparing a revision for this series, in the past days, when I was working on testing our new 'minimal' kernel config support for all of the architectures, the time cost (and wait) is really appreciable and the repeated develop and test is really a big pain, I can also image when you was working on stack-protector and Willy was working on lots of old features ;-) As you explained before, I knew the idea of using '/proc/self' here is important to not using a fixed-time file, besides our proposed method (make sure it at least not fail, just skip for !procfs): - CASE_TEST(stat_timestamps); EXPECT_SYSZR(1, test_stat_timestamps()); break; + CASE_TEST(stat_timestamps); EXPECT_SYSZR(proc, test_stat_timestamps()); break; To further avoid skip it for !procfs (I don't mean relaly disable it for the default tinyconfig support, which need more discuss, at least provide the possibility to pass without procfs), do you like this change? it doesn't depend on 'proc' now. - if (stat("/proc/self/", &st)) + if (stat("/proc/self/", &st) && stat("/init", &st) && stat("/", &st)) The "/init" is compiled for 'run' target every time, so, the time stamp should be dynamic enough, for libc-test, the /proc/self should be always there (if still not enough, we can reuse the init file list here), the "/" here is only for the worst-case scene ;-) Thanks, Zhangjin > Since it is not really necessary to use /proc/self here, instead of > adding a condition check, we use the always existing '/' path instead of > /proc/self, this eventually let it work without procfs. > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org> > --- > tools/testing/selftests/nolibc/nolibc-test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c > index ebec948ec808..2ef44176f7a9 100644 > --- a/tools/testing/selftests/nolibc/nolibc-test.c > +++ b/tools/testing/selftests/nolibc/nolibc-test.c > @@ -520,7 +520,7 @@ static int test_stat_timestamps(void) > if (sizeof(st.st_atim.tv_sec) != sizeof(st.st_atime)) > return 1; > > - if (stat("/proc/self/", &st)) > + if (stat("/", &st)) > return 1; > > if (st.st_atim.tv_sec != st.st_atime || st.st_atim.tv_nsec > 1000000000) > -- > 2.25.1
Hi Zhangjin, On 2023-06-28 21:59:22+0800, Zhangjin Wu wrote: > I'm preparing a revision for this series, in the past days, when I was > working on testing our new 'minimal' kernel config support for all of > the architectures, the time cost (and wait) is really appreciable and the > repeated develop and test is really a big pain, I can also image when you > was working on stack-protector and Willy was working on lots of old > features ;-) To be honest I almost never built a kernel. Most of the time I tested my stuff with qemu-user. This made the dev-cycle really fast, especially with a binfmt setup that launches foreign binaries automatically with qemu-user. > As you explained before, I knew the idea of using '/proc/self' here is > important to not using a fixed-time file, besides our proposed method (make > sure it at least not fail, just skip for !procfs): > > - CASE_TEST(stat_timestamps); EXPECT_SYSZR(1, test_stat_timestamps()); break; > + CASE_TEST(stat_timestamps); EXPECT_SYSZR(proc, test_stat_timestamps()); break; > > To further avoid skip it for !procfs (I don't mean relaly disable it for the > default tinyconfig support, which need more discuss, at least provide the > possibility to pass without procfs), do you like this change? it doesn't depend > on 'proc' now. > > - if (stat("/proc/self/", &st)) > + if (stat("/proc/self/", &st) && stat("/init", &st) && stat("/", &st)) > > The "/init" is compiled for 'run' target every time, so, the time stamp should > be dynamic enough, for libc-test, the /proc/self should be always there (if > still not enough, we can reuse the init file list here), the "/" here is only > for the worst-case scene ;-) Both aproaches seem fine. Just skipping on !proc seems good enough. As for enabling proc in the test configs I just tested a plain tinyconfig vs one with CONFIG_PROC_FS enabled: tinyconfig: 375.06user 53.21system 2:05.80elapsed tinyconfig + CONFIG_PROC_FS: 397.77user 56.84system 2:09.24elapsed The overhead seems acceptable. Note as for disabling memfd: It seems currently MEMFD_CREATE is hardwired to only be enabled when either TMPFS or HUGETLBFS is enabled. But the memfd code and syscalls seem to work perfectly fine with those options disabled. I'll send a patch to fix up the Kconfigs to enable that usecase. > Thanks, > Zhangjin > > > Since it is not really necessary to use /proc/self here, instead of > > adding a condition check, we use the always existing '/' path instead of > > /proc/self, this eventually let it work without procfs. > > > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org> > > --- > > tools/testing/selftests/nolibc/nolibc-test.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c > > index ebec948ec808..2ef44176f7a9 100644 > > --- a/tools/testing/selftests/nolibc/nolibc-test.c > > +++ b/tools/testing/selftests/nolibc/nolibc-test.c > > @@ -520,7 +520,7 @@ static int test_stat_timestamps(void) > > if (sizeof(st.st_atim.tv_sec) != sizeof(st.st_atime)) > > return 1; > > > > - if (stat("/proc/self/", &st)) > > + if (stat("/", &st)) > > return 1; > > > > if (st.st_atim.tv_sec != st.st_atime || st.st_atim.tv_nsec > 1000000000) > > -- > > 2.25.1
> Hi Zhangjin, > > On 2023-06-28 21:59:22+0800, Zhangjin Wu wrote: > > I'm preparing a revision for this series, in the past days, when I was > > working on testing our new 'minimal' kernel config support for all of > > the architectures, the time cost (and wait) is really appreciable and the > > repeated develop and test is really a big pain, I can also image when you > > was working on stack-protector and Willy was working on lots of old > > features ;-) > > To be honest I almost never built a kernel. > Most of the time I tested my stuff with qemu-user. > This made the dev-cycle really fast, especially with a binfmt setup that > launches foreign binaries automatically with qemu-user. > Yeah, qemu-user-static + binfmt_misc work perfectly, but my host kernel is not that new, so, I'm still a little worried about that there may be some hidden issues. > > As you explained before, I knew the idea of using '/proc/self' here is > > important to not using a fixed-time file, besides our proposed method (make > > sure it at least not fail, just skip for !procfs): > > > > - CASE_TEST(stat_timestamps); EXPECT_SYSZR(1, test_stat_timestamps()); break; > > + CASE_TEST(stat_timestamps); EXPECT_SYSZR(proc, test_stat_timestamps()); break; > > > > To further avoid skip it for !procfs (I don't mean relaly disable it for the > > default tinyconfig support, which need more discuss, at least provide the > > possibility to pass without procfs), do you like this change? it doesn't depend > > on 'proc' now. > > > > - if (stat("/proc/self/", &st)) > > + if (stat("/proc/self/", &st) && stat("/init", &st) && stat("/", &st)) > > > > The "/init" is compiled for 'run' target every time, so, the time stamp should > > be dynamic enough, for libc-test, the /proc/self should be always there (if > > still not enough, we can reuse the init file list here), the "/" here is only > > for the worst-case scene ;-) > > Both aproaches seem fine. Just skipping on !proc seems good enough. > To get less skips, let's use the second method, just updated my local patches ;-) > As for enabling proc in the test configs I just tested a plain > tinyconfig vs one with CONFIG_PROC_FS enabled: > > tinyconfig: 375.06user 53.21system 2:05.80elapsed > tinyconfig + CONFIG_PROC_FS: 397.77user 56.84system 2:09.24elapsed > > The overhead seems acceptable. > Yeah, only one option is ok, but "multiple options x multiple architectures x multiple repeated runs", that is 'huge' ;-) > > Note as for disabling memfd: > > It seems currently MEMFD_CREATE is hardwired to only be enabled when > either TMPFS or HUGETLBFS is enabled. > > But the memfd code and syscalls seem to work perfectly fine with those > options disabled. I'll send a patch to fix up the Kconfigs to enable > that usecase. Good catch! but for the vfprintf test cases, It is able to open a file from tmpfs directly. If no tmpfs, use the default ramfs (initramfs uses) instead, this will also avoid the new flags trying (to silence the warning). static int expect_vfprintf(int llen, size_t c, const char *expected, const char *fmt, ...) { + static const char *tmpfile = "/tmp/nolibc-vfprintf"; + struct stat stat_buf; int ret, fd, w, r; char buf[100]; FILE *memfile; va_list args; - fd = memfd_create("vfprintf", 0); + if (stat("/tmp/.", &stat_buf)) { + pad_spc(llen, 64, "[SKIPPED]\n"); + return 0; + } + + fd = open(tmpfile, O_CREAT | O_TRUNC | O_RDWR, 0755); ... + unlink(tmpfile); ... tmpfs is mounted (in another patch) like procfs in prepare() for pid==1. I plan to use this method in the revision, do you like this? memfd_create() was designed to do this work, but in current stage, opening tmpfile ourselves may be better. Thanks, Zhangjin
On 2023-06-30 05:23:35+0800, Zhangjin Wu wrote: > > Hi Zhangjin, > > > > On 2023-06-28 21:59:22+0800, Zhangjin Wu wrote: > > > I'm preparing a revision for this series, in the past days, when I was > > > working on testing our new 'minimal' kernel config support for all of > > > the architectures, the time cost (and wait) is really appreciable and the > > > repeated develop and test is really a big pain, I can also image when you > > > was working on stack-protector and Willy was working on lots of old > > > features ;-) > > > > To be honest I almost never built a kernel. > > Most of the time I tested my stuff with qemu-user. > > This made the dev-cycle really fast, especially with a binfmt setup that > > launches foreign binaries automatically with qemu-user. > > > > Yeah, qemu-user-static + binfmt_misc work perfectly, but my host kernel > is not that new, so, I'm still a little worried about that there may be > some hidden issues. qemu-user shouldn't have specific requirements for the host kernel. Could you elaborate? > > > As you explained before, I knew the idea of using '/proc/self' here is > > > important to not using a fixed-time file, besides our proposed method (make > > > sure it at least not fail, just skip for !procfs): > > > > > > - CASE_TEST(stat_timestamps); EXPECT_SYSZR(1, test_stat_timestamps()); break; > > > + CASE_TEST(stat_timestamps); EXPECT_SYSZR(proc, test_stat_timestamps()); break; > > > > > > To further avoid skip it for !procfs (I don't mean relaly disable it for the > > > default tinyconfig support, which need more discuss, at least provide the > > > possibility to pass without procfs), do you like this change? it doesn't depend > > > on 'proc' now. > > > > > > - if (stat("/proc/self/", &st)) > > > + if (stat("/proc/self/", &st) && stat("/init", &st) && stat("/", &st)) > > > > > > The "/init" is compiled for 'run' target every time, so, the time stamp should > > > be dynamic enough, for libc-test, the /proc/self should be always there (if > > > still not enough, we can reuse the init file list here), the "/" here is only > > > for the worst-case scene ;-) > > > > Both aproaches seem fine. Just skipping on !proc seems good enough. > > > > To get less skips, let's use the second method, just updated my local > patches ;-) > > > As for enabling proc in the test configs I just tested a plain > > tinyconfig vs one with CONFIG_PROC_FS enabled: > > > > tinyconfig: 375.06user 53.21system 2:05.80elapsed > > tinyconfig + CONFIG_PROC_FS: 397.77user 56.84system 2:09.24elapsed > > > > The overhead seems acceptable. > > > > Yeah, only one option is ok, but "multiple options x multiple > architectures x multiple repeated runs", that is 'huge' ;-) In your other patchset you mentioned a few options that were needed. But if we can drop CONFIG_NET completely, reduce CONFIG_TMPFS to CONFIG_MEMFD I would like to reevaluate the overall impact. > > > > Note as for disabling memfd: > > > > It seems currently MEMFD_CREATE is hardwired to only be enabled when > > either TMPFS or HUGETLBFS is enabled. > > > > But the memfd code and syscalls seem to work perfectly fine with those > > options disabled. I'll send a patch to fix up the Kconfigs to enable > > that usecase. > > Good catch! > > but for the vfprintf test cases, It is able to open a file from tmpfs > directly. If no tmpfs, use the default ramfs (initramfs uses) instead, > this will also avoid the new flags trying (to silence the warning). > > static int expect_vfprintf(int llen, size_t c, const char *expected, const char *fmt, ...) > { > + static const char *tmpfile = "/tmp/nolibc-vfprintf"; > + struct stat stat_buf; This should not be static. > int ret, fd, w, r; > char buf[100]; > FILE *memfile; > va_list args; > > - fd = memfd_create("vfprintf", 0); > + if (stat("/tmp/.", &stat_buf)) { > + pad_spc(llen, 64, "[SKIPPED]\n"); > + return 0; > + } Instead of checking with stat() here it would be nicer to check the result of open() below. > + > + fd = open(tmpfile, O_CREAT | O_TRUNC | O_RDWR, 0755); Seems like a good usecase for open("/tmp", O_TMPFILE | O_EXCL | ...) > ... > + unlink(tmpfile); ... and drop this. fd = memfd_open(); + if (fd == -1) + fd = open("/tmp", O_TMPFILE | O_EXCL | O_CREAT | O_RDWR, 0600); + if (fd == -1) + skip() May be enough. > ... > > tmpfs is mounted (in another patch) like procfs in prepare() for pid==1. > > I plan to use this method in the revision, do you like this? > > memfd_create() was designed to do this work, but in current stage, > opening tmpfile ourselves may be better. I'm not really a fan of having so much fallback code for features that should always be available. But if we can keep it straightforward it's probably fine. Or Willy has another opinion :-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index ebec948ec808..2ef44176f7a9 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -520,7 +520,7 @@ static int test_stat_timestamps(void) if (sizeof(st.st_atim.tv_sec) != sizeof(st.st_atime)) return 1; - if (stat("/proc/self/", &st)) + if (stat("/", &st)) return 1; if (st.st_atim.tv_sec != st.st_atime || st.st_atim.tv_nsec > 1000000000)