Re: Upcoming: Notifications, FS notifications and fsinfo()

[Miklos Szeredi <miklos@szeredi.hu>]

[-- Attachment #1: Type: text/plain, Size: 1653 bytes --]

On Wed, Apr 1, 2020 at 3:58 PM David Howells <dhowells@redhat.com> wrote:
>
> David Howells <dhowells@redhat.com> wrote:
>
> > > Attached patch applies against readfile patch.
> >
> > But doesn't actually do what Karel asked for.  show_mountinfo() itself does
> > not give you what Karel asked for.

Not sure what you mean.  I think it shows precisely the information
Karel asked for.

>  Plus there's more information you need to
> > add to it.

The mountinfo format is extensible (see
Documentation/filesystems/proc.txt) so for example adding the change
counters would be simple.

> And arguably, it's worse than just reading /proc/mounts.  If you get a
> notification that something changed (ie. you poll /proc/mounts or mount
> notifications gives you an overrun) you now have to read *every*
> /mountfs/*/info file.  That is way more expensive.

fsinfo(2) will never be substantially cheaper than reading and parsing
/mnt/MNT_ID/info.  In fact reading a large part of the mount table
using fsinfo(2) will be substantially slower than parsing
/proc/self/mountinfo (this doesn't actually do the parsing but that
would add a very small amount of overhead):

root@kvm:~# ./test-fsinfo-perf /tmp/a 30000
--- make mounts ---
--- test fsinfo by path ---
sum(mnt_id) = 960000
--- test fsinfo by mnt_id ---
sum(mnt_id) = 960000
--- test /proc/fdinfo ---
sum(mnt_id) = 960000
--- test mountfs ---
sum(mnt_id) = 960000
--- test mountinfo ---
sum(mnt_id) = 960000
For   30000 mounts, f=    154963us f2=    148337us p=   1803699us p2=
  257019us; m=     53996us; p=11.6*f p=12.2*f2 p=7.0*p2 p=33.4*m
--- umount ---

Yes, that's 33 times faster!

Thanks,
Miklos

[-- Attachment #2: test-fsinfo-perf-mountinfo.patch --]
[-- Type: text/x-patch, Size: 3709 bytes --]

---
 samples/vfs/test-fsinfo-perf.c |   91 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 87 insertions(+), 4 deletions(-)

--- a/samples/vfs/test-fsinfo-perf.c
+++ b/samples/vfs/test-fsinfo-perf.c
@@ -339,6 +339,79 @@ static void get_id_by_mountfs(void)
 	} while (p = comma, *comma);
 }
 
+static void get_id_by_mountinfo(void)
+{
+	unsigned int base_mnt_id, mnt_id, x;
+	ssize_t len;
+	char procfile[100], buffer[4096], *p, *nl;
+	int fd, fd2, mntfd;
+
+	/* Start off by reading the mount ID from the base path */
+	fd = open(base_path, O_PATH);
+	ERR(fd, "open/path");
+	sprintf(procfile, "/proc/self/fdinfo/%u", fd);
+	fd2 = open(procfile, O_RDONLY);
+	ERR(fd2, "open/proc");
+	len = read(fd2, buffer, sizeof(buffer) - 1);
+	ERR(len, "read");
+	buffer[len] = 0;
+	close(fd2);
+	close(fd);
+
+	p = buffer;
+	do {
+		nl = strchr(p, '\n');
+		if (nl)
+			*nl++ = '\0';
+		else
+			nl = NULL;
+
+		if (strncmp(p, "mnt_id:", 7) != 0)
+			continue;
+		p += 7;
+		while (isblank(*p))
+			p++;
+		/* Have to allow for extra numbers being added to the line */
+		if (sscanf(p, "%u", &base_mnt_id) != 1) {
+			fprintf(stderr, "Bad format %s\n", procfile);
+			exit(3);
+		}
+		break;
+
+	} while ((p = nl));
+
+	if (!p) {
+		fprintf(stderr, "Missing field %s\n", procfile);
+		exit(3);
+	}
+
+	if (0) printf("[B] %u\n", base_mnt_id);
+
+	mntfd = open("/proc/self/mountinfo", O_RDONLY);
+	ERR(mntfd, "/proc/self/mountinfo");
+
+	while ((len = read(mntfd, buffer, sizeof(buffer)))) {
+		ERR(len, "read/mountinfo");
+
+		for (p = buffer; p < buffer + len; p = nl + 1) {
+			nl = strchr(p, '\n');
+			if (!nl) {
+				fprintf(stderr, "error parsing mountinfo\n");
+				exit(3);
+			}
+			*nl = '\0';
+			if (sscanf(p, "%i %i", &mnt_id, &x) != 2) {
+				fprintf(stderr, "error parsing mountinfo\n");
+				exit(3);
+			}
+			if (x == base_mnt_id)
+				sum_check += x;
+		}
+	}
+
+	close(mntfd);
+}
+
 static unsigned long duration(struct timeval *before, struct timeval *after)
 {
 	unsigned long a, b;
@@ -354,8 +427,9 @@ int main(int argc, char **argv)
 	struct timeval f2_before, f2_after;
 	struct timeval p_before, p_after;
 	struct timeval p2_before, p2_after;
+	struct timeval m_before, m_after;
 	const char *path;
-	unsigned long f_dur, f2_dur, p_dur, p2_dur;
+	unsigned long f_dur, f2_dur, p_dur, p2_dur, m_dur;
 
 	if (argc < 2) {
 		fprintf(stderr, "Format: %s <path> [nr_mounts]\n", argv[0]);
@@ -402,17 +476,26 @@ int main(int argc, char **argv)
 	ERR(gettimeofday(&p2_after, NULL), "gettimeofday");
 	printf("sum(mnt_id) = %lu\n", sum_check);
 
+	printf("--- test mountinfo ---\n");
+	sum_check = 0;
+	ERR(gettimeofday(&m_before, NULL), "gettimeofday");
+	get_id_by_mountinfo();
+	ERR(gettimeofday(&m_after, NULL), "gettimeofday");
+	printf("sum(mnt_id) = %lu\n", sum_check);
+
 	f_dur  = duration(&f_before,  &f_after);
 	f2_dur = duration(&f2_before, &f2_after);
 	p_dur  = duration(&p_before,  &p_after);
 	p2_dur = duration(&p2_before, &p2_after);
+	m_dur  = duration(&m_before,  &m_after);
 	//printf("fsinfo duration %10luus for %d mounts\n", f_dur, nr_mounts);
 	//printf("procfd duration %10luus for %d mounts\n", p_dur, nr_mounts);
 
-	printf("For %7d mounts, f=%10luus f2=%10luus p=%10luus p2=%10luus; p=%.1f*f p=%.1f*f2 p=%.1f*p2\n",
-	       nr_mounts, f_dur, f2_dur, p_dur, p2_dur,
+	printf("For %7d mounts, f=%10luus f2=%10luus p=%10luus p2=%10luus; m=%10luus; p=%.1f*f p=%.1f*f2 p=%.1f*p2 p=%.1f*m\n",
+	       nr_mounts, f_dur, f2_dur, p_dur, p2_dur, m_dur,
 	       (double)p_dur / (double)f_dur,
 	       (double)p_dur / (double)f2_dur,
-	       (double)p_dur / (double)p2_dur);
+	       (double)p_dur / (double)p2_dur,
+	       (double)p_dur / (double)m_dur);
 	return 0;
 }