[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>

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 |  1 +
 lib/Kconfig.debug                  | 18 +++++++++---------
 2 files changed, 10 insertions(+), 9 deletions(-)


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

[PATCH 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>
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

[PATCH 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 3/3] lockdep: document MAX_LOCKDEP_CHAIN_HLOCKS calculation

[Carlos Llamas]

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

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>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 kernel/locking/lockdep_internals.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index bbe9000260d0..2b429ed103a8 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -119,6 +119,7 @@ static const unsigned long LOCKF_USED_IN_IRQ_READ =
 
 #define MAX_LOCKDEP_CHAINS	(1UL << MAX_LOCKDEP_CHAINS_BITS)
 
+/* We estimate that a chain holds 5 locks on average. */
 #define MAX_LOCKDEP_CHAIN_HLOCKS (MAX_LOCKDEP_CHAINS*5)
 
 extern struct lock_chain lock_chains[];
-- 
2.46.0.rc2.264.g509ed76dc8-goog

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

[Waiman Long]

On 8/5/24 21:01, Carlos Llamas wrote:
> Add a comment to document the magic number '5' used in the calculation
> of MAX_LOCKDEP_CHAIN_HLOCKS. This number represents the estimated
> average depth (number of locks held) of a lock chain. This definition
> was added in commit 443cd507ce7f ("lockdep: add lock_class information
> to lock_chain and output it").
>
> 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>
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> ---
>   kernel/locking/lockdep_internals.h | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
> index bbe9000260d0..2b429ed103a8 100644
> --- a/kernel/locking/lockdep_internals.h
> +++ b/kernel/locking/lockdep_internals.h
> @@ -119,6 +119,7 @@ static const unsigned long LOCKF_USED_IN_IRQ_READ =
>   
>   #define MAX_LOCKDEP_CHAINS	(1UL << MAX_LOCKDEP_CHAINS_BITS)
>   
> +/* We estimate that a chain holds 5 locks on average. */
>   #define MAX_LOCKDEP_CHAIN_HLOCKS (MAX_LOCKDEP_CHAINS*5)
>   
>   extern struct lock_chain lock_chains[];

I think it is better to define another macro like

diff --git a/kernel/locking/lockdep_internals.h 
b/kernel/locking/lockdep_internals.h
index bbe9000260d0..8805122cc9f1 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[];

Cheers,
Longman

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

[Waiman Long]

On 8/5/24 21:01, Carlos Llamas wrote:
> 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>
> 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.

I agree that we should further limit the max values.

Acked-by: Waiman Long <longman@redhat.com>

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

[Waiman Long]

On 8/5/24 21:01, Carlos Llamas wrote:
> 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

Many kernel developers understand that BITS refers to a size of 2^n. 
Besides LOCKDEP, there are also many instances of such use in other 
kconfig entries. It can be a bit odd to explicitly state that just for 
LOCKDEP.

Cheers,
Longman

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

[Carlos Llamas]

On Mon, Aug 05, 2024 at 09:36:43PM -0400, Waiman Long wrote:
> 
> Many kernel developers understand that BITS refers to a size of 2^n. Besides
> LOCKDEP, there are also many instances of such use in other kconfig entries.
> It can be a bit odd to explicitly state that just for LOCKDEP.
> 
> Cheers,
> Longman

Right, and similar to BITS there is SHIFT, which is also a common way to
specify the 2^n values. I'd point out though, that it is also common to
clarify the "power of two" explicitly. To name a few examples that are
doing so: SECURITY_SELINUX_SIDTAB_HASH_BITS, NODES_SHIFT, CMA_ALIGNMENT,
IP_VS_SH_TAB_BITS, LOG_BUF_SHIFT but there is more.

Perhaps this is because the audience for these configs is not always a
kernel developer?

Anyway, this is pretty much a trivial patch to address Andrew's comment
below. But let me know if you think I should drop it, it seems to me it
can be helpful.

  [...]
  btw, the help text "Bitsize for MAX_LOCKDEP_CHAINS" is odd.  What's a
  bitsize?  Maybe "bit shift count for..." or such.

Thanks,
--
Carlos Llamas

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

[Carlos Llamas]

On Mon, Aug 05, 2024 at 09:27:52PM -0400, Waiman Long wrote:
> 
> I think it is better to define another macro like
> 
> diff --git a/kernel/locking/lockdep_internals.h
> b/kernel/locking/lockdep_internals.h
> index bbe9000260d0..8805122cc9f1 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[];
> 
> Cheers,
> Longman

Sounds good, I'll add your suggestion for v2. Thanks!

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

[Waiman Long]

On 8/6/24 10:47, Carlos Llamas wrote:
> On Mon, Aug 05, 2024 at 09:36:43PM -0400, Waiman Long wrote:
>> Many kernel developers understand that BITS refers to a size of 2^n. Besides
>> LOCKDEP, there are also many instances of such use in other kconfig entries.
>> It can be a bit odd to explicitly state that just for LOCKDEP.
>>
>> Cheers,
>> Longman
> Right, and similar to BITS there is SHIFT, which is also a common way to
> specify the 2^n values. I'd point out though, that it is also common to
> clarify the "power of two" explicitly. To name a few examples that are
> doing so: SECURITY_SELINUX_SIDTAB_HASH_BITS, NODES_SHIFT, CMA_ALIGNMENT,
> IP_VS_SH_TAB_BITS, LOG_BUF_SHIFT but there is more.
>
> Perhaps this is because the audience for these configs is not always a
> kernel developer?
>
> Anyway, this is pretty much a trivial patch to address Andrew's comment
> below. But let me know if you think I should drop it, it seems to me it
> can be helpful.
>
>    [...]
>    btw, the help text "Bitsize for MAX_LOCKDEP_CHAINS" is odd.  What's a
>    bitsize?  Maybe "bit shift count for..." or such.

I am not against this patch. Currently I am neutral. Let's see what 
Boqun think about it.

Cheers,
Longman

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

[Boqun Feng]

On Tue, Aug 06, 2024 at 10:52:52AM -0400, Waiman Long wrote:
> 
> On 8/6/24 10:47, Carlos Llamas wrote:
> > On Mon, Aug 05, 2024 at 09:36:43PM -0400, Waiman Long wrote:
> > > Many kernel developers understand that BITS refers to a size of 2^n. Besides
> > > LOCKDEP, there are also many instances of such use in other kconfig entries.
> > > It can be a bit odd to explicitly state that just for LOCKDEP.
> > > 
> > > Cheers,
> > > Longman
> > Right, and similar to BITS there is SHIFT, which is also a common way to
> > specify the 2^n values. I'd point out though, that it is also common to
> > clarify the "power of two" explicitly. To name a few examples that are
> > doing so: SECURITY_SELINUX_SIDTAB_HASH_BITS, NODES_SHIFT, CMA_ALIGNMENT,
> > IP_VS_SH_TAB_BITS, LOG_BUF_SHIFT but there is more.
> > 
> > Perhaps this is because the audience for these configs is not always a
> > kernel developer?
> > 
> > Anyway, this is pretty much a trivial patch to address Andrew's comment
> > below. But let me know if you think I should drop it, it seems to me it
> > can be helpful.
> > 
> >    [...]
> >    btw, the help text "Bitsize for MAX_LOCKDEP_CHAINS" is odd.  What's a
> >    bitsize?  Maybe "bit shift count for..." or such.
> 
> I am not against this patch. Currently I am neutral. Let's see what Boqun
> think about it.
> 

This looks good to me. Maybe it's a bit verbose but that's what the doc
part should be: providing enough information so more people can be on
the same page. Please keep this one, thanks!

Regards,
Boqun

> Cheers,
> Longman
> 

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

[Carlos Llamas]

On Tue, Aug 06, 2024 at 10:42:52AM -0700, Boqun Feng wrote:
> On Tue, Aug 06, 2024 at 10:52:52AM -0400, Waiman Long wrote:
> > 
> > On 8/6/24 10:47, Carlos Llamas wrote:
> > > On Mon, Aug 05, 2024 at 09:36:43PM -0400, Waiman Long wrote:
> > > > Many kernel developers understand that BITS refers to a size of 2^n. Besides
> > > > LOCKDEP, there are also many instances of such use in other kconfig entries.
> > > > It can be a bit odd to explicitly state that just for LOCKDEP.
> > > > 
> > > > Cheers,
> > > > Longman
> > > Right, and similar to BITS there is SHIFT, which is also a common way to
> > > specify the 2^n values. I'd point out though, that it is also common to
> > > clarify the "power of two" explicitly. To name a few examples that are
> > > doing so: SECURITY_SELINUX_SIDTAB_HASH_BITS, NODES_SHIFT, CMA_ALIGNMENT,
> > > IP_VS_SH_TAB_BITS, LOG_BUF_SHIFT but there is more.
> > > 
> > > Perhaps this is because the audience for these configs is not always a
> > > kernel developer?
> > > 
> > > Anyway, this is pretty much a trivial patch to address Andrew's comment
> > > below. But let me know if you think I should drop it, it seems to me it
> > > can be helpful.
> > > 
> > >    [...]
> > >    btw, the help text "Bitsize for MAX_LOCKDEP_CHAINS" is odd.  What's a
> > >    bitsize?  Maybe "bit shift count for..." or such.
> > 
> > I am not against this patch. Currently I am neutral. Let's see what Boqun
> > think about it.
> > 
> 
> This looks good to me. Maybe it's a bit verbose but that's what the doc
> part should be: providing enough information so more people can be on
> the same page. Please keep this one, thanks!

Sounds good. I'll send out the v2 and keep this patch then.

Thanks,
Carlos Llamas