Re: [RFC][PATCH] fanotify: Enable FAN_REPORT_FID on more filesystem types
From: Amir Goldstein <amir73il@gmail.com>
Date: 2023-10-25 14:12:07
Also in:
linux-fsdevel
quoted
Hi Jan, I seem to have dropped the ball on this after implementing AT_HANDLE_FID. It was step one in a larger plan.No problem, I forgot about this as well :)
Following up...
quoted
Christian, Jeff, Do you have an objection to this plan: 1. Convert all "legacy" FILEID_INO32_GEN fs with non-empty s_export_op and no explicit ->encode_fh() to use an explicit generic_encode_ino32_gen_fh() 2. Relax requirement of non-empty s_export_op for AT_HANDLE_FID to support encoding a (non-NFS) file id on all fs 3. For fs with empty s_export_op, allow fallback of AT_HANDLE_FID in exportfs_encode_inode_fh() to encode FILEID_INO64_GEN
These are now queued on Christian's vfs.f_fsid branch.
quoted
quoted
quoted
quoted
Also I have noticed your workaround with using st_dev for fsid. As I've checked, there are actually very few filesystems that don't set fsid these days. So maybe we could just get away with still refusing to report on filesystems without fsid and possibly fixup filesystems which don't set fsid yet and are used enough so that users complain?I started going down this path to close the gap with inotify. inotify is capable of watching all fs including pseudo fs, so I would like to have this feature parity.Well, but with pseudo filesystems (similarly as with FUSE) the notification was always unreliable. As in: some cases worked but others did not. I'm not sure that is something we should try to replicate :) So still I'd be interested to know which filesystems we are exactly interested to support and whether we are not better off to explicitly add fsid support to them like we did for tmpfs.Since this email, kernfs derivative fs gained fsid as well. Quoting your audit of remaining fs from another thread:quoted
...As far as I remember fanotify should be now able to handle anything that provides f_fsid in its statfs(2) call. And as I'm checking filesystems not setting fsid currently are: afs, coda, nfs - networking filesystems where inotify and fanotify have dubious value anywayBe that as it may, there may be users that use inotify on network fs and it even makes a lot of sense in controlled environments with single NFS client per NFS export (e.g. home dirs), so I think we will need to support those fs as well.Fair enough.
I have sent an fsid patch for NFS and for FUSE. I am not going to deal with afs and coda unless there is explicit demand.
quoted
Maybe the wise thing to do is to opt-in to monitor those fs after all? Maybe with explicit opt-in to watch a single fs, fanotify group will limit itself to marks on a specific sb and then a null fsid won't matter?We have virtual filesystems with all sorts of missing or weird notification functionality and we don't flag that in any way. So making a special flag for network filesystems seems a bit arbitrary. I'd just make them provide fsid and be done with it.quoted
quoted
configfs, debugfs, devpts, efivarfs, hugetlbfs, openpromfs, proc, pstore, ramfs, sysfs, tracefs - virtual filesystems where fsnotify functionality is quite limited. But some special cases could be useful. Adding fsid support is the same amount of trouble as for kernfs - a few LOC. In fact, we could perhaps add a fstype flag to indicate that this is a filesystem without persistent identification and so uuid should be autogenerated on mount (likely in alloc_super()) and f_fsid generated from sb->s_uuid. This way we could handle all these filesystems with trivial amount of effort.
Patch for simple fs fsid also queued on Christian's vfs.f_fsid branch.
quoted
Christian, I recall that you may have had reservations on initializing s_uuid and f_fsid in vfs code? Does an opt-in fstype flag address your concerns? Will you be ok with doing the tmpfs/kernfs trick for every fs that opted-in with fstype flag in generic vfs code?quoted
freevxfs - the only real filesystem without f_fsid. Trivial to handle one way or the other.
fsid patch was posted for freevxfs. fsid patch for gfs2 was posted and applied by the maintainer.
quoted
Last but not least, btrfs subvolumes. They do have an fsid, but it is different from the sb fsid, so we disallow (even inode) fanotify marks. I am not sure how to solve this one, but if we choose to implement the opt-in fanotify flag for "watch single fs", we can make this problem go away, along with the problem of network fs fsid and other odd fs that we do not want to have to deal with. On top of everything, it is a fast solution and it doesn't involve vfs and changing any fs at all.Yeah, right, forgot about this one. Thanks for reminding me. But this is mostly a kernel internal implementation issue and doesn't seem to be a principial problem so I'd prefer not to complicate the uAPI for this. We could for example mandate a special super_operation for fetching fsid for a dentry for filesystems which don't have uniform fsids over the whole filesystem (i.e., btrfs) and call this when generating event for such filesystems. Or am I missing some other complication?
No complication AFAICS. btrfs fsid patches posted for review. Thanks, Amir.