Re: [RFC][PATCH] fanotify: Enable FAN_REPORT_FID on more filesystem types

[Amir Goldstein <amir73il@gmail.com>]

> > 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...

> > 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.

> >
> > > > > 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:
> >
> > > ...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.

>
> > 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.
>
> > > 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.

> >
> > 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?
> >
> > > 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.

> >
> > 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.