On Thu 10-01-19 19:04:28, Amir Goldstein wrote:
> "dirent" events are referring to events that modify directory entries,
> such as create,delete,rename. Those events should always be reported
> on a watched directory, regardless if FS_EVENT_ON_CHILD is set
> on the watch mask.
>
> fsnotify_nameremove() and fsnotify_move() were modified to no longer
> set the FS_EVENT_ON_CHILD event bit. This is a semantic change to
> align with the "dirent" event definition. It has no effect on any
> existing backend, because dnotify, inotify and audit always requets the
> child events and fanotify does not get the delete,rename events.
>
> The fsnotify_dirent() helper is used instead of fsnotify_parent() to
> report a dirent event to dentry->d_parent without FS_EVENT_ON_CHILD
> and regardless if parent has the FS_EVENT_ON_CHILD bit set.
>
> Unlike fsnotify_parent(), fsnotify_dirent() assumes that dentry->d_name
> and dentry->d_parent are stable. For fsnotify_create()/fsnotify_mkdir(),
> this assumption is abviously correct. For fsnotify_nameremove(), it is
> less trvial, so we use dget_parent() and take_dentry_name_snapshot() to
> grab stable references.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
I've taken the liberty of removing the case analysis from
fsnotify_nameremove() comment. That is bound to become out of date pretty
soon and not very useful for the reader. Otherwise the patch looks good and
I've added it to my tree.
Honza
> ---
> include/linux/fsnotify.h | 68 ++++++++++++++++++++++++++++++++++------
> 1 file changed, 59 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 2ccb08cb5d6a..116907928c7f 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -17,8 +17,22 @@
> #include <linux/slab.h>
> #include <linux/bug.h>
>
> +/*
> + * Notify this @dir inode about a change in the directory entry @dentry.
> + *
> + * Unlike fsnotify_parent(), the event will be reported regardless of the
> + * FS_EVENT_ON_CHILD mask on the parent inode.
> + */
> +static inline int fsnotify_dirent(struct inode *dir, struct dentry *dentry,
> + __u32 mask)
> +{
> + return fsnotify(dir, mask, d_inode(dentry), FSNOTIFY_EVENT_INODE,
> + dentry->d_name.name, 0);
> +}
> +
> /* Notify this dentry's parent about a child's events. */
> -static inline int fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask)
> +static inline int fsnotify_parent(const struct path *path,
> + struct dentry *dentry, __u32 mask)
> {
> if (!dentry)
> dentry = path->dentry;
> @@ -85,8 +99,8 @@ static inline void fsnotify_move(struct inode *old_dir, struct inode *new_dir,
> {
> struct inode *source = moved->d_inode;
> u32 fs_cookie = fsnotify_get_cookie();
> - __u32 old_dir_mask = (FS_EVENT_ON_CHILD | FS_MOVED_FROM);
> - __u32 new_dir_mask = (FS_EVENT_ON_CHILD | FS_MOVED_TO);
> + __u32 old_dir_mask = FS_MOVED_FROM;
> + __u32 new_dir_mask = FS_MOVED_TO;
> const unsigned char *new_name = moved->d_name.name;
>
> if (old_dir == new_dir)
> @@ -128,15 +142,54 @@ static inline void fsnotify_vfsmount_delete(struct vfsmount *mnt)
>
> /*
> * fsnotify_nameremove - a filename was removed from a directory
> + *
> + * Called from d_delete() and nfs_complete_sillyrename().
> + * The latter is called from nfs client ->unlink() ->rmdir() ->rename()
> + * under parent vfs inode lock.
> + *
> + * Most filesystems call d_delete() from ->unlink() ->rmdir() ->rename()
> + * ops under parent vfs inode lock.
> + *
> + * Some pseudo filesystems call d_delete() without parent inode lock.
> + * Those filesystems have no ->rename() op and they do not call
> + * d_move() directly, so d_parent and d_name are stable by definition.
> + * Examples: devpts, efivarfs, rpc_pipefs, functionfs.
> + *
> + * Some clustered filesystems call d_delete() on remote nodes, not under
> + * vfs parent inode lock, but they use cluster distributed locks on local
> + * and remote nodes. Those filesystems call d_delete() under their cluster
> + * lock. Examples:
> + * - in ceph_fill_trace() under CEPH_MDS_R_PARENT_LOCKED
> + * - in ocfs2_dentry_convert_worker() under ocfs2_dentry_lock
> + * But those filesystems also call d_move() under the same cluster lock
> + * (i.e. FS_RENAME_DOES_D_MOVE), so d_parent and d_name are also stable.
> + *
> + * However, to be on the safe side and be reselient to future callers
> + * and out of tree users of d_delete(), we do not assume that d_parent and
> + * d_name are stable and we use dget_parent() and take_dentry_name_snapshot()
> + * to grab stable references.
> */
> static inline void fsnotify_nameremove(struct dentry *dentry, int isdir)
> {
> + struct dentry *parent;
> + struct name_snapshot name;
> __u32 mask = FS_DELETE;
>
> + /* d_delete() of pseudo inode? (e.g. __ns_get_path() playing tricks) */
> + if (IS_ROOT(dentry))
> + return;
> +
> if (isdir)
> mask |= FS_ISDIR;
>
> - fsnotify_parent(NULL, dentry, mask);
> + parent = dget_parent(dentry);
> + take_dentry_name_snapshot(&name, dentry);
> +
> + fsnotify(d_inode(parent), mask, d_inode(dentry), FSNOTIFY_EVENT_INODE,
> + name.name, 0);
> +
> + release_dentry_name_snapshot(&name);
> + dput(parent);
> }
>
> /*
> @@ -155,7 +208,7 @@ static inline void fsnotify_create(struct inode *inode, struct dentry *dentry)
> {
> audit_inode_child(inode, dentry, AUDIT_TYPE_CHILD_CREATE);
>
> - fsnotify(inode, FS_CREATE, dentry->d_inode, FSNOTIFY_EVENT_INODE, dentry->d_name.name, 0);
> + fsnotify_dirent(inode, dentry, FS_CREATE);
> }
>
> /*
> @@ -176,12 +229,9 @@ static inline void fsnotify_link(struct inode *dir, struct inode *inode, struct
> */
> static inline void fsnotify_mkdir(struct inode *inode, struct dentry *dentry)
> {
> - __u32 mask = (FS_CREATE | FS_ISDIR);
> - struct inode *d_inode = dentry->d_inode;
> -
> audit_inode_child(inode, dentry, AUDIT_TYPE_CHILD_CREATE);
>
> - fsnotify(inode, mask, d_inode, FSNOTIFY_EVENT_INODE, dentry->d_name.name, 0);
> + fsnotify_dirent(inode, dentry, FS_CREATE | FS_ISDIR);
> }
>
> /*
> --
> 2.17.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
