[mm,1/4] lib/stackdepot: add printk_deferred_enter/exit guards

  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>
---
 lib/stackdepot.c | 9 +++++++++
 1 file changed, 9 insertions(+)
  

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!