Thread (13 messages) 13 messages, 3 authors, 2023-10-25

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 anyway
Be 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help