Re: [PATCH v19 19/40] dept: apply timeout consideration to hashed-waitqueue wait

On Tue, Jul 07, 2026 at 07:33:36AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing `sdt_might_sleep_end()` cleanup in the error path of wait macros (e.g., `___wait_var_event`, `___wait_event`, `___swait_event`) bypasses `dept_clean_stage()`, leaking the staged wait event state and corrupting DEPT tracking.
> --
> 
> commit cf04720896ab037609320947c88e8faa57dbddd7
> Author: Byungchul Park <[email protected]>
> 
> dept: apply timeout consideration to hashed-waitqueue wait
> 
> Now that CONFIG_DEPT_AGGRESSIVE_TIMEOUT_WAIT was introduced, apply the
> consideration to hashed-waitqueue wait, assuming an input 'ret' in
> ___wait_var_event() macro is used as a timeout value.
> 
> > diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h
> > index 179a616ad245a..9885ac4e1ded5 100644
> > --- a/include/linux/wait_bit.h
> > +++ b/include/linux/wait_bit.h
> > @@ -258,7 +258,7 @@ extern wait_queue_head_t *__var_waitqueue(void *p);
> >       struct wait_bit_queue_entry __wbq_entry;                        \
> >       long __ret = ret; /* explicit shadow */                         \
> >                                                                       \
> > -     sdt_might_sleep_start(NULL);                                    \
> > +     sdt_might_sleep_start_timeout(NULL, __ret);                     \
> 
> [Severity: High]
> Does this error path properly clean up the dept wait state?
> 
> Lower in the ___wait_var_event() macro, if a signal is received it jumps to
> __out:
> 
>                 if (___wait_is_interruptible(state) && __int) {         \
>                         __ret = __int;                                  \
>                         goto __out;                                     \
>                 }                                                       \
> 
>                 ...
> 
>         sdt_might_sleep_end();                                          \
> __out:  __ret;                                                          \
> 
> Because the goto __out branch skips sdt_might_sleep_end(), does it leave a
> dangling stage context in the dept subsystem's tracking graph?
> 
> If the stage context remains populated, could a subsequent wait on the same
> task be incorrectly seen as a nested stage, potentially corrupting dept
> tracking or leading to false deadlock reports?

I will fix it.

> (Note: A similar goto __out pattern skipping sdt_might_sleep_end() also
> exists in ___wait_event() in include/linux/wait.h and ___swait_event() in
> include/linux/swait.h).

I will fix it.

	Byungchul

> 
> >       init_wait_var_entry(&__wbq_entry, var,                          \
> >                           exclusive ? WQ_FLAG_EXCLUSIVE : 0);         \
> >       for (;;) {                                                      \
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=19