Re: [PATCH v1 01/12] VFS: don't count references through ->d_parent
NeilBrown <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.kernel.autofs,gmane.comp.file-systems.ceph.devel,gmane.comp.file-systems.coda.general,gmane.linux.file-systems,gmane.linux.nfs |
|---|---|
| Message-ID | <[email protected]> |
On Tue, 11 Aug 2026, Miklos Szeredi wrote: > On Tue, 11 Aug 2026 at 04:12, NeilBrown <[email protected]> wrote: > > > --- a/fs/libfs.c > > +++ b/fs/libfs.c > > @@ -169,6 +169,8 @@ loff_t dcache_dir_lseek(struct file *file, loff_t offset, int whence) > > hlist_del_init(&cursor->d_sib); > > if (to) > > hlist_add_behind(&cursor->d_sib, &to->d_sib); > > + else if (hlist_empty(&dentry->d_children) > > + dput_dlock(dentry); > > you mean dput_dlock(cursor)? No, I really do mean dput_dlock(dentry). dentry->d_children has just transitioned from non-empty to empty, so we need to decrement the refcount. Though maybe cursor wasn't actually on the d_children list. hlist_del_init() checks if it is on the list before removing it, so we lose that info. I've been wondering if some helpers would make this clearer. static inline void hlist_move_behind(struct hlist_node *n, struct hlist_node *prev) { hlist_del_init(n); hlist_add_behind(n, prev); } static inline void d_detach_cursor(struct dentry *cursor) { if (hlist_unhashed(&cursor->d_sib)) return; hlist_del_init(&cursor->d_sib); if (hlist_empty(&cursor->d_parent->d_children)) /* ->d_children has just become empty, so drop the implied reference */ dput_dlock(cursor->d_parent); } Then the above snippet would be if (to) hlist_move_behind(&cursor->d_sib, &to->d_sib); else d_detach_cursor(cursor); Thanks, NeilBrown > > But that's no good, since the directory could be repopulated and the > cursor needed again. > > Thanks, > Miklos >