On Tue 05-04-22 16:09:00, Amir Goldstein wrote: > On Tue, Apr 5, 2022 at 3:54 PM Jan Kara <jack@xxxxxxx> wrote: > > > > 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? > > > > I meant it is a relic from the time before s_fsnotify_inode_refs became > s_fsnotify_connectors. > > Nowadays, one s_fsnotify_connectors refcount per connector is enough. > No need for one refcount per inode. > > Open code the the sequence: > if (inode) > fsnotify_put_inode_ref(inode); > fsnotify_put_sb_connectors(conn); > > To see how silly it is. I see your point and I agree the general direction makes sense but technically I think your patch is buggy. Because notice that we do fsnotify_put_sb_connectors() in fsnotify_detach_connector_from_object() so after this call there's nothing blocking umount while we can be still holding inode references from some marks attached to this connector. So racing inode mark destruction & umount can lead to "Busy inodes after umount" messages. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
