On Thu 20-01-22 16:31:11, Amir Goldstein wrote:
> On Thu, Jan 20, 2022 at 2:52 PM Jan Kara <jack@xxxxxxx> wrote:
> > > +/*
> > > + * fsnotify_delete - @dentry was unlinked and unhashed
> > > + *
> > > + * Caller must make sure that dentry->d_name is stable.
> > > + *
> > > + * Note: unlike fsnotify_unlink(), we have to pass also the unlinked inode
> > > + * as this may be called after d_delete() and old_dentry may be negative.
> > > + */
> > > +static inline void fsnotify_delete(struct inode *dir, struct inode *inode,
> > > + struct dentry *dentry)
> > > +{
> > > + __u32 mask = FS_DELETE;
> > > +
> > > + if (S_ISDIR(inode->i_mode))
> > > + mask |= FS_ISDIR;
> > > +
> > > + fsnotify_name(mask, inode, FSNOTIFY_EVENT_INODE, dir, &dentry->d_name,
> > > + 0);
> > > +}
> > > +
> >
> > OK, this is fine because we use dentry only for FAN_RENAME event, don't we?
>
> Almost.
> We also use dentry in FS_CREATE to get sb from d_sb for error event, because:
> * Note: some filesystems (e.g. kernfs) leave @dentry negative and instantiate
> * ->d_inode later
Ah, right.
> > In all other cases we always use only inode anyway. Can we perhaps cleanup
> > include/linux/fsnotify.h to use FSNOTIFY_EVENT_DENTRY only in that one call
> > site inside fsnotify_move() and use FSNOTIFY_EVENT_INODE in all the other
> > cases? So that this is clear and also so that we don't start using dentry
> > inadvertedly for something inside fsnotify thus breaking unlink reporting
> > in subtle ways...
> >
>
> I don't know.
> For fsnotify_unlink/rmdir we check d_is_negative, so it's fine to use
> FSNOTIFY_EVENT_INODE.
> For fsnotify_link,fsnotify_move we get the inode explicitly, but we already
> use FSNOTIFY_EVENT_INODE in those cases (except FS_RENAME).
Yeah, plus we have the xattr and attrib events which need dentry to lookup
parent. So scratch this idea.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
