On Thu, 4 Sept 2025 at 16:00, Luis Henriques <luis@xxxxxxxxxx> wrote:
>
> Hi Miklos,
>
> On Thu, Sep 04 2025, Miklos Szeredi wrote:
>
> > On Thu, 28 Aug 2025 at 18:30, Luis Henriques <luis@xxxxxxxxxx> wrote:
> >> + if (!inval_wq && RB_EMPTY_NODE(&fd->node))
> >> + return;
> >
> > inval_wq can change to zero, which shouldn't prevent removing from the rbtree.
>
> Maybe I didn't understood your comment, but isn't that what's happening
> here? If the 'fd' is in a tree, it will be removed, independently of the
> 'inval_wq' value.
I somehow thought it was || not &&.
But I still don't see the point. The only caller already checked
RB_EMPTY_NODE, so that is false. No race possible since it's called
form the destruction of the dentry, and so this expression is
guaranteed to evaluate to false.
> (By the way, I considered using mutexes here instead. Do you have any
> thoughts on this?)
Use mutex where protected code might sleep, spin lock otherwise.
>
> What I don't understand in your comment is where you suggest these helpers
> could be in a higher level. Could you elaborate on what exactly you have
> in mind?
E.g.
void d_dispose_if_unused(struct dentry *dentry, struct list_head *dispose)
{
spin_lock(&dentry->d_lock);
if (!dentry->d_lockref.count)
to_shrink_list(dentry, dispose);
spin_unlock(&dentry->d_lock);
}
Which is in fact taken from d_prune_aliases(), which could be modified
to use this helper.
Thanks,
Miklos
