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 <[email protected]> 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/[email protected]?part=15
