2023-08-16 10:48:08

by Andre Przywara

[permalink] [raw]
Subject: [PATCH 0/3] selftests: cachestat: fix build and run on older kernels

I ran all kernel selftests on some test machine, and stumbled upon
cachestat failing (among others).
Those patches fix the cachestat test compilation and run on older
kernels.

Also I found that the but-last test (on a normal file) fails when run on
a tmpfs mounted directory, as it happens on an initramfs-only system, or
when the current directory happens to be /dev/shm or /tmp:
# Create/open tmpfilecachestat
# Cachestat call returned 0
# Using cachestat: Cached: 4, Dirty: 4, Writeback: 0, Evicted: 0, Recently Evicted: 0
# Cachestat call (after fsync) returned 0
# Using cachestat: Cached: 4, Dirty: 4, Writeback: 0, Evicted: 0, Recently Evicted: 0
# Number of dirty should be zero after fsync.
not ok 6 cachestat fails with normal file

That same test binary succeeds on the same machine right afterwards if
the current directory is changed to an ext4 filesystem.

I don't really know if this is expected, and whether we should try to
figure out if the test file lives on a tmpfs filesystem, or whether the
test itself is not strict enough, and requires more "flushing"
(drop_caches?) to cover tmpfs directories as well.

Any ideas how to fix this would be appreciated.

Cheers,
Andre

Andre Przywara (3):
selftests: cachestat: properly link in librt
selftests: cachestat: use proper syscall number macro
selftests: cachestat: test for cachestat availability

tools/testing/selftests/cachestat/Makefile | 2 +-
.../selftests/cachestat/test_cachestat.c | 29 +++++++++++++++----
2 files changed, 25 insertions(+), 6 deletions(-)

--
2.25.1



2023-08-16 16:01:29

by Andre Przywara

[permalink] [raw]
Subject: [PATCH 1/3] selftests: cachestat: properly link in librt

Libraries should be listed last on the compiler's command line, so that
the linker can look for and find still unresolved symbols. The librt
library, required for the shm_* functions, was announced using CFLAGS,
which puts the library *before* the source files, and fails compilation
on my system:
======================
gcc -isystem /src/linux-selftests/usr/include -Wall -lrt test_cachestat.c
-o /src/linux-selftests/kselftest/cachestat/test_cachestat
/usr/bin/ld: /tmp/cceQWO3u.o: in function `test_cachestat_shmem':
test_cachestat.c:(.text+0x890): undefined reference to `shm_open'
/usr/bin/ld: test_cachestat.c:(.text+0x99c): undefined reference to `shm_unlink'
collect2: error: ld returned 1 exit status
make[4]: *** [../lib.mk:181: /src/linux-selftests/kselftest/cachestat/test_cachestat] Error 1
======================

Announce the library using the LDLIBS variable, which ensures the proper
ordering on the command line.

Signed-off-by: Andre Przywara <[email protected]>
---
tools/testing/selftests/cachestat/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/cachestat/Makefile b/tools/testing/selftests/cachestat/Makefile
index fca73aaa7d141..778b54ebb0364 100644
--- a/tools/testing/selftests/cachestat/Makefile
+++ b/tools/testing/selftests/cachestat/Makefile
@@ -3,6 +3,6 @@ TEST_GEN_PROGS := test_cachestat

CFLAGS += $(KHDR_INCLUDES)
CFLAGS += -Wall
-CFLAGS += -lrt
+LDLIBS += -lrt

include ../lib.mk
--
2.25.1


2023-08-17 19:01:58

by Andre Przywara

[permalink] [raw]
Subject: [PATCH 3/3] selftests: cachestat: test for cachestat availability

As cachestat is a new syscall, it won't be available on older kernels,
for instance those running on a build machine. In this case, a run
reports all tests as "not ok" at the moment.

Test for the cachestat syscall availability first, before doing further
tests, and bail out early with a TAP SKIP comment.

This also uses the opportunity to add the proper TAP headers, and add
one check for the syscall error handling (illegal file descriptor).

Signed-off-by: Andre Przywara <[email protected]>
---
.../selftests/cachestat/test_cachestat.c | 22 ++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
index a5a4ac8dcb76c..77620e7ecf562 100644
--- a/tools/testing/selftests/cachestat/test_cachestat.c
+++ b/tools/testing/selftests/cachestat/test_cachestat.c
@@ -15,6 +15,8 @@

#include "../kselftest.h"

+#define NR_TESTS 8
+
static const char * const dev_files[] = {
"/dev/zero", "/dev/null", "/dev/urandom",
"/proc/version", "/proc"
@@ -235,7 +237,25 @@ bool test_cachestat_shmem(void)

int main(void)
{
- int ret = 0;
+ int ret;
+
+ ksft_print_header();
+
+ ret = syscall(__NR_cachestat, -1, NULL, NULL, 0);
+ if (ret == -1 && errno == ENOSYS) {
+ printf("1..0 # Skipped: cachestat syscall not available\n");
+ return KSFT_SKIP;
+ }
+
+ ksft_set_plan(NR_TESTS);
+
+ if (ret == -1 && errno == EBADF) {
+ ksft_test_result_pass("bad file descriptor recognized\n");
+ ret = 0;
+ } else {
+ ksft_test_result_fail("bad file descriptor ignored\n");
+ ret = 1;
+ }

for (int i = 0; i < 5; i++) {
const char *dev_filename = dev_files[i];
--
2.25.1


2023-08-17 20:51:16

by Andre Przywara

[permalink] [raw]
Subject: Re: [PATCH 3/3] selftests: cachestat: test for cachestat availability

On Wed, 16 Aug 2023 11:11:49 -0600
Shuah Khan <[email protected]> wrote:

Hi,

> On 8/15/23 09:56, Andre Przywara wrote:
> > As cachestat is a new syscall, it won't be available on older kernels,
> > for instance those running on a build machine. In this case, a run
> > reports all tests as "not ok" at the moment.
> >
> > Test for the cachestat syscall availability first, before doing further
> > tests, and bail out early with a TAP SKIP comment.
> >
> > This also uses the opportunity to add the proper TAP headers, and add
> > one check for the syscall error handling (illegal file descriptor).
> >
> > Signed-off-by: Andre Przywara <[email protected]>
> > ---
> > .../selftests/cachestat/test_cachestat.c | 22 ++++++++++++++++++-
> > 1 file changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
> > index a5a4ac8dcb76c..77620e7ecf562 100644
> > --- a/tools/testing/selftests/cachestat/test_cachestat.c
> > +++ b/tools/testing/selftests/cachestat/test_cachestat.c
> > @@ -15,6 +15,8 @@
> >
> > #include "../kselftest.h"
> >
> > +#define NR_TESTS 8
> > +
> > static const char * const dev_files[] = {
> > "/dev/zero", "/dev/null", "/dev/urandom",
> > "/proc/version", "/proc"
> > @@ -235,7 +237,25 @@ bool test_cachestat_shmem(void)
> >
> > int main(void)
> > {
> > - int ret = 0;
> > + int ret;
> > +
> > + ksft_print_header();
> > +
> > + ret = syscall(__NR_cachestat, -1, NULL, NULL, 0);
> > + if (ret == -1 && errno == ENOSYS) {
> > + printf("1..0 # Skipped: cachestat syscall not available\n");
> > + return KSFT_SKIP;
> What happens when other errors besides ENOSYS? The test shouldn't
> continue.

-1 is an illegal file descriptor, and this is checked below (still using
the same ret and errno), but reported using the normal framework.
This check above is done early, before we even announce the plan, so that
we can skip *all* of the tests, since they don't make any sense when the
syscall is not available at all.

Does that make sense?

Cheers,
Andre

>
> > + }
> > +
> > + ksft_set_plan(NR_TESTS);
> > +
> > + if (ret == -1 && errno == EBADF) {
> > + ksft_test_result_pass("bad file descriptor recognized\n");
> > + ret = 0;
> > + } else {
> > + ksft_test_result_fail("bad file descriptor ignored\n");
> > + ret = 1;
> > + }
> >
> > for (int i = 0; i < 5; i++) {
> > const char *dev_filename = dev_files[i];
>
> thanks,
> -- Shuah