linux-kernel - Re: Upcoming: Notifications, FS notifications and fsinfo()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 31 Mar 2020 22:23:50 +0100
From:   David Howells <dhowells@...hat.com>
To:     Miklos Szeredi <miklos@...redi.hu>
Cc:     dhowells@...hat.com,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Al Viro <viro@...iv.linux.org.uk>, dray@...hat.com,
        Karel Zak <kzak@...hat.com>,
        Miklos Szeredi <mszeredi@...hat.com>,
        Steven Whitehouse <swhiteho@...hat.com>,
        Jeff Layton <jlayton@...hat.com>, Ian Kent <raven@...maw.net>,
        andres@...razel.de,
        Christian Brauner <christian.brauner@...ntu.com>,
        Lennart Poettering <mzxreary@...inter.de>,
        keyrings@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: Upcoming: Notifications, FS notifications and fsinfo()

David Howells <dhowells@...hat.com> wrote:

> > So even the p2 method will give at least 80k queries/s, which is quite
> > good, considering that the need to rescan the complete mount tree
> > should be exceedingly rare (and in case it mattered, could be
> > optimized by priming from /proc/self/mountinfo).
> 
> One thing to note is that the test is actually a little biased in favour of
> the "p" test, where the mnt_id is looked up by path from /proc/fdinfo.  That's
> not all that useful, except as an index into mountfs.  I'm not sure how much
> use it as a check on whether the mount is the same mount or not since mount
> IDs can get reused.

However, to deal with an overrun, you're going to have to read multiple
attributes.  So I've added an attribute file to expose the topology change
counter and it now reads that as well.

For 10000 mounts, f=22899us f2=18240us p=101054us p2=117273us <-- prev email
For 10000 mounts, f=24853us f2=20453us p=235581us p2= 59798us <-- parent_id
For 10000 mounts, f=24621us f2=20528us p=320164us p2=111416us <-- counter

Probably unsurprisingly, this doesn't affect fsinfo() significantly since I've
tried to expose the change counters in relevant places.  It does, however,
significantly affect mountfs because you seem to want every value to be
exposed through its own file.

Now this can be worked around by having files that bundle up several values
that are of interest to a particular operation (e.g. rescanning after a
notification queue overrun).

See the attached additional patch.  Note that the

	sum_check_2 += r.mnt_topology_changes;

bits in the fsinfo() tests accidentally got left in the preceding patch and so
aren't in this one.

David
---
commit 6c62787aec41f67c1d5a55a0d59578854bcef6f8
Author: David Howells <dhowells@...hat.com>
Date:   Tue Mar 31 21:53:11 2020 +0100

    Add a mountfs file to export the topology counter

diff --git a/fs/mountfs/super.c b/fs/mountfs/super.c
index 82c01eb6154d..58c05feb4fdd 100644
--- a/fs/mountfs/super.c
+++ b/fs/mountfs/super.c
@@ -22,7 +22,7 @@ struct mountfs_entry {
 
 static const char *mountfs_attrs[] = {
 	"root", "mountpoint", "id", "parent", "options", "children",
-	"group", "master", "propagate_from"
+	"group", "master", "propagate_from", "counter"
 };
 
 #define MOUNTFS_INO(id) (((unsigned long) id + 1) * \
@@ -128,6 +128,8 @@ static int mountfs_attr_show(struct seq_file *sf, void *v)
 			if (tmp)
 				seq_printf(sf, "%i\n", tmp);
 		}
+	} else if (strcmp(name, "counter") == 0) {
+		seq_printf(sf, "%u\n", atomic_read(&mnt->mnt_topology_changes));
 	} else {
 		WARN_ON(1);
 		err = -EIO;
diff --git a/samples/vfs/test-fsinfo-perf.c b/samples/vfs/test-fsinfo-perf.c
index 2bcde06ee78b..2b7606a53c2d 100644
--- a/samples/vfs/test-fsinfo-perf.c
+++ b/samples/vfs/test-fsinfo-perf.c
@@ -149,6 +149,26 @@ static void get_id_by_proc(int ix, const char *path)
 	}
 
 	sum_check += x;
+
+	/* And now the topology change counter */
+	sprintf(procfile, "/mnt/%u/counter", mnt_id);
+	fd = open(procfile, O_RDONLY);
+	ERR(fd, procfile);
+	len = read(fd, buffer, sizeof(buffer) - 1);
+	ERR(len, "read/counter");
+	close(fd);
+	if (len > 0 && buffer[len - 1] == '\n')
+		len--;
+	buffer[len] = 0;
+
+	x = strtoul(buffer, &q, 10);
+
+	if (*q) {
+		fprintf(stderr, "Bad format in %s '%s'\n", procfile, buffer);
+		exit(3);
+	}
+
+	sum_check_2 += x;
 	//printf("[%u] %u\n", ix, x);
 }
 
@@ -204,7 +224,7 @@ static void get_id_by_mountfs(void)
 	unsigned int base_mnt_id, mnt_id, x;
 	ssize_t len, s_children;
 	char procfile[100], buffer[100], *children, *p, *q, *nl, *comma;
-	int fd, fd2, mntfd, i;
+	int fd, fd2, mntfd;
 
 	/* Start off by reading the mount ID from the base path */
 	fd = open(base_path, O_PATH);
@@ -269,7 +289,6 @@ static void get_id_by_mountfs(void)
 	p = children;
 	if (!*p)
 		return;
-	i = 0;
 	do {
 		mnt_id = strtoul(p, &comma, 10);
 		if (*comma) {
@@ -297,8 +316,26 @@ static void get_id_by_mountfs(void)
 			exit(3);
 		}
 
-		if (0) printf("[%u] %u\n", i++, x);
 		sum_check += x;
+
+		sprintf(procfile, "%u/counter", mnt_id);
+		fd = openat(mntfd, procfile, O_RDONLY);
+		ERR(fd, procfile);
+		len = read(fd, buffer, sizeof(buffer) - 1);
+		ERR(len, "read/counter");
+		close(fd);
+		if (len > 0 && buffer[len - 1] == '\n')
+			len--;
+		buffer[len] = 0;
+
+		x = strtoul(buffer, &q, 10);
+
+		if (*q) {
+			fprintf(stderr, "Bad format in %s '%s'\n", procfile, buffer);
+			exit(3);
+		}
+
+		sum_check_2 += x;
 	} while (p = comma, *comma);
 }