[PATCH v2] erofs-utils: tests: add a splice() test for inode_share mode

Zhan Xusheng <[email protected]>
Newsgroups org.ozlabs.lists.linux-erofs
Message-ID <[email protected]>
From: Zhan Xusheng <[email protected]>

From: Zhan Xusheng <[email protected]>

splice() and sendfile() went through filemap_splice_read() on the user
file, so they populated the per-inode page cache instead of the shared one
that read() and mmap() use.  The content was correct either way, which is
why nothing caught it: only which mapping the pages land in changes.

So read one file with splice() alone and then look for the pages through
the other one, where they only show up if the read went through the shared
mapping.  pgresident counts them with mmap() + mincore(), which reaches
the backing file because erofs_ishare_mmap() points the vma there.

Measured on 7.2 with two identical 8 MiB files, before and after the
kernel change, as pages seen through the file that was not spliced:

  before   0
  after    2048

Two small helpers because no common tool reads a file purely with
splice(2) -- dd and cat both use read(2) -- and because counting resident
pages otherwise means either cachestat(), which reports the file's own
mapping rather than the shared one, or coaxing fincore off it.

Signed-off-by: Zhan Xusheng <[email protected]>
---
v1->v2:
- Dropped cachestat() as asked.  v1 both asserted what it reports and
  used strace -efault=cachestat to push fincore onto mmap()+mincore();
  neither is left.  If erofs_ishare_fops ever grows a ->cachestat()
  that fault injection would have been the wrong way round anyway.
- Observe through the file that was not spliced, so the count is only
  non-zero if the read went through the shared mapping.
- Count the pages in a helper, which also drops the strace, fincore
  and fadvise dependencies.  The empty starting state is now asserted
  rather than forced with fadvise.

v1: https://lore.kernel.org/all/[email protected]

 tests/Makefile.am      |  3 ++
 tests/common/rc        | 12 +++++++
 tests/erofs/032        | 64 +++++++++++++++++++++++++++++++++++
 tests/erofs/032.out    |  2 ++
 tests/src/Makefile.am  |  7 +++-
 tests/src/pgresident.c | 76 ++++++++++++++++++++++++++++++++++++++++++
 tests/src/spliceread.c | 61 +++++++++++++++++++++++++++++++++
 7 files changed, 224 insertions(+), 1 deletion(-)
 create mode 100755 tests/erofs/032
 create mode 100644 tests/erofs/032.out
 create mode 100644 tests/src/pgresident.c
 create mode 100644 tests/src/spliceread.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c0291ac..d7674fe 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -132,6 +132,9 @@ TESTS += erofs/030
 # 031 - test chunk-based mapping with shared chunks across inodes
 TESTS += erofs/031
 
+# 032 - test that splice() uses the shared page cache in inode_share mode
+TESTS += erofs/032
+
 # NEW TEST CASE HERE
 # TESTS += erofs/999
 
diff --git a/tests/common/rc b/tests/common/rc
index aa7ef99..e4d9f5e 100644
--- a/tests/common/rc
+++ b/tests/common/rc
@@ -357,6 +357,18 @@ _require_fssum()
 	[ -x $FSSUM_PROG ] || _notrun "fssum not built"
 }
 
+_require_spliceread()
+{
+	SPLICEREAD_PROG=${PWD}/src/spliceread
+	[ -x $SPLICEREAD_PROG ] || _notrun "spliceread not built"
+}
+
+_require_pgresident()
+{
+	PGRESIDENT_PROG=${PWD}/src/pgresident
+	[ -x $PGRESIDENT_PROG ] || _notrun "pgresident not built"
+}
+
 # generate random string with maximum $1 length
 _random()
 {
diff --git a/tests/erofs/032 b/tests/erofs/032
new file mode 100755
index 0000000..958e5e2
--- /dev/null
+++ b/tests/erofs/032
@@ -0,0 +1,64 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Test that splice() reads through the shared page cache in inode_share mode
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$(echo $0 | awk '{print $((NF-1))"/"$NF}' FS="/")
+
+# get standard environment, filters and checks
+. "${srcdir}/common/rc"
+
+cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+_require_erofs
+_require_erofs_inode_sharing
+_require_spliceread
+_require_pgresident
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+echo "QA output created by $seq"
+
+if [ -z $SCRATCH_DEV ]; then
+	SCRATCH_DEV=$tmp/erofs_$seq.img
+	rm -f $SCRATCH_DEV
+fi
+
+localdir="$tmp/$seq"
+rm -rf $localdir
+mkdir -p $localdir
+
+# two files with identical content, so that they end up sharing one inode
+dd if=/dev/urandom of=$localdir/a bs=1M count=8 2>/dev/null
+cp $localdir/a $localdir/b
+
+_scratch_mkfs --xattr-inode-digest=system.erofs.fingerprint $localdir \
+	>> $seqres.full 2>&1 || _fail "failed to mkfs"
+_scratch_mount -oinode_share,domain_id=share_$seq 2>> $seqres.full
+
+# pgresident maps the file and counts the resident pages.  erofs_ishare_mmap()
+# points the vma at the backing file, so this counts the shared mapping.
+before=$($PGRESIDENT_PROG $SCRATCH_MNT/a) || _fail "pgresident failed"
+echo "before $before" >> $seqres.full
+[ "$before" -eq 0 ] || _fail "shared page cache not empty after mount"
+
+# Read one file with splice() alone, then look for the pages through the other
+# one.  They only show up there if the read went through the shared mapping.
+$SPLICEREAD_PROG $SCRATCH_MNT/b >> $seqres.full 2>&1 || _fail "splice failed"
+
+after=$($PGRESIDENT_PROG $SCRATCH_MNT/a) || _fail "pgresident failed"
+echo "after $after" >> $seqres.full
+[ "$after" -gt 0 ] || _fail "splice did not populate the shared page cache"
+
+_scratch_unmount
+
+echo Silence is golden
+status=0
+exit 0
diff --git a/tests/erofs/032.out b/tests/erofs/032.out
new file mode 100644
index 0000000..34e059f
--- /dev/null
+++ b/tests/erofs/032.out
@@ -0,0 +1,2 @@
+QA output created by 032
+Silence is golden
diff --git a/tests/src/Makefile.am b/tests/src/Makefile.am
index 16de41a..99358de 100644
--- a/tests/src/Makefile.am
+++ b/tests/src/Makefile.am
@@ -3,12 +3,17 @@
 
 AUTOMAKE_OPTIONS	= foreign
 check_PROGRAMS		=
-noinst_PROGRAMS		= fssum
+noinst_PROGRAMS		= fssum spliceread pgresident
 
 fssum_CFLAGS = -Wall -I$(top_srcdir)/include
 fssum_LDADD = $(top_builddir)/lib/liberofs.la
 fssum_SOURCES = fssum.c
 
+spliceread_CFLAGS = -Wall
+spliceread_SOURCES = spliceread.c
+pgresident_CFLAGS = -Wall
+pgresident_SOURCES = pgresident.c
+
 if ENABLE_LZ4
 check_PROGRAMS += badlz4
 badlz4_SOURCES = badlz4.c
diff --git a/tests/src/pgresident.c b/tests/src/pgresident.c
new file mode 100644
index 0000000..2164766
--- /dev/null
+++ b/tests/src/pgresident.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Report how many pages of a file are resident in the page cache it is
+ * mapped from, using mmap() + mincore().
+ *
+ * For erofs inode_share this is the shared mapping, since
+ * erofs_ishare_mmap() points the vma at the backing file.
+ */
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+int main(int argc, char *argv[])
+{
+	unsigned char *vec;
+	size_t pages, i;
+	struct stat st;
+	long resident = 0;
+	long pagesize;
+	void *addr;
+	int fd;
+
+	if (argc != 2) {
+		fprintf(stderr, "usage: %s FILE\n", argv[0]);
+		return 2;
+	}
+
+	fd = open(argv[1], O_RDONLY);
+	if (fd < 0) {
+		perror("open");
+		return 1;
+	}
+	if (fstat(fd, &st)) {
+		perror("fstat");
+		return 1;
+	}
+	if (!st.st_size) {
+		printf("0\n");
+		return 0;
+	}
+
+	pagesize = sysconf(_SC_PAGESIZE);
+	if (pagesize <= 0) {
+		perror("sysconf");
+		return 1;
+	}
+	pages = ((size_t)st.st_size + pagesize - 1) / pagesize;
+
+	addr = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
+	if (addr == MAP_FAILED) {
+		perror("mmap");
+		return 1;
+	}
+	vec = malloc(pages);
+	if (!vec) {
+		perror("malloc");
+		return 1;
+	}
+	if (mincore(addr, st.st_size, vec)) {
+		perror("mincore");
+		return 1;
+	}
+	for (i = 0; i < pages; i++)
+		if (vec[i] & 1)
+			resident++;
+
+	printf("%ld\n", resident);
+
+	free(vec);
+	munmap(addr, st.st_size);
+	close(fd);
+	return 0;
+}
diff --git a/tests/src/spliceread.c b/tests/src/spliceread.c
new file mode 100644
index 0000000..cb9f09d
--- /dev/null
+++ b/tests/src/spliceread.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * spliceread.c
+ *
+ * Read a file into a pipe with splice(2) and discard it, so that a test can
+ * exercise ->splice_read without any read(2) touching the file.
+ */
+#define _GNU_SOURCE
+#include <fcntl.h>
+#include <stdio.h>
+#include <unistd.h>
+
+#define CHUNK	65536
+
+int main(int argc, char *argv[])
+{
+	static char buf[CHUNK];
+	int fd, p[2];
+
+	if (argc != 2) {
+		fprintf(stderr, "usage: %s FILE\n", argv[0]);
+		return 2;
+	}
+
+	fd = open(argv[1], O_RDONLY);
+	if (fd < 0) {
+		perror("open");
+		return 1;
+	}
+	if (pipe(p)) {
+		perror("pipe");
+		return 1;
+	}
+
+	for (;;) {
+		ssize_t n = splice(fd, NULL, p[1], NULL, CHUNK, 0);
+
+		if (n < 0) {
+			perror("splice");
+			return 1;
+		}
+		if (!n)
+			break;
+
+		/* drain the pipe so the next splice() has room */
+		while (n > 0) {
+			ssize_t m = read(p[0], buf, n > CHUNK ? CHUNK : n);
+
+			if (m <= 0) {
+				perror("read");
+				return 1;
+			}
+			n -= m;
+		}
+	}
+
+	close(p[0]);
+	close(p[1]);
+	close(fd);
+	return 0;
+}
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.