| Message ID | 6c38c31e304a55449f76f60b6f72e35f992cad99.1702339432.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | lib/stackdepot, kasan: fixes for stack eviction series | expand |
On Tue, 12 Dec 2023 at 01:14, <andrey.konovalov@linux.dev> wrote: > > From: Andrey Konovalov <andreyknvl@google.com> > > Stack depot functions can be called from various contexts that do > allocations, including with console locks taken. At the same time, stack > depot functions might print WARNING's or refcount-related failures. > > This can cause a deadlock on console locks. > > Add printk_deferred_enter/exit guards to stack depot to avoid this. > > Reported-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> > Closes: https://lore.kernel.org/all/000000000000f56750060b9ad216@google.com/ > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> Reviewed-by: Marco Elver <elver@google.com> Doesn't need Fixes, because the series is not yet in mainline, right? > --- > lib/stackdepot.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 870cce2f4cbd..a0be5d05c7f0 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -506,12 +506,14 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, > bucket = &stack_table[hash & stack_hash_mask]; > > read_lock_irqsave(&pool_rwlock, flags); > + printk_deferred_enter(); > > /* Fast path: look the stack trace up without full locking. */ > found = find_stack(bucket, entries, nr_entries, hash); > if (found) { > if (depot_flags & STACK_DEPOT_FLAG_GET) > refcount_inc(&found->count); > + printk_deferred_exit(); > read_unlock_irqrestore(&pool_rwlock, flags); > goto exit; > } > @@ -520,6 +522,7 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, > if (new_pool_required) > need_alloc = true; > > + printk_deferred_exit(); > read_unlock_irqrestore(&pool_rwlock, flags); > > /* > @@ -541,6 +544,7 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, > } > > write_lock_irqsave(&pool_rwlock, flags); > + printk_deferred_enter(); > > found = find_stack(bucket, entries, nr_entries, hash); > if (!found) { > @@ -562,6 +566,7 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, > depot_keep_new_pool(&prealloc); > } > > + printk_deferred_exit(); > write_unlock_irqrestore(&pool_rwlock, flags); > exit: > if (prealloc) { > @@ -600,9 +605,11 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle, > return 0; > > read_lock_irqsave(&pool_rwlock, flags); > + printk_deferred_enter(); > > stack = depot_fetch_stack(handle); > > + printk_deferred_exit(); > read_unlock_irqrestore(&pool_rwlock, flags); > > *entries = stack->entries; > @@ -619,6 +626,7 @@ void stack_depot_put(depot_stack_handle_t handle) > return; > > write_lock_irqsave(&pool_rwlock, flags); > + printk_deferred_enter(); > > stack = depot_fetch_stack(handle); > if (WARN_ON(!stack)) > @@ -633,6 +641,7 @@ void stack_depot_put(depot_stack_handle_t handle) > } > > out: > + printk_deferred_exit(); > write_unlock_irqrestore(&pool_rwlock, flags); > } > EXPORT_SYMBOL_GPL(stack_depot_put); > -- > 2.25.1 >
On Tue, 12 Dec 2023 19:59:29 +0100 Marco Elver <elver@google.com> wrote: > On Tue, 12 Dec 2023 at 01:14, <andrey.konovalov@linux.dev> wrote: > > > > From: Andrey Konovalov <andreyknvl@google.com> > > > > Stack depot functions can be called from various contexts that do > > allocations, including with console locks taken. At the same time, stack > > depot functions might print WARNING's or refcount-related failures. > > > > This can cause a deadlock on console locks. > > > > Add printk_deferred_enter/exit guards to stack depot to avoid this. > > > > Reported-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> > > Closes: https://lore.kernel.org/all/000000000000f56750060b9ad216@google.com/ > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > Reviewed-by: Marco Elver <elver@google.com> > > Doesn't need Fixes, because the series is not yet in mainline, right? I've moved the series "stackdepot: allow evicting stack traces, v4" (please, not "the stack depot eviction series") into mm-nonmm-stable. Which is allegedly non-rebasing. So yes please, provide Fixes: on each patch.
On Tue, Dec 12, 2023 at 9:57 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Tue, 12 Dec 2023 19:59:29 +0100 Marco Elver <elver@google.com> wrote: > > > On Tue, 12 Dec 2023 at 01:14, <andrey.konovalov@linux.dev> wrote: > > > > > > From: Andrey Konovalov <andreyknvl@google.com> > > > > > > Stack depot functions can be called from various contexts that do > > > allocations, including with console locks taken. At the same time, stack > > > depot functions might print WARNING's or refcount-related failures. > > > > > > This can cause a deadlock on console locks. > > > > > > Add printk_deferred_enter/exit guards to stack depot to avoid this. > > > > > > Reported-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> > > > Closes: https://lore.kernel.org/all/000000000000f56750060b9ad216@google.com/ > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > > > Reviewed-by: Marco Elver <elver@google.com> > > > > Doesn't need Fixes, because the series is not yet in mainline, right? > > I've moved the series "stackdepot: allow evicting stack traces, v4" > (please, not "the stack depot eviction series") into mm-nonmm-stable. > Which is allegedly non-rebasing. > > So yes please, provide Fixes: on each patch. Sure, I'll add them when I mail v2 after we figure out what to do with patch #2. Thanks!
diff --git a/lib/stackdepot.c b/lib/stackdepot.c index 870cce2f4cbd..a0be5d05c7f0 100644 --- a/lib/stackdepot.c +++ b/lib/stackdepot.c @@ -506,12 +506,14 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, bucket = &stack_table[hash & stack_hash_mask]; read_lock_irqsave(&pool_rwlock, flags); + printk_deferred_enter(); /* Fast path: look the stack trace up without full locking. */ found = find_stack(bucket, entries, nr_entries, hash); if (found) { if (depot_flags & STACK_DEPOT_FLAG_GET) refcount_inc(&found->count); + printk_deferred_exit(); read_unlock_irqrestore(&pool_rwlock, flags); goto exit; } @@ -520,6 +522,7 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, if (new_pool_required) need_alloc = true; + printk_deferred_exit(); read_unlock_irqrestore(&pool_rwlock, flags); /* @@ -541,6 +544,7 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, } write_lock_irqsave(&pool_rwlock, flags); + printk_deferred_enter(); found = find_stack(bucket, entries, nr_entries, hash); if (!found) { @@ -562,6 +566,7 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, depot_keep_new_pool(&prealloc); } + printk_deferred_exit(); write_unlock_irqrestore(&pool_rwlock, flags); exit: if (prealloc) { @@ -600,9 +605,11 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle, return 0; read_lock_irqsave(&pool_rwlock, flags); + printk_deferred_enter(); stack = depot_fetch_stack(handle); + printk_deferred_exit(); read_unlock_irqrestore(&pool_rwlock, flags); *entries = stack->entries; @@ -619,6 +626,7 @@ void stack_depot_put(depot_stack_handle_t handle) return; write_lock_irqsave(&pool_rwlock, flags); + printk_deferred_enter(); stack = depot_fetch_stack(handle); if (WARN_ON(!stack)) @@ -633,6 +641,7 @@ void stack_depot_put(depot_stack_handle_t handle) } out: + printk_deferred_exit(); write_unlock_irqrestore(&pool_rwlock, flags); } EXPORT_SYMBOL_GPL(stack_depot_put);