Re: [PATCH v19 29/40] dept: make dept aware of lockdep_set_lock_cmp_fn() annotation

[sashiko-bot]

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 <byungchul@xxxxxx>

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.

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.

Should lockdep_cmp_fn() be allowed to evaluate against any previously held
lock in the chain, rather than just the immediate predecessor?

> +
> +		add_dep(eh->ecxt, w);
>  	}
>  
>  	wg = next_wgen();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@xxxxxx?part=29