Re: [PATCH v19 15/40] dept: track timeout waits separately with a new Kconfig

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Semantic collision in the timeoutval argument causes 0-timeout try-waits to be incorrectly tracked as hard blocking dependencies.
- [High] Enabling CONFIG_DEPT_AGGRESSIVE_TIMEOUT_WAIT masks real deadlocks because a timeout wait permanently poisons the dependency edge, causing subsequent strict (non-timeout) waits to be ignored.
--

commit 1b06fc68b17e6c7948f8187e55952d1767e48762
Author: Byungchul Park <byungchul@xxxxxx>

dept: track timeout waits separately with a new Kconfig

This patch modifies DEPT to track timeout waits separately to avoid
reporting false positive deadlocks for deliberate timeouts. It also
introduces the CONFIG_DEPT_AGGRESSIVE_TIMEOUT_WAIT option to toggle this
tracking behavior.

> diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
> index 3af360ba17d8e..5ef85977e6314 100644
> --- a/kernel/dependency/dept.c
> +++ b/kernel/dependency/dept.c
[ ... ]
> @@ -810,6 +812,24 @@ static void print_dep(struct dept_dep *d)
>  
>  static void save_current_stack(int skip);
>  
> +static bool is_timeout_wait_circle(struct dept_class *c)
> +{
> +	struct dept_class *fc = c->bfs_parent;
> +	struct dept_class *tc = c;
> +
> +	do {
> +		struct dept_dep *d = lookup_dep(fc, tc);
> +
> +		if (d->wait->timeout)
> +			return true;

[Severity: High]
If CONFIG_DEPT_AGGRESSIVE_TIMEOUT_WAIT is enabled, does this mask genuine
deadlocks if a strict wait follows a timeout wait?

If a timeout wait between class A and class B occurs first, it creates a
dependency edge with wait->timeout set to true. If a strict (non-timeout)
wait subsequently occurs between the same classes, add_dep() finds the
existing edge and returns early:

kernel/dependency/dept.c:add_dep() {
        if (lookup_dep(fc, tc))
                return;
        ...
}

The edge is never upgraded to reflect the stricter dependency. 

If a deadlock later happens involving this edge, would is_timeout_wait_circle()
see the stale timeout flag and incorrectly report it as an intended circular
dependency instead of a genuine deadlock?

> +
> +		tc = fc;
> +		fc = fc->bfs_parent;
> +	} while (tc != c);
> +
> +	return false;
> +}
[ ... ]
> @@ -2355,14 +2381,23 @@ static void __dept_event(struct dept_map *m, struct dept_map *real_m,
>  }
>  
>  void dept_wait(struct dept_map *m, unsigned long w_f,
> -	       unsigned long ip, const char *w_fn, int sub_l)
> +	       unsigned long ip, const char *w_fn, int sub_l,
> +	       long timeoutval)
>  {
>  	struct dept_task *dt = dept_task();
>  	unsigned long flags;
> +	bool timeout;
>  
>  	if (unlikely(!dept_working()))
>  		return;
>  
> +	timeout = timeoutval > 0 && timeoutval < MAX_SCHEDULE_TIMEOUT;

[Severity: High]
Does this logic correctly handle try-waits or polling?

When a caller invokes a polling wait like wait_for_completion_timeout(x, 0),
the timeoutval is 0. Because 0 > 0 evaluates to false, this sets timeout to
false. DEPT then tracks this non-blocking poll as an indefinite hard wait.

Can this lead to false positive deadlock reports when no actual blocking
occurs?

> +
> +#if !defined(CONFIG_DEPT_AGGRESSIVE_TIMEOUT_WAIT)
> +	if (timeout)
> +		return;
> +#endif
> +
>  	if (dt->recursive)
>  		return;
>

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