Messages in this thread | | | From | Ryan Huang <> | Date | Thu, 27 Oct 2022 13:18:41 -0700 | 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@redhat.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@google.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 >
| |