linux-kernel - Re: [PATCH] lockdep: Allow tuning tracing keys constant.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+Jga0JauDYSwwL0dTTVkUyXrWQo=H2LAPwgF7yprNKJvZY5rA@mail.gmail.com>
Date:   Thu, 27 Oct 2022 13:18:41 -0700
From:   Ryan Huang <tzukui@...gle.com>
To:     Waiman Long <longman@...hat.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Sasha Levin <sashal@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] lockdep: Allow tuning tracing keys constant.

Sorry for the late reply.

Our use case is we will create workqueue for each ptr in one device abc
(Rename to abc here without leaking any information)

  ptr->wq = alloc_workqueue("abc_x_%d_y_%d_ptr_%d", WQ_HIGHPRI, 1, x, y, ptr);

where x is 0~7, y is 0~127, ptr is 0~4, and we have a maximum of 8 devices.
So there is a maximum 8x128x5x8=40960 workqueue.
I found the lockdep key is filled with

  register_lock_class: nr_lock_classes=2498, name=_rs.lock
  register_lock_class: nr_lock_classes=2499, name=semaphore->lock
  register_lock_class: nr_lock_classes=2500,
name=(wq_completion)abc_x_0_y_0_ptr_1
  register_lock_class: nr_lock_classes=2501,
name=(work_completion)(&wp->write_work)
  register_lock_class: nr_lock_classes=2502,
name=(wq_completion)abc_x_0_y_0_ptr_2
  register_lock_class: nr_lock_classes=2503,
name=(wq_completion)abc_x_0_y_0_ptr_3
  ...
  register_lock_class: nr_lock_classes=8189,
name=(wq_completion)abc_x_0_y_110_ptr_2
  register_lock_class: nr_lock_classes=8190,
name=(wq_completion)abc_x_0_y_110_ptr_3
  register_lock_class: nr_lock_classes=8191,
name=(wq_completion)abc_x_0_y_110_ptr_4
  register_lock_class: nr_lock_classes=8192,
name=(wq_completion)abc_x_0_y_110_ptr_5

It seems when we queue_work on one workqueue, it will occupy one lock
class. That's why 8k is not enough for our use case.

Regarding my patch, I just move the hard code value into config.
Increasing the value just increase one byte for the held_lock structure,
and 48 bytes for task_struct structure (around 6 u64 variables) in DEBUG
mode.

Let me know if you have a better way!

Thanks
Ryan


On Fri, Sep 30, 2022 at 10:34 AM Waiman Long <longman@...hat.com> wrote:
>
>
> On 9/30/22 13:00, Ryan Huang wrote:
> > Tetsuo Handa made a change for tuning lockdep tracing capacity constants
> > [1]. He created following tracing config constants:
> >    - LOCKDEP_BITS
> >    - LOCKDEP_CHAINS_BITS
> >    - LOCKDEP_STACK_TRACE_BITS
> > However there is a missing one for LOCKDEP_KEYS_BITS. We can see this is
> > also hitting the upper limits in [2].
> >
> > [1] https://github.com/torvalds/linux/commit/5dc33592e95534dc8455ce3e9baaaf3dae0fff82
> > [2] https://syzkaller.appspot.com/bug?id=df466e1151a7e45dd880d8c7033f1ad48acebfb4
> >
> > Fixes: 5dc33592e955 ("lockdep: Allow tuning tracing capacity constants.")
> > Signed-off-by: Ryan Huang <tzukui@...gle.com>
> > ---
> >   include/linux/lockdep.h | 2 +-
> >   lib/Kconfig.debug       | 8 ++++++++
> >   2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> > index 1f1099dac3f05..3bb1740296559 100644
> > --- a/include/linux/lockdep.h
> > +++ b/include/linux/lockdep.h
> > @@ -82,7 +82,7 @@ struct lock_chain {
> >       u64                             chain_key;
> >   };
> >
> > -#define MAX_LOCKDEP_KEYS_BITS                13
> > +#define MAX_LOCKDEP_KEYS_BITS                CONFIG_LOCKDEP_KEYS_BITS
> >   #define MAX_LOCKDEP_KEYS            (1UL << MAX_LOCKDEP_KEYS_BITS)
> >   #define INITIAL_CHAIN_KEY           -1
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index d3e5f36bb01e0..d15024bd14f1d 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -1398,6 +1398,14 @@ config LOCKDEP_CHAINS_BITS
> >       help
> >         Try increasing this value if you hit "BUG: MAX_LOCKDEP_CHAINS too low!" message.
> >
> > +config LOCKDEP_KEYS_BITS
> > +     int "Bitsize for MAX_LOCKDEP_KEYS"
> > +     depends on LOCKDEP && !LOCKDEP_SMALL
> > +     range 10 30
> > +     default 13
> > +     help
> > +       Try increasing this value if you hit "BUG: MAX_LOCKDEP_KEYS too low!" message.
> > +
> >   config LOCKDEP_STACK_TRACE_BITS
> >       int "Bitsize for MAX_STACK_TRACE_ENTRIES"
> >       depends on LOCKDEP && !LOCKDEP_SMALL
>
> The lockdep key is embedded in a bit field within the held_lock
> structure to utilize all the 32 bits of an integer. Changing its size
> will require adjusting other bit fields or expand the bit field size
> from 32 bits to 64 bits. 13 bits allows up to 8k unique lock classes.
> Unless there is good evidence that we are going to run out of that, we
> shouldn't change it.
>
> Thanks,
> Longman
>