On Tue, Jul 07, 2026 at 07:33:45AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [Medium] DEPT falsely reports a deadlock when a task sequentially acquires
> three or more locks of the same class (or interleaves them with other locks),
> because the `lockdep_cmp_fn` bypass is improperly restricted to only the
> immediately preceding lock.
> --
>
> commit d03c5b926a945b89f5a90fd98986386e113df5af
> Author: Byungchul Park <[email protected]>
>
> dept: make dept aware of lockdep_set_lock_cmp_fn() annotation
>
> This commit updates the DEPT (Dependency Tracker) subsystem to respect the
> lockdep_set_lock_cmp_fn() annotation. It allows DEPT to avoid false positive
> deadlock reports by using the custom comparison function when evaluating
> dependencies between locks of the same class.
>
> > diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
> > --- a/kernel/dependency/dept.c
> > +++ b/kernel/dependency/dept.c
>
> [ ... ]
>
> > @@ -1658,8 +1688,13 @@ static void add_wait(struct dept_class *c, unsigned
> > long ip,
> > if (!eh->ecxt)
> > continue;
> >
> > - if (eh->ecxt->class != c || eh->sub_l == sub_l)
> > - add_dep(eh->ecxt, w);
> > + if (eh->ecxt->class == c && eh->sub_l != sub_l)
> > + continue;
> > +
> > + if (i == dt->ecxt_held_pos - 1 && lockdep_cmp_fn(eh->map, m))
> > + continue;
>
> [Severity: Medium]
> Will this condition incorrectly bypass the comparison for earlier locks in
> the chain?
>
> By restricting the lockdep_cmp_fn() check to only the most recently
> acquired lock (i == dt->ecxt_held_pos - 1), it seems we might miss valid
> comparisons for older contexts.
No.
It's how lockdep works and it's intended by lockdep. The temporary code
just follows the rule.
> For example, if a task acquires lock A1, then lock B, and then lock A2
> (where A1 and A2 are the same class), the check against A1 will be skipped
> because A1 is no longer at dt->ecxt_held_pos - 1.
>
> This would result in a call to add_dep(A1, A2), creating a
> self-referential class dependency that triggers a false positive deadlock
> report.
No.
It's a real deadlock.
> Should lockdep_cmp_fn() be allowed to evaluate against any previously held
> lock in the chain, rather than just the immediate predecessor?
No.
Check the lockdep code first. See check_deadlock() and check_prev_add().
Byungchul
> > +
> > + add_dep(eh->ecxt, w);
> > }
> >
> > wg = next_wgen();
>
> --
> Sashiko AI review ยท
> https://sashiko.dev/#/patchset/[email protected]?part=29
