On Wed 14-11-18 19:43:40, Amir Goldstein wrote: > Define a new data type to pass for event FSNOTIFY_EVENT_DENTRY > and use it whenever a dentry is available instead of passing > it's ->d_inode as data type FSNOTIFY_EVENT_INODE. > > None of the current fsnotify backends make use of the inode data > with data type FSNOTIFY_EVENT_INODE - only the data of type > FSNOTIFY_EVENT_PATH is ever used, so this change has no immediate > consequences. > > Soon, we are going to use the dentry data type to support more > events with fanotify backend. > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> One general coment: Audit is using FSNOTIFY_EVENT_INODE and this change is going to break it. So it needs updating as well. Also I'd prefer a more justified selection of dentry events than 'those where we can do it'. Because eventually that set will translate to events available to userspace in fanotify and that should be a reasonably consistent set. So here I suspect you target at directory modification events (you call them filename events in the next patch). So just define which FS_<foo> events these exactly are and convert exactly those event helpers to dentry ones... > diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h > index 2ccb08cb5d6a..9dadc0bcd7a9 100644 > --- a/include/linux/fsnotify.h > +++ b/include/linux/fsnotify.h > @@ -99,14 +99,14 @@ static inline void fsnotify_move(struct inode *old_dir, struct inode *new_dir, > > fsnotify(old_dir, old_dir_mask, source, FSNOTIFY_EVENT_INODE, old_name, > fs_cookie); > - fsnotify(new_dir, new_dir_mask, source, FSNOTIFY_EVENT_INODE, new_name, > + fsnotify(new_dir, new_dir_mask, moved, FSNOTIFY_EVENT_DENTRY, new_name, > fs_cookie); > > if (target) > fsnotify_link_count(target); > > if (source) > - fsnotify(source, FS_MOVE_SELF, moved->d_inode, FSNOTIFY_EVENT_INODE, NULL, 0); > + fsnotify(source, FS_MOVE_SELF, moved, FSNOTIFY_EVENT_DENTRY, NULL, 0); > audit_inode_child(new_dir, moved, AUDIT_TYPE_CHILD_CREATE); > } > I'd postpone these fsnotify_move() changes to a patch where you make fsnotify_move() fully dentry based. Having it converted half-way looks confusing... > @@ -168,7 +168,7 @@ static inline void fsnotify_link(struct inode *dir, struct inode *inode, struct > fsnotify_link_count(inode); > audit_inode_child(dir, new_dentry, AUDIT_TYPE_CHILD_CREATE); > > - fsnotify(dir, FS_CREATE, inode, FSNOTIFY_EVENT_INODE, new_dentry->d_name.name, 0); > + fsnotify(dir, FS_CREATE, new_dentry, FSNOTIFY_EVENT_DENTRY, new_dentry->d_name.name, 0); > } So a change like this makes me slightly nervous. What guarantees that new_dentry->d_inode is going to be the same thing as 'inode' passed in? Aren't races changing new_dentry before we look at new_dentry->d_inode possible? In this specific case I don't think anything unexpected is possible - we hold i_rwsem, hopefully nobody does anything fancy like instantiating a different inode in their ->link method. But generally non-obvious changes like this should be split out in separate commits with justification why the change is safe. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
