Re: [PATCH v1 01/12] VFS: don't count references through ->d_parent

From: NeilBrown

Date: Tue Aug 11 2026 - 19:36:23 EST


On Tue, 11 Aug 2026, Miklos Szeredi wrote:
> On Tue, 11 Aug 2026 at 04:12, NeilBrown <neilb@xxxxxxxxxxx> 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
>