The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul@sk.com>
To: sashiko-reviews@lists.linux.dev
Cc: Chuck Lever <cel@kernel.org>,
	tytso@mit.edu, Anna Schumaker <anna@kernel.org>,
	linux-kernel@vger.kernel.org, dlemoal@kernel.org,
	dri-devel@lists.freedesktop.org, Jeff Layton <jlayton@kernel.org>,
	linux-rt-devel@lists.linux.dev, ojeda@kernel.org,
	linux-modules@vger.kernel.org, cassel@kernel.org,
	linux-ide@vger.kernel.org, kernel_team@skhynix.com
Subject: Re: [PATCH v19 13/40] dept: apply sdt_might_sleep_{start, end}() to hashed-waitqueue wait
Date: Fri, 10 Jul 2026 15:42:52 +0900	[thread overview]
Message-ID: <20260710064252.GI37630@system.software.com> (raw)
In-Reply-To: <20260707073317.37C831F000E9@smtp.kernel.org>

On Tue, Jul 07, 2026 at 07:33:16AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The `sdt_might_sleep_end()` macro is bypassed when `___wait_var_event` is interrupted, leaving the DEPT tracker in an inconsistent state.
> - [Medium] The `sdt_might_sleep_end()` macro is bypassed when `___wait_event` is interrupted, leaving the DEPT tracker in an inconsistent state.
> --
> 
> commit 0bd2d42866fa6b852ee5317edcaf6890a90c3c65
> Author: Byungchul Park <byungchul@sk.com>
> 
> dept: apply sdt_might_sleep_{start,end}() to hashed-waitqueue wait
> 
> Make dept able to track dependencies by hashed-waitqueue waits.
> 
> > diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h
> > index 9e29d79fc790a..179a616ad245a 100644
> > --- a/include/linux/wait_bit.h
> > +++ b/include/linux/wait_bit.h
> [ ... ]
> > @@ -257,6 +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);                                    \
> >       init_wait_var_entry(&__wbq_entry, var,                          \
> >                           exclusive ? WQ_FLAG_EXCLUSIVE : 0);         \
> >       for (;;) {                                                      \
> > @@ -274,6 +276,7 @@ extern wait_queue_head_t *__var_waitqueue(void *p);
> >               cmd;                                                    \
> >       }                                                               \
> >       finish_wait(__wq_head, &__wbq_entry.wq_entry);                  \
> > +     sdt_might_sleep_end();                                          \
> >  __out:       __ret;                                                          \
> >  })
> 
> [Severity: Medium]
> If this wait is interrupted by a signal, the loop executes a goto __out
> which bypasses sdt_might_sleep_end(). Does this leave the dept tracker
> in an inconsistent state where the task's stage remains permanently armed?
> 
> This could cause false positive dependency tracking splats on subsequent
> waits, or fail to record new dependencies because the tracker is blinded by
> the stale stage.
> 
> I noticed the exact same issue is present in ___wait_event():
> 
> include/linux/wait.h:___wait_event() {
>     ...
>                 if (___wait_is_interruptible(state) && __int) {
>                         __ret = __int;
>                         goto __out;
>                 }
>     ...
>         sdt_might_sleep_end();
> __out:  __ret;
> })

I will fix it.

> [Severity: Medium]
> Does ___wait_event() also skip sdt_might_sleep_end() when interrupted?
> By missing the paired cleanup call, the task's stage could remain
> permanently armed here as well.
> 
> Would it make sense to move the __out label above sdt_might_sleep_end(),
> or otherwise ensure the state is cleaned up on the interrupted paths for
> both of these macros?

Right.  I will fix it.

	Byungchul

> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=13

  reply	other threads:[~2026-07-10  6:42 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06  6:18 [PATCH v19 00/40] DEPT(DEPendency Tracker) Byungchul Park
2026-07-06  6:18 ` [PATCH v19 01/40] dept: implement " Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:18 ` [PATCH v19 02/40] dept: add single event dependency tracker APIs Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:18 ` [PATCH v19 03/40] dept: add lock " Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:18 ` [PATCH v19 04/40] dept: tie to lockdep and IRQ tracing Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-10  6:54     ` Byungchul Park
2026-07-06  6:18 ` [PATCH v19 05/40] dept: add proc knobs to show stats and dependency graph Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:18 ` [PATCH v19 06/40] dept: distinguish each kernel context from another Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:18 ` [PATCH v19 07/40] dept: distinguish each work " Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-10  6:40     ` Byungchul Park
2026-07-06  6:18 ` [PATCH v19 08/40] dept: add a mechanism to refill the internal memory pools on running out Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:18 ` [PATCH v19 09/40] dept: record the latest one out of consecutive waits of the same class Byungchul Park
2026-07-06  6:18 ` [PATCH v19 10/40] dept: apply sdt_might_sleep_{start,end}() to wait_for_completion()/complete() Byungchul Park
2026-07-07  7:33   ` [PATCH v19 10/40] dept: apply sdt_might_sleep_{start, end}() " sashiko-bot
2026-07-06  6:18 ` [PATCH v19 11/40] dept: apply sdt_might_sleep_{start,end}() to swait Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 12/40] dept: apply sdt_might_sleep_{start,end}() to waitqueue wait Byungchul Park
2026-07-07  7:33   ` [PATCH v19 12/40] dept: apply sdt_might_sleep_{start, end}() " sashiko-bot
2026-07-06  6:19 ` [PATCH v19 13/40] dept: apply sdt_might_sleep_{start,end}() to hashed-waitqueue wait Byungchul Park
2026-07-07  7:33   ` [PATCH v19 13/40] dept: apply sdt_might_sleep_{start, end}() " sashiko-bot
2026-07-10  6:42     ` Byungchul Park [this message]
2026-07-06  6:19 ` [PATCH v19 14/40] dept: apply sdt_might_sleep_{start,end}() to dma fence Byungchul Park
2026-07-06  6:19 ` [PATCH v19 15/40] dept: track timeout waits separately with a new Kconfig Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 16/40] dept: apply timeout consideration to wait_for_completion()/complete() Byungchul Park
2026-07-06  6:19 ` [PATCH v19 17/40] dept: apply timeout consideration to swait Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 18/40] dept: apply timeout consideration to waitqueue wait Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 19/40] dept: apply timeout consideration to hashed-waitqueue wait Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-10  6:41     ` Byungchul Park
2026-07-06  6:19 ` [PATCH v19 20/40] dept: apply timeout consideration to dma fence wait Byungchul Park
2026-07-06  6:19 ` [PATCH v19 21/40] dept: make dept able to work with an external wgen Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 22/40] dept: track PG_locked with dept Byungchul Park
2026-07-06 18:05   ` Matthew Wilcox
2026-07-07  2:35     ` Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 23/40] dept: print staged wait's stacktrace on report Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 24/40] locking/lockdep: prevent various lockdep assertions when lockdep_off()'ed Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 25/40] dept: add documents for dept Byungchul Park
2026-07-06  6:19 ` [PATCH v19 26/40] cpu/hotplug: use a weaker annotation in AP thread Byungchul Park
2026-07-06  6:19 ` [PATCH v19 27/40] dept: assign dept map to mmu notifier invalidation synchronization Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 28/40] dept: assign unique dept_key to each distinct dma fence caller Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-10  6:24     ` Byungchul Park
2026-07-06  6:19 ` [PATCH v19 29/40] dept: make dept aware of lockdep_set_lock_cmp_fn() annotation Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-10  6:04     ` Byungchul Park
2026-07-06  6:19 ` [PATCH v19 30/40] dept: make dept stop from working on debug_locks_off() Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 31/40] dept: assign unique dept_key to each distinct wait_for_completion() caller Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-07 14:18   ` Gary Guo
2026-07-10  5:53     ` Byungchul Park
2026-07-06  6:19 ` [PATCH v19 32/40] completion, dept: introduce init_completion_dmap() API Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-10  6:12     ` Byungchul Park
2026-07-06  6:19 ` [PATCH v19 33/40] dept: call dept_hardirqs_off() in local_irq_*() regardless of irq state Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 34/40] rcu/update: fix same dept key collision between various types of RCU Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-10  6:20     ` Byungchul Park
2026-07-06  6:19 ` [PATCH v19 35/40] dept: introduce APIs to set page usage and use subclasses_evt for the usage Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 36/40] dept: track PG_writeback with dept Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-06  6:19 ` [PATCH v19 37/40] SUNRPC: relocate struct rcu_head to the first field of struct rpc_xprt Byungchul Park
2026-07-06  6:19 ` [PATCH v19 38/40] mm: percpu: increase PERCPU_DYNAMIC_SIZE_SHIFT on DEPT and large PAGE_SIZE Byungchul Park
2026-07-07  7:33   ` sashiko-bot
2026-07-10  5:55     ` Byungchul Park
2026-07-06  6:19 ` [PATCH v19 39/40] rust: completion: Add __rust_helper to rust_helper_wait_for_completion() Byungchul Park
2026-07-06  6:19 ` [PATCH v19 40/40] dept: implement a basic unit test for dept Byungchul Park
2026-07-07  7:33   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260710064252.GI37630@system.software.com \
    --to=byungchul@sk.com \
    --cc=anna@kernel.org \
    --cc=cassel@kernel.org \
    --cc=cel@kernel.org \
    --cc=dlemoal@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jlayton@kernel.org \
    --cc=kernel_team@skhynix.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox