On Wed, Apr 6, 2022 at 8:47 PM Jan Kara <jack@xxxxxxx> wrote: > > 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. > Yes, I see it now. IIRC, it was just a cleanup patch, I can remove it and amend the rest of the series. Thanks, Amir.
