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

[Miklos Szeredi <miklos@szeredi.hu>]

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

On Tue, Apr 7, 2020 at 4:22 AM Ian Kent <raven@themaw.net> wrote:
> > Right now, when you have n mounts, and any mount changes, or one is
> > added or removed then we have to parse the whole mount table again,
> > asynchronously, processing all n entries again, every frickin
> > time. This means the work to process n mounts popping up at boot is
> > O(n²). That sucks, it should be obvious to anyone. Now if we get that
> > fixed, by some mount API that can send us minimal notifications about
> > what happened and where, then this becomes O(n), which is totally OK.

Something's not right with the above statement.  Hint: if there are
lots of events in quick succession, you can batch them quite easily to
prevent overloading the system.

Wrote a pair of utilities to check out the capabilities of the current
API.   The first one just creates N mounts, optionally sleeping
between each.  The second one watches /proc/self/mountinfo and
generates individual (add/del/change) events based on POLLPRI and
comparing contents with previous instance.

First use case: create 10,000 mounts, then start the watcher and
create 1000 mounts with a 50ms sleep between them.  Total time (user +
system) consumed by the watcher: 25s.  This is indeed pretty dismal,
and a per-mount query will help tremendously.  But it's still "just"
25ms per mount, so if the mounts are far apart (which is what this
test is about), this won't thrash the system.  Note, how this is self
regulating: if the load is high, it will automatically batch more
requests, preventing overload.  It is also prone to lose pairs of add
+ remove in these case (and so is the ring buffer based one from
David).

Second use case: start the watcher and create 50,000 mounts with no
sleep between them.   Total time consumed by the watcher: 0.154s or
3.08us/event.    Note, the same test case adds about 5ms for the
50,000 umount events, which is 0.1us/event.

Real life will probably be between these extremes, but it's clear that
there's room for improvement in userspace as well as kernel
interfaces.  The current kernel interface is very efficient in
retrieving a lot of state in one go.  It is not efficient in handling
small differences.

> > Anyway, I have the suspicion this discussion has stopped being
> > useful. I think you are trying to fix problems that userspce actually
> > doesn't have. I can just tell you what we understand the problems
> > are,
> > but if you are out trying to fix other percieved ones, then great,
> > but
> > I mostly lost interest.

I was, and still am, trying to see the big picture.

Whatever.   I think it's your turn to show some numbers about how the
new API improves performance of systemd with a large number of mounts.

Thanks,
Miklos

[-- Attachment #2: many-mounts.c --]
[-- Type: text/x-csrc, Size: 1155 bytes --]

#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <err.h>
#include <sys/stat.h>
#include <sys/mount.h>

int main(int argc, char *argv[])
{
	char *base_path = argv[1];
	char name[4096];
	int nr_mounts, i, sleep_ms = 0;

	if (argc < 3 || argc > 4)
		errx(1, "usage: %s base_path nr_mounts [sleep_ms]", argv[0]);

	nr_mounts = atoi(argv[2]);
	if (argc > 3)
		sleep_ms = atoi(argv[3]);

	fprintf(stderr, "Mounting...\n");
	if (mount("none", base_path, "tmpfs", 0, NULL) == -1)
		err(1, "mount/tmpfs");
	if (mount("none", base_path, NULL, MS_PRIVATE, NULL) == -1)
		err(1, "mount/MS_PRIVATE");
	for (i = 0; i < nr_mounts; i++) {
		sprintf(name, "%s/%d", base_path, i);
		if (mkdir(name, 0755) == -1)
			err(1, "mkdir");
		if (mount("none", name, "tmpfs", 0, NULL) == -1)
			err(1, "mount/tmpfs");
		if (mount("none", name, NULL, MS_PRIVATE, NULL) == -1)
			err(1, "mount/MS_PRIVATE");
		if (sleep_ms)
			usleep(sleep_ms * 1000);
	}
	fprintf(stderr, "Press ENTER\n");
	getchar();

	fprintf(stderr, "Unmounting...\n");
	if (umount2(base_path, MNT_DETACH) == -1)
		err(1, "umount");

	fprintf(stderr, "Done\n");

	return 0;
}

[-- Attachment #3: watch_mounts.c --]
[-- Type: text/x-csrc, Size: 3380 bytes --]

#include <stdio.h>
#include <fcntl.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>
#include <poll.h>
#include <err.h>

struct index {
	struct index *next;
	struct index *prev;
	const char *line;
};

struct state {
	size_t bufsize;
	char *buf;
	size_t index_size;
	struct index *index;
	struct index head;
};

static void read_mountinfo(struct pollfd *pfd, char *buf, size_t bufsize)
{
	int readcnt, backoff = 0, retry = 0;
	size_t len;
	ssize_t res;

retry:
	if (lseek(pfd->fd, 0, SEEK_SET) == (off_t) -1)
		err(1, "lseek");
	len = 0;
	readcnt = 0;
	do {
		if (len >= bufsize - 4096)
			errx(1, "buffer overrun");
		res = read(pfd->fd, buf + len, bufsize - len);
		if (res == -1)
			err(1, "read");
		len += res;

		if (!res || !(++readcnt % 16)) {
			if (poll(pfd, 1, 0) == -1)
				err(1, "poll/0");
			if (pfd->revents & POLLPRI) {
				if (!backoff) {
					backoff++;
					goto retry;
				}
				if (!retry) {
					fprintf(stderr, "retry.");
					retry = 1;
				}
				do {
					usleep(backoff * 1000);
					if (backoff < 128)
						backoff *= 2;
					if (poll(pfd, 1, 0) == -1)
						err(1, "poll/0");
				} while (pfd->revents & POLLPRI);
				goto retry;
			}
		}
	} while (res);
	buf[len] = '\0';

	if (retry) {
		fprintf(stderr, "..\n");
		retry = 0;
	}
}

static void add_index(struct state *s, struct index *this, const char *line)
{
	struct index *prev = s->head.prev, *next = &s->head;

	if (this->line)
		errx(1, "index corruption");

	this->line = line;
	this->next = next;
	this->prev = prev;
	prev->next = next->prev = this;
}

static void del_index(struct index *this)
{
	struct index *prev = this->prev, *next = this->next;

	this->line = NULL;
	prev->next = next;
	next->prev = prev;
}

static void diff_mountinfo(struct state *old, struct state *cur)
{
	char *line, *end;
	struct index *this;
	int mntid;

	cur->head.next = cur->head.prev = &cur->head;
	for (line = cur->buf; line[0]; line = end + 1) {
		end = strchr(line, '\n');
		if (!end)
			errx(1, "parsing (1)");
		*end = '\0';
		if (sscanf(line, "%i", &mntid) != 1)
			errx(1, "parsing (2)");
		if (mntid < 0 || (size_t) mntid >= cur->index_size)
			errx(1, "index overflow");
		add_index(cur, &cur->index[mntid], line);

		this = &old->index[mntid];
		if (this->line) {
			if (strcmp(this->line, line))
				printf("* %s\n", line);
			del_index(this);
		} else {
			printf("+ %s\n", line);
		}
	}
	while (old->head.next != &old->head) {
		this = old->head.next;
		printf("- %s\n", this->line);
		del_index(this);
	}
	fflush(stdout);
}

int main(void)
{
	struct state state[2], *old = &state[0], *cur = &state[1], *tmp;
	struct pollfd pfd = { .events = POLLPRI };

	old->index_size = cur->index_size = 131072;
	old->bufsize = cur->bufsize = cur->index_size * 128;
	old->index = calloc(old->index_size, sizeof(struct index));
	cur->index = calloc(cur->index_size, sizeof(struct index));
	old->buf = malloc(old->bufsize);
	cur->buf = malloc(cur->bufsize);
	if (!old->index || !cur->index || !old->buf || !cur->buf)
		err(1, "allocating buffers");

	old->buf[0] = '\0';
	old->head.prev = old->head.next = &old->head;

	pfd.fd = open("/proc/self/mountinfo", O_RDONLY);
	if (pfd.fd == -1)
		err(1, "open");

	while (1) {
		read_mountinfo(&pfd, cur->buf, cur->bufsize);
		diff_mountinfo(old, cur);

		tmp = old;
		old = cur;
		cur = tmp;

		if (poll(&pfd, 1, -1) == -1)
			err(1, "poll/inf");
	}
}