Re: [PATCH v7 07/10] fsmonitor: implement filesystem change listener for Linux

[Patrick Steinhardt <ps@pks.im>]

On Thu, Feb 26, 2026 at 12:27:20AM +0000, Paul Tarjan via GitGitGadget wrote:
> From: Paul Tarjan <github@paulisageek.com>
> 
> Implement the built-in fsmonitor daemon for Linux using the inotify
> API, bringing it to feature parity with the existing Windows and macOS
> implementations.
> 
> The implementation uses inotify rather than fanotify because fanotify
> requires either CAP_SYS_ADMIN or CAP_PERFMON capabilities, making it
> unsuitable for an unprivileged user-space daemon.  While inotify has
> the limitation of requiring a separate watch on every directory (unlike
> macOS's FSEvents, which can monitor an entire directory tree with a
> single watch), it operates without elevated privileges and provides
> the per-file event granularity needed for fsmonitor.

Thanks for adding this explanation, makes sense.

> diff --git a/compat/fsmonitor/fsm-path-utils-linux.c b/compat/fsmonitor/fsm-path-utils-linux.c
> new file mode 100644
> index 0000000000..b4c19e0655
> --- /dev/null
> +++ b/compat/fsmonitor/fsm-path-utils-linux.c
> @@ -0,0 +1,220 @@
> +#include "git-compat-util.h"
> +#include "fsmonitor-ll.h"
> +#include "fsmonitor-path-utils.h"
> +#include "gettext.h"
> +#include "trace.h"
> +
> +#include <sys/statfs.h>
> +
> +#ifdef HAVE_LINUX_MAGIC_H

I saw that this define is only wired up for CMake. I guess we should
also add it to our Makefile (probably via config.mak.uname) and Meson
(probably via compiler.has_header()).

[snip]
> +/*
> + * Get the filesystem type name for logging purposes.
> + */
> +static const char *get_fs_typename(unsigned long f_type)
> +{
> +	switch (f_type) {
> +	case CIFS_SUPER_MAGIC:
> +		return "cifs";
> +	case SMB_SUPER_MAGIC:
> +		return "smb";
> +	case SMB2_SUPER_MAGIC:
> +		return "smb2";
> +	case NFS_SUPER_MAGIC:
> +		return "nfs";
> +	case AFS_SUPER_MAGIC:
> +		return "afs";
> +	case CODA_SUPER_MAGIC:
> +		return "coda";
> +	case V9FS_MAGIC:
> +		return "9p";
> +	case FUSE_SUPER_MAGIC:
> +		return "fuse";
> +	default:
> +		return "unknown";
> +	}
> +}

This selection looks rather interesting to me. Why wouldn't we include
common filesystems like ext4 and the like? Certainly hints that the
function needs better documentation, and potentially a better name.

> +/*
> + * Find the mount point for a given path by reading /proc/mounts.
> + * Returns the filesystem type for the longest matching mount point.
> + */
> +static char *find_mount(const char *path, struct statfs *fs)
> +{
> +	FILE *fp;
> +	struct strbuf line = STRBUF_INIT;
> +	struct strbuf match = STRBUF_INIT;
> +	struct strbuf fstype = STRBUF_INIT;
> +	char *result = NULL;
> +	struct statfs path_fs;
> +
> +	if (statfs(path, &path_fs) < 0)
> +		return NULL;
> +
> +	fp = fopen("/proc/mounts", "r");
> +	if (!fp)
> +		return NULL;
> +
> +	while (strbuf_getline(&line, fp) != EOF) {
> +		char *fields[6];
> +		char *p = line.buf;
> +		int i;
> +
> +		/* Parse mount entry: device mountpoint fstype options dump pass */
> +		for (i = 0; i < 6 && p; i++) {
> +			fields[i] = p;
> +			p = strchr(p, ' ');
> +			if (p)
> +				*p++ = '\0';
> +		}
> +
> +		if (i >= 3) {
> +			const char *mountpoint = fields[1];
> +			const char *type = fields[2];
> +			struct statfs mount_fs;
> +
> +			/* Check if this mount point is a prefix of our path */
> +			if (starts_with(path, mountpoint) &&
> +			    (path[strlen(mountpoint)] == '/' ||
> +			     path[strlen(mountpoint)] == '\0')) {
> +				/* Check if filesystem ID matches */
> +				if (statfs(mountpoint, &mount_fs) == 0 &&
> +				    !memcmp(&mount_fs.f_fsid, &path_fs.f_fsid,
> +					    sizeof(mount_fs.f_fsid))) {
> +					/* Keep the longest matching mount point */
> +					if (strlen(mountpoint) > match.len) {
> +						strbuf_reset(&match);
> +						strbuf_addstr(&match, mountpoint);
> +						strbuf_reset(&fstype);
> +						strbuf_addstr(&fstype, type);
> +						*fs = mount_fs;
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +	fclose(fp);
> +	strbuf_release(&line);
> +	strbuf_release(&match);
> +
> +	if (fstype.len)
> +		result = strbuf_detach(&fstype, NULL);
> +	else
> +		strbuf_release(&fstype);
> +
> +	return result;
> +}

Sorry, but I still don't quite understand what we're doing here. Isn't
the longest matching mount point always the one that statfs(3p) gave us?
Why do we have to scan "/proc/mounts"?

Patrick