From: Amir Goldstein Since we only ever set the PARENT_WATCHED flag from fsnotify_recalc_mask(), there is never a need to update children flags when removing mark mask bits only when adding them. Add a helper fsnotify_recalc_mask_and_update_children() to be called in cases where updating children flags may be needed (i.e. add mark). Also avoid update children walk with mark spinlock held. This was only done by dnotify and can be easily avoided. neilb: added update_children flag in fcntl_dirnotify() so we only do the update when needed. Fixes: 172e422ffea20 ("fsnotify: clear PARENT_WATCHED flags lazily") Signed-off-by: Amir Goldstein Signed-off-by: NeilBrown --- fs/notify/dnotify/dnotify.c | 12 ++++++--- fs/notify/fanotify/fanotify_user.c | 2 +- fs/notify/inotify/inotify_user.c | 2 +- fs/notify/mark.c | 40 +++++++++++++++++++++--------- include/linux/fsnotify_backend.h | 7 +++++- 5 files changed, 44 insertions(+), 19 deletions(-) diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c index 9fb73bafd41d..ccd93928ea86 100644 --- a/fs/notify/dnotify/dnotify.c +++ b/fs/notify/dnotify/dnotify.c @@ -58,8 +58,9 @@ struct dnotify_mark { * the set of all those events. After it updates what dnotify is interested in * it calls the fsnotify function so it can update the set of all events relevant * to this inode. + * Returns "true" if fsnotify_conn_set_children_dentry_flags() needs to be called. */ -static void dnotify_recalc_inode_mask(struct fsnotify_mark *fsn_mark) +static bool dnotify_recalc_inode_mask(struct fsnotify_mark *fsn_mark) { __u32 new_mask = 0; struct dnotify_struct *dn; @@ -72,10 +73,10 @@ static void dnotify_recalc_inode_mask(struct fsnotify_mark *fsn_mark) for (dn = dn_mark->dn; dn != NULL; dn = dn->dn_next) new_mask |= (dn->dn_mask & ~FS_DN_MULTISHOT); if (fsn_mark->mask == new_mask) - return; + return false; fsn_mark->mask = new_mask; - fsnotify_recalc_mask(fsn_mark->connector); + return fsnotify_recalc_mask(fsn_mark->connector); } /* @@ -265,6 +266,7 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned int arg) fl_owner_t id = current->files; struct file *f = NULL; int destroy = 0, error = 0; + bool update_children = false; __u32 mask; /* we use these to tell if we need to kfree */ @@ -377,9 +379,11 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned int arg) else if (error == -EEXIST) error = 0; - dnotify_recalc_inode_mask(fsn_mark); + update_children = dnotify_recalc_inode_mask(fsn_mark); out: spin_unlock(&fsn_mark->lock); + if (update_children) + fsnotify_conn_set_children_dentry_flags(fsn_mark->connector); if (destroy) fsnotify_detach_mark(fsn_mark); diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index a32c6634d592..d7c03d25ffe1 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -1538,7 +1538,7 @@ static int fanotify_add_mark(struct fsnotify_group *group, recalc = fanotify_mark_add_to_mask(fsn_mark, mask, fan_flags); if (recalc) - fsnotify_recalc_mask(fsn_mark->connector); + fsnotify_recalc_mask_and_update_children(fsn_mark->connector); out: fsnotify_group_unlock(group); diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 5f19c24ec187..6018c2d717ac 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -562,7 +562,7 @@ static int inotify_update_existing_watch(struct fsnotify_group *group, fsn_mark->flags |= inotify_arg_to_flags(arg); spin_unlock(&fsn_mark->lock); - fsnotify_recalc_mask(fsn_mark->connector); + fsnotify_recalc_mask_and_update_children(fsn_mark->connector); /* return the wd */ ret = i_mark->wd; diff --git a/fs/notify/mark.c b/fs/notify/mark.c index cb07f42d3422..d289843ea149 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -307,7 +307,7 @@ static bool fsnotify_conn_watches_children( return fsnotify_inode_watches_children(fsnotify_conn_inode(conn)); } -static void fsnotify_conn_set_children_dentry_flags( +void fsnotify_conn_set_children_dentry_flags( struct fsnotify_mark_connector *conn) { if (conn->type != FSNOTIFY_OBJ_TYPE_INODE) @@ -321,25 +321,41 @@ static void fsnotify_conn_set_children_dentry_flags( * connector and connector->obj cannot disappear under us. Callers achieve * this by holding a mark->lock or mark->group->mark_mutex for a mark on this * list. + * + * Returns true if the connector started watching children as a result of + * the mask recalculation. Callers that may be adding watch bits should + * use fsnotify_recalc_mask_and_update_children() or call + * fsnotify_conn_set_children_dentry_flags() when safe to do so. */ -void fsnotify_recalc_mask(struct fsnotify_mark_connector *conn) +bool fsnotify_recalc_mask(struct fsnotify_mark_connector *conn) { bool update_children; if (!conn) - return; + return false; spin_lock(&conn->lock); update_children = !fsnotify_conn_watches_children(conn); fsnotify_recalc_mask_set_iref(conn); update_children &= fsnotify_conn_watches_children(conn); spin_unlock(&conn->lock); - /* - * Set children's PARENT_WATCHED flags only if parent started watching. - * When parent stops watching, we clear false positive PARENT_WATCHED - * flags lazily in __fsnotify_parent(). - */ - if (update_children) + + return update_children; +} + +/* + * Recalculate mask and update children dentry PARENT_WATCHED flags if the + * connector started watching children. When parent stops watching, we + * clear false positive PARENT_WATCHED flags lazily in __fsnotify_parent(). + * + * The caller must make sure connector and connector->obj cannot disappear + * under us. This is achieved by holding mark->group->mark_mutex for a + * mark on this list. mark->lock must not be held by the caller. + */ +void fsnotify_recalc_mask_and_update_children( + struct fsnotify_mark_connector *conn) +{ + if (fsnotify_recalc_mask(conn)) fsnotify_conn_set_children_dentry_flags(conn); } @@ -367,8 +383,8 @@ void fsnotify_modify_mark_mask(struct fsnotify_mark *mark, u32 set, u32 clear) recalc = true; spin_unlock(&mark->lock); - if (recalc) - fsnotify_recalc_mask(mark->connector); + if (recalc && fsnotify_recalc_mask(mark->connector) && set) + fsnotify_conn_set_children_dentry_flags(mark->connector); } EXPORT_SYMBOL_GPL(fsnotify_modify_mark_mask); @@ -1007,7 +1023,7 @@ int fsnotify_add_mark_locked(struct fsnotify_mark *mark, if (ret) goto err; - fsnotify_recalc_mask(mark->connector); + fsnotify_recalc_mask_and_update_children(mark->connector); return ret; err: diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index a6d5c1a89cff..e637bad698a5 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -887,7 +887,12 @@ static inline __u32 fsnotify_calc_mask(struct fsnotify_mark *mark) /* Get mask of events for a list of marks */ extern __u32 fsnotify_conn_mask(struct fsnotify_mark_connector *conn); /* Calculate mask of events for a list of marks */ -extern void fsnotify_recalc_mask(struct fsnotify_mark_connector *conn); +extern bool fsnotify_recalc_mask(struct fsnotify_mark_connector *conn); +/* Recalculate mask and update children dentry flags if needed */ +extern void fsnotify_recalc_mask_and_update_children( + struct fsnotify_mark_connector *conn); +extern void fsnotify_conn_set_children_dentry_flags( + struct fsnotify_mark_connector *conn); extern void fsnotify_init_mark(struct fsnotify_mark *mark, struct fsnotify_group *group); /* Find mark belonging to given group in the list of marks */ -- 2.50.0.107.gf914562f5916.dirty