On Tue, Apr 7, 2020 at 4:22 AM Ian Kent <raven@xxxxxxxxxx> 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
#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; }
#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"); } }