On Wed 31-03-21 14:29:04, Amir Goldstein wrote: > On Wed, Mar 31, 2021 at 12:46 PM Christian Brauner > <christian.brauner@xxxxxxxxxx> wrote: > > > > On Tue, Mar 30, 2021 at 05:56:25PM +0300, Amir Goldstein wrote: > > > > > > My example probably would be something like: > > > > > > > > > > > > mount -t ext4 /dev/sdb /A > > > > > > > > > > > > 1. FAN_MARK_MOUNT(/A) > > > > > > > > > > > > mount --bind /A /B > > > > > > > > > > > > 2. FAN_MARK_MOUNT(/B) > > > > > > > > > > > > mount -t ecryptfs /B /C > > > > > > > > > > > > 3. FAN_MARK_MOUNT(/C) > > > > > > > > > > > > let's say I now do > > > > > > > > > > > > touch /C/bla > > > > > > > > > > > > I may be way off here but intuitively it seems both 1. and 2. should get > > > > > > a creation event but not 3., right? > > > > > > > > > > > > > > > > Why not 3? > > > > > You explicitly set a mark on /C requesting to be notified when > > > > > objects are created via /C. > > > > > > > > Sorry, that was a typo. I meant to write, both 2. and 3. should get a > > > > creation event but not 1. > > > > > > > > > > > > > > > But with your proposal would both 1. and 2. still get a creation event? > > > > > > > > > > > > > > Same obvious typo. The correct question would be: with your proposal do > > > > 2. and 3. both get an event? > > > > > > > > Because it feels like they both should since /C is mounted on top of /B > > > > and ecryptfs acts as a shim. Both FAN_MARK_MOUNT(/B) and > > > > FAN_MARK_MOUNT(/C) should get a creation event after all both will have > > > > mnt->mnt_fsnotify_marks set. > > > > > > > > > > Right. > > > > > > There are two ways to address this inconsistency: > > > 1. Change internal callers of vfs_ helpers to use a private mount, > > > as you yourself suggested for ecryptfs and cachefiles > > > > I feel like this is he correct thing to do independently of the fanotify > > considerations. I think I'll send an RFC for this today or later this > > week. > > > > > 2. Add fsnotify_path_ hooks at caller site - that would be the > > > correct thing to do for nfsd IMO > > > > I do not have an informed opinion yet on nfsd so I simply need to trust > > you here. :) > > > > As long as "exp_export: export of idmapped mounts not yet supported.\n" > I don't think it matters much. > It feels like adding idmapped mounts to nfsd is on your roadmap. > When you get to that we can discuss adding fsnotify path hooks to nfsd > if Jan agrees to the fsnotify path hooks concept. I was looking at the patch and thinking about it for a few days already. I think that generating fsnotify event later (higher up the stack where we have mount information) is fine and a neat idea. I just dislike the hackery with dentry flags. Also I'm somewhat uneasy that it is random (from userspace POV) when path event is generated and when not (at least that's my impression from the patch - maybe I'm wrong). How difficult would it be to get rid of it? I mean what if we just moved say fsnotify_create() call wholly up the stack? It would mean more explicit calls to fsnotify_create() from filesystems - as far as I'm looking nfsd, overlayfs, cachefiles, ecryptfs. But that would seem to be manageable. Also, to maintain sanity, we would probably have to lift generation of all directory events like that. That would be already notable churn but maybe doable... I know you've been looking at similar things in the past so if you are aware why this won't fly, please tell me. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
