On Tue 29-03-22 10:48:52, Amir Goldstein wrote:
> s_fsnotify_connectors is elevated for every inode mark in addition to
> the refcount already taken by the inode connector.
>
> This is a relic from s_fsnotify_inode_refs pre connector era.
> Remove those unneeded recounts.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
I disagree it is a relict. fsnotify_sb_delete() relies on
s_fsnotify_connectors to wait for all connectors to be properly torn down
on unmount so that we don't get "Busy inodes after unmount" error messages
(and use-after-free issues). Am I missing something?
Honza
> ---
> fs/notify/mark.c | 21 +++------------------
> 1 file changed, 3 insertions(+), 18 deletions(-)
>
> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index b1443e66ba26..698ed0a1a47e 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -169,21 +169,6 @@ static void fsnotify_connector_destroy_workfn(struct work_struct *work)
> }
> }
>
> -static void fsnotify_get_inode_ref(struct inode *inode)
> -{
> - ihold(inode);
> - atomic_long_inc(&inode->i_sb->s_fsnotify_connectors);
> -}
> -
> -static void fsnotify_put_inode_ref(struct inode *inode)
> -{
> - struct super_block *sb = inode->i_sb;
> -
> - iput(inode);
> - if (atomic_long_dec_and_test(&sb->s_fsnotify_connectors))
> - wake_up_var(&sb->s_fsnotify_connectors);
> -}
> -
> static void fsnotify_get_sb_connectors(struct fsnotify_mark_connector *conn)
> {
> struct super_block *sb = fsnotify_connector_sb(conn);
> @@ -245,7 +230,7 @@ static void fsnotify_drop_object(unsigned int type, void *objp)
> /* Currently only inode references are passed to be dropped */
> if (WARN_ON_ONCE(type != FSNOTIFY_OBJ_TYPE_INODE))
> return;
> - fsnotify_put_inode_ref(objp);
> + iput(objp);
> }
>
> void fsnotify_put_mark(struct fsnotify_mark *mark)
> @@ -519,7 +504,7 @@ static int fsnotify_attach_connector_to_object(fsnotify_connp_t *connp,
> }
> if (conn->type == FSNOTIFY_OBJ_TYPE_INODE) {
> inode = fsnotify_conn_inode(conn);
> - fsnotify_get_inode_ref(inode);
> + ihold(inode);
> }
> fsnotify_get_sb_connectors(conn);
>
> @@ -530,7 +515,7 @@ static int fsnotify_attach_connector_to_object(fsnotify_connp_t *connp,
> if (cmpxchg(connp, NULL, conn)) {
> /* Someone else created list structure for us */
> if (inode)
> - fsnotify_put_inode_ref(inode);
> + iput(inode);
> fsnotify_put_sb_connectors(conn);
> kmem_cache_free(fsnotify_mark_connector_cachep, conn);
> }
> --
> 2.25.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
