Re: [PATCH v1 00/17] selftests/nolibc: allow run with minimal kernel config

From: Thomas Weißschuh
Date: Wed Jun 21 2023 - 15:22:07 EST

Next message: T.J. Mercier: "Re: [PATCH v4 2/6] dma-buf/heaps: Don't assert held reservation lock for dma-buf mmapping"
Previous message: Nathan Chancellor: "Re: [PATCH v7 10/19] s390: mm: Convert to GENERIC_IOREMAP"
In reply to: Zhangjin Wu: "[PATCH v1 17/17] selftests/nolibc: vfprintf: support tmpfs and hugetlbfs"
Next in thread: Zhangjin Wu: "Re: [PATCH v1 00/17] selftests/nolibc: allow run with minimal kernel config"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Zhangjin,

some general comments for the whole series.

On 2023-06-21 20:52:30+0800, Zhangjin Wu wrote:
> Hi, Willy
>
> This patchset mainly allows speed up the nolibc test with a minimal
> kernel config.
>
> As the nolibc supported architectures become more and more, the 'run'
> test with DEFCONFIG may cost several hours, which is not friendly to
> develop testing and even for release testing, so, smaller kernel configs
> may be required, and firstly, we should let nolibc-test work with less
> kernel config options, this patchset aims to this goal.
>
> This patchset mainly remove the dependency from procfs, tmpfs, net and
> memfd_create, many failures have been fixed up.
>
> When CONFIG_TMPFS and CONFIG_SHMEM are disabled, kernel will provide a
> ramfs based tmpfs (mm/shmem.c), it will be used as a choice to fix up
> some failures and also allow skip less tests.

Did you look into how much this duplicates from the kernels already
existing "tinyconfig" and "kvm_guest.config" functionality?

And it would be interesting how much impact the enablement of procfs,
tmpfs, net and memfd_create has in constrast to the minimal
configuration.
It seems unfortunate to me to complicate the testsuite to handle such
uncommon scenarios.

> Besides, it also adds musl support, improves glibc support and fixes up
> a kernel cmdline passing use case.
>
> This is based on the dev.2023.06.14a branch of linux-rcu [1], all of the
> supported architectures are tested (with local minimal configs, [5]
> pasted the one for i386) without failures:
>
> arch/board | result
> ------------|------------
> arm/vexpress-a9 | 138 test(s) passed, 1 skipped, 0 failed. See all results in /labs/linux-lab/logging/nolibc/arm-vexpress-a9-nolibc-test.log
> aarch64/virt | 138 test(s) passed, 1 skipped, 0 failed. See all results in /labs/linux-lab/logging/nolibc/aarch64-virt-nolibc-test.log
> ppc/g3beige | not supported
> i386/pc | 136 test(s) passed, 3 skipped, 0 failed. See all results in /labs/linux-lab/logging/nolibc/i386-pc-nolibc-test.log
> x86_64/pc | 138 test(s) passed, 1 skipped, 0 failed. See all results in /labs/linux-lab/logging/nolibc/x86_64-pc-nolibc-test.log
> mipsel/malta | 138 test(s) passed, 1 skipped, 0 failed. See all results in /labs/linux-lab/logging/nolibc/mipsel-malta-nolibc-test.log
> loongarch64/virt | 138 test(s) passed, 1 skipped, 0 failed. See all results in /labs/linux-lab/logging/nolibc/loongarch64-virt-nolibc-test.log
> riscv64/virt | 136 test(s) passed, 3 skipped, 0 failed. See all results in /labs/linux-lab/logging/nolibc/riscv64-virt-nolibc-test.log
> riscv32/virt | no test log found
> s390x/s390-ccw-virtio | 138 test(s) passed, 1 skipped, 0 failed. See all results in /labs/linux-lab/logging/nolibc/s390x-s390-ccw-virtio-nolibc-test.log
>
> Notes:
> * The skipped ones are -fstackprotector, chmod_self and chown_self
>
> The -fstackprotector skip is due to gcc version.
> chmod_self and chmod_self skips are due to procfs not enabled
>
> * ppc/g3beige support is added locally, but not added in this patchset
>
> will send ppc support as a new patchset, it depends on v2 test
> report patchset [3] and the v5 rv32 support, require changes on
> Makefile
>
> * riscv32/virt support is still in review, see v5 rv32 support [4]
>
> This patchset doesn't depends on any of my other nolibc patch series,
> but the new rmdir() routine added in this patchset may be requird to
> apply the __sysret() from our v4 syscall helper series [2] after that
> series being merged, currently, we use the old method to let it compile
> without any dependency.
>
> Here explains all of the patches:
>
> * selftests/nolibc: stat_fault: silence NULL argument warning with glibc
> selftests/nolibc: gettid: restore for glibc and musl
> selftests/nolibc: add _LARGEFILE64_SOURCE for musl
>
> The above 3 patches adds musl compile support and improve glibc support.
>
> It is able to build and run nolibc-test with musl libc now, but there
> are some failures/skips due to the musl its own issues/requirements:
>
> $ sudo ./nolibc-test | grep -E 'FAIL|SKIP'
> 8 sbrk = 1 ENOMEM [FAIL]
> 9 brk = -1 ENOMEM [FAIL]
> 46 limit_int_fast16_min = -2147483648 [FAIL]
> 47 limit_int_fast16_max = 2147483647 [FAIL]
> 49 limit_int_fast32_min = -2147483648 [FAIL]
> 50 limit_int_fast32_max = 2147483647 [FAIL]
> 0 -fstackprotector not supported [SKIPPED]
>
> musl disabled sbrk and brk for some conflicts with its malloc and the
> fast version of int types are defined in 32bit, which differs from nolibc
> and glibc. musl reserved the sbrk(0) to allow get current brk, we
> added a test for this in the v4 __sysret() helper series [2].

We could add new macros

#define UINT_MAX(t) (~(t)0)
#define SINT_MAX(t) (((t)1 << (sizeof(t) * 8 - 2)) - (t)1 + ((t)1 << (sizeof(t) * 8 - 2)))

to get whatever is appropriate for the respective type.

>
> * selftests/nolibc: fix up kernel parameters support
>
> kernel cmdline allows pass two types of parameters, one is without
> '=', another is with '=', the first one is passed as init arguments,
> the sencond one is passed as init environment variables.
>
> Our nolibc-test prefer arguments to environment variables, this not
> work when users add such parameters in the kernel cmdline:
>
> noapic NOLIBC_TEST=syscall
>
> So, this patch will verify the setting from arguments at first, if it
> is no valid, will try the environment variables instead.

This would be much simpler as:

test = getenv("NOLIBC_TEST");
if (!test)
test = argv[1];

It changes the semantics a bit, but it doesn't seem to be an issue.
(Maybe gated behind getpid() == 1).

> * selftests/nolibc: stat_timestamps: remove procfs dependency
>
> Use '/' instead of /proc/self, or we can add a 'has_proc' condition
> for this test case, but it is not that necessary to skip the whole
> stat_timestamps tests for such a subtest binding to /proc/self.
>
> Welcome suggestion from Thomas.

As above, I think the impact of depending on CONFIG_PROC_FS is
justifiable.

The usage of /proc/self was actually intentional.
This file has a timestamp of the start of the referenced process.
So each invocation of nolibc-test tests a new timestamp.

In contrast if nolibc-test is invocated from a prebaked filesystem the
timestamp of "/" will always be fixed, reducing the chance to find
errors.

> * tools/nolibc: add rmdir() support
> selftests/nolibc: add a new rmdir() test case
>
> rmdir() routine and test case are added for the coming requirement.
>
> Note, if the __sysret() patchset [2] is applied before us, this patch
> should be rebased on it and apply the __sysret() helper.
>
> * selftests/nolibc: fix up failures when there is no procfs
>
> call rmdir() to remove /proc completely to rework the checking of
> /proc, before, the existing of /proc not means the procfs is really
> mounted.
>
> * selftests/nolibc: rename proc variable to has_proc
> selftests/nolibc: rename euid0 variable to is_root
>
> align with the has_gettid, has_xxx variables.
>
> * selftests/nolibc: prepare tmpfs and hugetlbfs
> selftests/nolibc: rename chmod_net to chmod_good
> selftests/nolibc: link_cross: support tmpfs
> selftests/nolibc: rename chroot_exe to chroot_file
>
> use file from /tmp instead of file from /proc when there is no procfs
> this avoid skipping the chmod_net, link_cross, chroot_exe tests
>
> * selftests/nolibc: vfprintf: silence memfd_create() warning
> selftests/nolibc: vfprintf: skip if neither tmpfs nor hugetlbfs
> selftests/nolibc: vfprintf: support tmpfs and hugetlbfs
>
> memfd_create from kernel >= v6.2 forcely warn on missing
> MFD_NOEXEC_SEAL flag, the first one silence it with such flag, for
> older kernels, use 0 flag as before.

Given this is only a problem when nolibc-test is PID1 and printing to
the system console, we could also just disable warnings on the system
console through syscall() or /proc/sys/kernel/printk.

It would also avoid cluttering the tests for limited gain.

> since memfd_create() depends on TMPFS or HUGETLBFS, the second one
> skip the whole vfprintf instead of simply fail if memfd_create() not
> work.
>
> the 3rd one futher try the ramfs based tmpfs even when memfd_create()
> not work.
>
> At last, let's simply discuss about the configs, I have prepared minimal
> configs for all of the nolibc supported architectures but not sure where
> should we put them, what about tools/testing/selftests/nolibc/configs ?
>
> Thanks!
>
> Best regards,
> Zhangjin
> ---
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/
> [2]: https://lore.kernel.org/linux-riscv/cover.1687187451.git.falcon@xxxxxxxxxxx/
> [3]: https://lore.kernel.org/lkml/cover.1687156559.git.falcon@xxxxxxxxxxx/
> [4]: https://lore.kernel.org/linux-riscv/cover.1687176996.git.falcon@xxxxxxxxxxx/
> [5]: https://pastebin.com/5jq0Vxbz
>
> Zhangjin Wu (17):
> selftests/nolibc: stat_fault: silence NULL argument warning with glibc
> selftests/nolibc: gettid: restore for glibc and musl
> selftests/nolibc: add _LARGEFILE64_SOURCE for musl
> selftests/nolibc: fix up kernel parameters support
> selftests/nolibc: stat_timestamps: remove procfs dependency
> tools/nolibc: add rmdir() support
> selftests/nolibc: add a new rmdir() test case
> selftests/nolibc: fix up failures when there is no procfs
> selftests/nolibc: rename proc variable to has_proc
> selftests/nolibc: rename euid0 variable to is_root
> selftests/nolibc: prepare tmpfs and hugetlbfs
> selftests/nolibc: rename chmod_net to chmod_good
> selftests/nolibc: link_cross: support tmpfs
> selftests/nolibc: rename chroot_exe to chroot_file
> selftests/nolibc: vfprintf: silence memfd_create() warning
> selftests/nolibc: vfprintf: skip if neither tmpfs nor hugetlbfs
> selftests/nolibc: vfprintf: support tmpfs and hugetlbfs
>
> tools/include/nolibc/sys.h | 28 ++++
> tools/testing/selftests/nolibc/nolibc-test.c | 132 +++++++++++++++----
> 2 files changed, 138 insertions(+), 22 deletions(-)
>
> --
> 2.25.1
>

Next message: T.J. Mercier: "Re: [PATCH v4 2/6] dma-buf/heaps: Don't assert held reservation lock for dma-buf mmapping"
Previous message: Nathan Chancellor: "Re: [PATCH v7 10/19] s390: mm: Convert to GENERIC_IOREMAP"
In reply to: Zhangjin Wu: "[PATCH v1 17/17] selftests/nolibc: vfprintf: support tmpfs and hugetlbfs"
Next in thread: Zhangjin Wu: "Re: [PATCH v1 00/17] selftests/nolibc: allow run with minimal kernel config"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]