On Wed 14-11-18 19:43:41, Amir Goldstein wrote:
> Filename 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.
OK, I find 'directory modification events' clearer than 'filename events'.
But I can live with your name since I don't really have a better
alternative :). Just please define these events in terms of all FS_<foo>
events that are involved so that everyone is on the same page which events
you mean.
> 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 filename event definition. It has no effect on any
> existing backend, because dnotify and inotify always requets the
> child events and fanotify does not get the delete,rename events.
You keep forgetting about audit ;) But in this case it is fine as it always
sets FS_EVENT_ON_CHILD as well.
> The fsnotify_filename() helper is used to report all the filename
> events. It gets a reference on parent dentry and passes it as the
> data for the event along with the filename.
>
> fsnotify_filename() is different from fsnotify_parent().
> fsnotify_parent() is intended to report any events that happened on
> child inodes when FS_EVENT_ON_CHILD is requested.
> fsnotify_filename() is intended to report only filename events,
> such as create,mkdir,link. Those events must always be reported
> on a watched directory, regardless if FS_EVENT_ON_CHILD was requested.
>
> fsnotify_d_name() is a helper for the common case where the
> filename to pass is dentry->d_name.name.
>
> It is safe to use these helpers with negative or not instantiated
> dentries, such as the case with fsnotify_link() and
> fsnotify_nameremove().
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> include/linux/fsnotify.h | 40 +++++++++++++++++++++++++++++++---------
> 1 file changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 9dadc0bcd7a9..d00ec5838d6e 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -17,8 +17,31 @@
> #include <linux/slab.h>
> #include <linux/bug.h>
>
> +/*
> + * Notify this parent about a filename event (create,delete,rename).
> + * Unlike fsnotify_parent(), the event will be reported regardless of the
> + * FS_EVENT_ON_CHILD mask on the parent inode
> + */
How about specifying this as 'Notify directory 'parent' about a change of
some of its directory entries'? That way you avoid using 'filename' event
in the description which is not defined.
> +static inline int fsnotify_filename(struct dentry *parent, __u32 mask,
> + const unsigned char *file_name, u32 cookie)
And how about calling the function fsnotify_dir_change()?
> +{
> + return fsnotify(d_inode(parent), mask, parent, FSNOTIFY_EVENT_DENTRY,
> + file_name, cookie);
> +}
> +
> +/*
> + * Call fsnotify_filename() with parent and d_name of this dentry.
> + * Safe to call with negative dentry, e.g. from fsnotify_nameremove()
> + */
> +static inline int fsnotify_d_name(struct dentry *dentry, __u32 mask)
Maybe call this fsnotify_dir_dentry_change()?
> +{
> + return fsnotify_filename(dentry->d_parent, mask,
> + 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 +108,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;
You can just inline these two masks now. There's no point for the variable
anymore.
> @@ -99,8 +122,7 @@ 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, moved, FSNOTIFY_EVENT_DENTRY, new_name,
> - fs_cookie);
> + fsnotify_filename(moved->d_parent, new_dir_mask, new_name, fs_cookie);
The same comment as for the patch 1 here. You should justify that
moved->d_parent is actually the same as new_dir and the dentry cannot
change under us. The same holds for all the calls of fsnotify_d_name()
where the directory inode originally passed in gets replaced with
d_inode(dentry->d_parent).
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
