[PATCH 0/3] lockdep: minor config and documentation fixes

[PATCH 0/3] lockdep: minor config and documentation fixes

[Carlos Llamas]

These are some minor follow-up patches that came up during conversation
at: https://lore.kernel.org/all/30795.1620913191@jrobl/

Note this patchset is sent on top of akpm mm-nonmm-unstable as it
depends on "[PATCH v2] lockdep: upper limit LOCKDEP_CHAINS_BITS" which
as been previously applied there.

Cc: Huang Ying <ying.huang@intel.com>
Cc: J. R. Okajima <hooanon05g@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>

v1: https://lore.kernel.org/all/20240806010128.402852-1-cmllamas@google.com/

Carlos Llamas (3):
  lockdep: fix upper limit for LOCKDEP_*_BITS configs
  lockdep: clarify size for LOCKDEP_*_BITS configs
  lockdep: document MAX_LOCKDEP_CHAIN_HLOCKS calculation

 kernel/locking/lockdep_internals.h |  3 ++-
 lib/Kconfig.debug                  | 18 +++++++++---------
 2 files changed, 11 insertions(+), 10 deletions(-)


base-commit: 766e1913ce7c3add51f49bc1861441e3a3275df2
-- 
2.46.0.rc2.264.g509ed76dc8-goog

[PATCH v2 1/3] lockdep: fix upper limit for LOCKDEP_*_BITS configs

[Carlos Llamas]

Lockdep has a set of configs used to determine the size of the static
arrays that it uses. However, the upper limit that was initially setup
for these configs is too high (30 bit shift). This equates to several
GiB of static memory for individual symbols. Using such high values
leads to linker errors:

  $ make defconfig
  $ ./scripts/config -e PROVE_LOCKING --set-val LOCKDEP_BITS 30
  $ make olddefconfig all
  [...]
  ld: kernel image bigger than KERNEL_IMAGE_SIZE
  ld: section .bss VMA wraps around address space

Adjust the upper limits to the maximum values that avoid these issues.
The need for anything more, likely points to a problem elsewhere. Note
that LOCKDEP_CHAINS_BITS was intentionally left out as its upper limit
had a different symptom and has already been fixed [1].

Reported-by: J. R. Okajima <hooanon05g@gmail.com>
Closes: https://lore.kernel.org/all/30795.1620913191@jrobl/ [1]
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will@kernel.org>
Acked-by: Waiman Long <longman@redhat.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 lib/Kconfig.debug | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index a81d452941ce..baaaedfde0cb 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1507,7 +1507,7 @@ config LOCKDEP_SMALL
 config LOCKDEP_BITS
 	int "Bitsize for MAX_LOCKDEP_ENTRIES"
 	depends on LOCKDEP && !LOCKDEP_SMALL
-	range 10 30
+	range 10 24
 	default 15
 	help
 	  Try increasing this value if you hit "BUG: MAX_LOCKDEP_ENTRIES too low!" message.
@@ -1523,7 +1523,7 @@ config LOCKDEP_CHAINS_BITS
 config LOCKDEP_STACK_TRACE_BITS
 	int "Bitsize for MAX_STACK_TRACE_ENTRIES"
 	depends on LOCKDEP && !LOCKDEP_SMALL
-	range 10 30
+	range 10 26
 	default 19
 	help
 	  Try increasing this value if you hit "BUG: MAX_STACK_TRACE_ENTRIES too low!" message.
@@ -1531,7 +1531,7 @@ config LOCKDEP_STACK_TRACE_BITS
 config LOCKDEP_STACK_TRACE_HASH_BITS
 	int "Bitsize for STACK_TRACE_HASH_SIZE"
 	depends on LOCKDEP && !LOCKDEP_SMALL
-	range 10 30
+	range 10 26
 	default 14
 	help
 	  Try increasing this value if you need large STACK_TRACE_HASH_SIZE.
@@ -1539,7 +1539,7 @@ config LOCKDEP_STACK_TRACE_HASH_BITS
 config LOCKDEP_CIRCULAR_QUEUE_BITS
 	int "Bitsize for elements in circular_queue struct"
 	depends on LOCKDEP
-	range 10 30
+	range 10 26
 	default 12
 	help
 	  Try increasing this value if you hit "lockdep bfs error:-1" warning due to __cq_enqueue() failure.
-- 
2.46.0.rc2.264.g509ed76dc8-goog

Re: [PATCH v2 1/3] lockdep: fix upper limit for LOCKDEP_*_BITS configs

[J. R. Okajima]

Carlos Llamas:
> Adjust the upper limits to the maximum values that avoid these issues.
> The need for anything more, likely points to a problem elsewhere. Note
> that LOCKDEP_CHAINS_BITS was intentionally left out as its upper limit
> had a different symptom and has already been fixed [1].

I tried setting all these configs to maximum, but still I got the error.
	ld: kernel image bigger than KERNEL_IMAGE_SIZE

For me, these are the maximum.
They are compilable, but could not boot due to "out of memory".
Also I am not sure whether these values are meaningful.

CONFIG_LOCKDEP_BITS=23
(CONFIG_LOCKDEP_CHAINS_BITS=21)
CONFIG_LOCKDEP_STACK_TRACE_BITS=25
CONFIG_LOCKDEP_STACK_TRACE_HASH_BITS=24
CONFIG_LOCKDEP_CIRCULAR_QUEUE_BITS=22


J. R. Okajima

Re: [PATCH v2 1/3] lockdep: fix upper limit for LOCKDEP_*_BITS configs

[Carlos Llamas]

On Thu, Aug 08, 2024 at 10:20:58AM +0900, J. R. Okajima wrote:
> Carlos Llamas:
> > Adjust the upper limits to the maximum values that avoid these issues.
> > The need for anything more, likely points to a problem elsewhere. Note
> > that LOCKDEP_CHAINS_BITS was intentionally left out as its upper limit
> > had a different symptom and has already been fixed [1].
> 
> I tried setting all these configs to maximum, but still I got the error.
> 	ld: kernel image bigger than KERNEL_IMAGE_SIZE
> 
> For me, these are the maximum.
> They are compilable, but could not boot due to "out of memory".
> Also I am not sure whether these values are meaningful.
> 
> CONFIG_LOCKDEP_BITS=23
> (CONFIG_LOCKDEP_CHAINS_BITS=21)
> CONFIG_LOCKDEP_STACK_TRACE_BITS=25
> CONFIG_LOCKDEP_STACK_TRACE_HASH_BITS=24
> CONFIG_LOCKDEP_CIRCULAR_QUEUE_BITS=22

Yeah, I say that's expected if you bump these values to the max all at
once. The values I gave were tested individually on top of the defconfig
and boot completed fine (qemu x86_64 and aarch64 with -m 8G). I think
it's fair to leave room to configure these knobs individually.

--
Carlos Llamas

> 
> J. R. Okajima

Re: [PATCH v2 1/3] lockdep: fix upper limit for LOCKDEP_*_BITS configs

[hooanon05g]

Carlos Llamas:
> Yeah, I say that's expected if you bump these values to the max all at
> once. The values I gave were tested individually on top of the defconfig
> and boot completed fine (qemu x86_64 and aarch64 with -m 8G). I think
> it's fair to leave room to configure these knobs individually.

I see, understood.
Now you can freely add Acked-by from me.


J. R. Okajima

[PATCH v2 2/3] lockdep: clarify size for LOCKDEP_*_BITS configs

[Carlos Llamas]

The LOCKDEP_*_BITS configs control the size of internal structures used
by lockdep. The size is calculated as a power of two of the configured
value (e.g. 16 => 64KB). Update these descriptions to more accurately
reflect this, as "Bitsize" can be misleading.

Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 lib/Kconfig.debug | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index baaaedfde0cb..e0614a415348 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1505,7 +1505,7 @@ config LOCKDEP_SMALL
 	bool
 
 config LOCKDEP_BITS
-	int "Bitsize for MAX_LOCKDEP_ENTRIES"
+	int "Size for MAX_LOCKDEP_ENTRIES (as Nth power of 2)"
 	depends on LOCKDEP && !LOCKDEP_SMALL
 	range 10 24
 	default 15
@@ -1513,7 +1513,7 @@ config LOCKDEP_BITS
 	  Try increasing this value if you hit "BUG: MAX_LOCKDEP_ENTRIES too low!" message.
 
 config LOCKDEP_CHAINS_BITS
-	int "Bitsize for MAX_LOCKDEP_CHAINS"
+	int "Size for MAX_LOCKDEP_CHAINS (as Nth power of 2)"
 	depends on LOCKDEP && !LOCKDEP_SMALL
 	range 10 21
 	default 16
@@ -1521,7 +1521,7 @@ config LOCKDEP_CHAINS_BITS
 	  Try increasing this value if you hit "BUG: MAX_LOCKDEP_CHAINS too low!" message.
 
 config LOCKDEP_STACK_TRACE_BITS
-	int "Bitsize for MAX_STACK_TRACE_ENTRIES"
+	int "Size for MAX_STACK_TRACE_ENTRIES (as Nth power of 2)"
 	depends on LOCKDEP && !LOCKDEP_SMALL
 	range 10 26
 	default 19
@@ -1529,7 +1529,7 @@ config LOCKDEP_STACK_TRACE_BITS
 	  Try increasing this value if you hit "BUG: MAX_STACK_TRACE_ENTRIES too low!" message.
 
 config LOCKDEP_STACK_TRACE_HASH_BITS
-	int "Bitsize for STACK_TRACE_HASH_SIZE"
+	int "Size for STACK_TRACE_HASH_SIZE (as Nth power of 2)"
 	depends on LOCKDEP && !LOCKDEP_SMALL
 	range 10 26
 	default 14
@@ -1537,7 +1537,7 @@ config LOCKDEP_STACK_TRACE_HASH_BITS
 	  Try increasing this value if you need large STACK_TRACE_HASH_SIZE.
 
 config LOCKDEP_CIRCULAR_QUEUE_BITS
-	int "Bitsize for elements in circular_queue struct"
+	int "Size for elements in circular_queue struct (as Nth power of 2)"
 	depends on LOCKDEP
 	range 10 26
 	default 12
-- 
2.46.0.rc2.264.g509ed76dc8-goog

[PATCH v2 3/3] lockdep: document MAX_LOCKDEP_CHAIN_HLOCKS calculation

[Carlos Llamas]

Define a macro AVG_LOCKDEP_CHAIN_DEPTH to document the magic number '5'
used in the calculation of MAX_LOCKDEP_CHAIN_HLOCKS. The number
represents the estimated average depth (number of locks held) of a lock
chain. The calculation of MAX_LOCKDEP_CHAIN_HLOCKS was first added in
commit 443cd507ce7f ("lockdep: add lock_class information to lock_chain
and output it").

Suggested-by: Waiman Long <longman@redhat.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: J. R. Okajima <hooanon05g@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
v2: switched the comment for a macro as suggested by Waiman Long.

 kernel/locking/lockdep_internals.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index bbe9000260d0..20f9ef58d3d0 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -119,7 +119,8 @@ static const unsigned long LOCKF_USED_IN_IRQ_READ =
 
 #define MAX_LOCKDEP_CHAINS	(1UL << MAX_LOCKDEP_CHAINS_BITS)
 
-#define MAX_LOCKDEP_CHAIN_HLOCKS (MAX_LOCKDEP_CHAINS*5)
+#define AVG_LOCKDEP_CHAIN_DEPTH		5
+#define MAX_LOCKDEP_CHAIN_HLOCKS (MAX_LOCKDEP_CHAINS * AVG_LOCKDEP_CHAIN_DEPTH)
 
 extern struct lock_chain lock_chains[];
 
-- 
2.46.0.rc2.264.g509ed76dc8-goog

Re: [PATCH v2 3/3] lockdep: document MAX_LOCKDEP_CHAIN_HLOCKS calculation

[Waiman Long]

On 8/7/24 10:39, Carlos Llamas wrote:
> Define a macro AVG_LOCKDEP_CHAIN_DEPTH to document the magic number '5'
> used in the calculation of MAX_LOCKDEP_CHAIN_HLOCKS. The number
> represents the estimated average depth (number of locks held) of a lock
> chain. The calculation of MAX_LOCKDEP_CHAIN_HLOCKS was first added in
> commit 443cd507ce7f ("lockdep: add lock_class information to lock_chain
> and output it").
>
> Suggested-by: Waiman Long <longman@redhat.com>
> Cc: Huang Ying <ying.huang@intel.com>
> Cc: J. R. Okajima <hooanon05g@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> ---
> v2: switched the comment for a macro as suggested by Waiman Long.
>
>   kernel/locking/lockdep_internals.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
> index bbe9000260d0..20f9ef58d3d0 100644
> --- a/kernel/locking/lockdep_internals.h
> +++ b/kernel/locking/lockdep_internals.h
> @@ -119,7 +119,8 @@ static const unsigned long LOCKF_USED_IN_IRQ_READ =
>   
>   #define MAX_LOCKDEP_CHAINS	(1UL << MAX_LOCKDEP_CHAINS_BITS)
>   
> -#define MAX_LOCKDEP_CHAIN_HLOCKS (MAX_LOCKDEP_CHAINS*5)
> +#define AVG_LOCKDEP_CHAIN_DEPTH		5
> +#define MAX_LOCKDEP_CHAIN_HLOCKS (MAX_LOCKDEP_CHAINS * AVG_LOCKDEP_CHAIN_DEPTH)
>   
>   extern struct lock_chain lock_chains[];
>   
Acked-by: Waiman Long <longman@redhat.com>