Both arch/um/ and arch/xtensa/ cause a Kconfig warning for LOCKDEP.
These arch-es select LOCKDEP_SUPPORT but they are not listed as one
of the arch-es that LOCKDEP depends on.
Since (16) arch-es define the Kconfig symbol LOCKDEP_SUPPORT if they
intend to have LOCKDEP support, replace the awkward list of
arch-es that LOCKDEP depends on with the LOCKDEP_SUPPORT symbol.
Fixes this kconfig warning: (for both um and xtensa)
WARNING: unmet direct dependencies detected for LOCKDEP
Depends on [n]: DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y] && (FRAME_POINTER [=n] || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
Selected by [y]:
- PROVE_LOCKING [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
- LOCK_STAT [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
- DEBUG_LOCK_ALLOC [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
Signed-off-by: Randy Dunlap <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: Boqun Feng <[email protected]>
Cc: Chris Zankel <[email protected]>
Cc: Max Filippov <[email protected]>
Cc: [email protected]
Cc: Johannes Berg <[email protected]>
Cc: Jeff Dike <[email protected]>
Cc: Richard Weinberger <[email protected]>
Cc: Anton Ivanov <[email protected]>
Cc: [email protected]
---
lib/Kconfig.debug | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20210514.orig/lib/Kconfig.debug
+++ linux-next-20210514/lib/Kconfig.debug
@@ -1383,7 +1383,7 @@ config LOCKDEP
bool
depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
select STACKTRACE
- depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86
+ depends on FRAME_POINTER || LOCKDEP_SUPPORT
select KALLSYMS
select KALLSYMS_ALL
* Randy Dunlap <[email protected]> wrote:
> Both arch/um/ and arch/xtensa/ cause a Kconfig warning for LOCKDEP.
> These arch-es select LOCKDEP_SUPPORT but they are not listed as one
> of the arch-es that LOCKDEP depends on.
>
> Since (16) arch-es define the Kconfig symbol LOCKDEP_SUPPORT if they
> intend to have LOCKDEP support, replace the awkward list of
> arch-es that LOCKDEP depends on with the LOCKDEP_SUPPORT symbol.
>
> Fixes this kconfig warning: (for both um and xtensa)
>
> WARNING: unmet direct dependencies detected for LOCKDEP
> Depends on [n]: DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y] && (FRAME_POINTER [=n] || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
> Selected by [y]:
> - PROVE_LOCKING [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
> - LOCK_STAT [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
> - DEBUG_LOCK_ALLOC [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>
> Signed-off-by: Randy Dunlap <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Waiman Long <[email protected]>
> Cc: Boqun Feng <[email protected]>
> Cc: Chris Zankel <[email protected]>
> Cc: Max Filippov <[email protected]>
> Cc: [email protected]
> Cc: Johannes Berg <[email protected]>
> Cc: Jeff Dike <[email protected]>
> Cc: Richard Weinberger <[email protected]>
> Cc: Anton Ivanov <[email protected]>
> Cc: [email protected]
> ---
> lib/Kconfig.debug | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-next-20210514.orig/lib/Kconfig.debug
> +++ linux-next-20210514/lib/Kconfig.debug
> @@ -1383,7 +1383,7 @@ config LOCKDEP
> bool
> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
> select STACKTRACE
> - depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86
> + depends on FRAME_POINTER || LOCKDEP_SUPPORT
Ok - the FRAME_POINTER bit is weird. Are there any architectures that have
FRAME_POINTER defined but no LOCKDEP_SUPPORT?
Thanks,
Ingo
On 5/17/21 3:11 AM, Ingo Molnar wrote:
> * Randy Dunlap <[email protected]> wrote:
>
>> Both arch/um/ and arch/xtensa/ cause a Kconfig warning for LOCKDEP.
>> These arch-es select LOCKDEP_SUPPORT but they are not listed as one
>> of the arch-es that LOCKDEP depends on.
>>
>> Since (16) arch-es define the Kconfig symbol LOCKDEP_SUPPORT if they
>> intend to have LOCKDEP support, replace the awkward list of
>> arch-es that LOCKDEP depends on with the LOCKDEP_SUPPORT symbol.
>>
>> Fixes this kconfig warning: (for both um and xtensa)
>>
>> WARNING: unmet direct dependencies detected for LOCKDEP
>> Depends on [n]: DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y] && (FRAME_POINTER [=n] || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
>> Selected by [y]:
>> - PROVE_LOCKING [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>> - LOCK_STAT [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>> - DEBUG_LOCK_ALLOC [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>
>> Signed-off-by: Randy Dunlap <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Waiman Long <[email protected]>
>> Cc: Boqun Feng <[email protected]>
>> Cc: Chris Zankel <[email protected]>
>> Cc: Max Filippov <[email protected]>
>> Cc: [email protected]
>> Cc: Johannes Berg <[email protected]>
>> Cc: Jeff Dike <[email protected]>
>> Cc: Richard Weinberger <[email protected]>
>> Cc: Anton Ivanov <[email protected]>
>> Cc: [email protected]
>> ---
>> lib/Kconfig.debug | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- linux-next-20210514.orig/lib/Kconfig.debug
>> +++ linux-next-20210514/lib/Kconfig.debug
>> @@ -1383,7 +1383,7 @@ config LOCKDEP
>> bool
>> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
>> select STACKTRACE
>> - depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86
>> + depends on FRAME_POINTER || LOCKDEP_SUPPORT
> Ok - the FRAME_POINTER bit is weird. Are there any architectures that have
> FRAME_POINTER defined but no LOCKDEP_SUPPORT?
LOCK_DEBUGGING_SUPPORT depends on LOCKDEP_SUPPORT. So this patch is
equivalent to just delete the second depends-on line. Beside LOCKDEP,
LATENCYTOP also have exactly the same depends-on line.
So isn't FRAME_POINTER used mainly to support STACK_TRACE? However,
LOCK_DEBUGGING_SUPPORT has already included STACK_TRACE_SUPPORT in its
dependency. So why there is a FRAME_POINTER dependency?
Cheers,
Longman
On 5/17/21 12:11 AM, Ingo Molnar wrote:
>
> * Randy Dunlap <[email protected]> wrote:
>
>> Both arch/um/ and arch/xtensa/ cause a Kconfig warning for LOCKDEP.
>> These arch-es select LOCKDEP_SUPPORT but they are not listed as one
>> of the arch-es that LOCKDEP depends on.
>>
>> Since (16) arch-es define the Kconfig symbol LOCKDEP_SUPPORT if they
>> intend to have LOCKDEP support, replace the awkward list of
>> arch-es that LOCKDEP depends on with the LOCKDEP_SUPPORT symbol.
>>
>> Fixes this kconfig warning: (for both um and xtensa)
>>
>> WARNING: unmet direct dependencies detected for LOCKDEP
>> Depends on [n]: DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y] && (FRAME_POINTER [=n] || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
>> Selected by [y]:
>> - PROVE_LOCKING [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>> - LOCK_STAT [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>> - DEBUG_LOCK_ALLOC [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>
>> Signed-off-by: Randy Dunlap <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Waiman Long <[email protected]>
>> Cc: Boqun Feng <[email protected]>
>> Cc: Chris Zankel <[email protected]>
>> Cc: Max Filippov <[email protected]>
>> Cc: [email protected]
>> Cc: Johannes Berg <[email protected]>
>> Cc: Jeff Dike <[email protected]>
>> Cc: Richard Weinberger <[email protected]>
>> Cc: Anton Ivanov <[email protected]>
>> Cc: [email protected]
>> ---
>> lib/Kconfig.debug | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- linux-next-20210514.orig/lib/Kconfig.debug
>> +++ linux-next-20210514/lib/Kconfig.debug
>> @@ -1383,7 +1383,7 @@ config LOCKDEP
>> bool
>> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
>> select STACKTRACE
>> - depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86
>> + depends on FRAME_POINTER || LOCKDEP_SUPPORT
>
> Ok - the FRAME_POINTER bit is weird. Are there any architectures that have
> FRAME_POINTER defined but no LOCKDEP_SUPPORT?
arch/h8300/ does AFAICT.
and arch/parisc/ has user-choosable FRAME_POINTER and no LOCKDEP support.
--
~Randy
On 5/17/21 7:02 AM, Waiman Long wrote:
> On 5/17/21 3:11 AM, Ingo Molnar wrote:
>> * Randy Dunlap <[email protected]> wrote:
>>
>>> Both arch/um/ and arch/xtensa/ cause a Kconfig warning for LOCKDEP.
>>> These arch-es select LOCKDEP_SUPPORT but they are not listed as one
>>> of the arch-es that LOCKDEP depends on.
>>>
>>> Since (16) arch-es define the Kconfig symbol LOCKDEP_SUPPORT if they
>>> intend to have LOCKDEP support, replace the awkward list of
>>> arch-es that LOCKDEP depends on with the LOCKDEP_SUPPORT symbol.
>>>
>>> Fixes this kconfig warning: (for both um and xtensa)
>>>
>>> WARNING: unmet direct dependencies detected for LOCKDEP
>>> Depends on [n]: DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y] && (FRAME_POINTER [=n] || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
>>> Selected by [y]:
>>> - PROVE_LOCKING [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>> - LOCK_STAT [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>> - DEBUG_LOCK_ALLOC [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>
>>> Signed-off-by: Randy Dunlap <[email protected]>
>>> Cc: Peter Zijlstra <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: Will Deacon <[email protected]>
>>> Cc: Waiman Long <[email protected]>
>>> Cc: Boqun Feng <[email protected]>
>>> Cc: Chris Zankel <[email protected]>
>>> Cc: Max Filippov <[email protected]>
>>> Cc: [email protected]
>>> Cc: Johannes Berg <[email protected]>
>>> Cc: Jeff Dike <[email protected]>
>>> Cc: Richard Weinberger <[email protected]>
>>> Cc: Anton Ivanov <[email protected]>
>>> Cc: [email protected]
>>> ---
>>> lib/Kconfig.debug | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> --- linux-next-20210514.orig/lib/Kconfig.debug
>>> +++ linux-next-20210514/lib/Kconfig.debug
>>> @@ -1383,7 +1383,7 @@ config LOCKDEP
>>> bool
>>> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
>>> select STACKTRACE
>>> - depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86
>>> + depends on FRAME_POINTER || LOCKDEP_SUPPORT
>> Ok - the FRAME_POINTER bit is weird. Are there any architectures that have
>> FRAME_POINTER defined but no LOCKDEP_SUPPORT?
>
> LOCK_DEBUGGING_SUPPORT depends on LOCKDEP_SUPPORT. So this patch is equivalent to just delete the second depends-on line. Beside LOCKDEP, LATENCYTOP also have exactly the same depends-on line.
I will attempt to consolidate those.
> So isn't FRAME_POINTER used mainly to support STACK_TRACE? However, LOCK_DEBUGGING_SUPPORT has already included STACK_TRACE_SUPPORT in its dependency. So why there is a FRAME_POINTER dependency?
I dunno why FRAME_POINTER is there. It was there before this patch.
thanks.
--
~Randy
On 5/17/21 7:02 AM, Waiman Long wrote:
> On 5/17/21 3:11 AM, Ingo Molnar wrote:
>> * Randy Dunlap <[email protected]> wrote:
>>
>>> Both arch/um/ and arch/xtensa/ cause a Kconfig warning for LOCKDEP.
>>> These arch-es select LOCKDEP_SUPPORT but they are not listed as one
>>> of the arch-es that LOCKDEP depends on.
>>>
>>> Since (16) arch-es define the Kconfig symbol LOCKDEP_SUPPORT if they
>>> intend to have LOCKDEP support, replace the awkward list of
>>> arch-es that LOCKDEP depends on with the LOCKDEP_SUPPORT symbol.
>>>
>>> Fixes this kconfig warning: (for both um and xtensa)
>>>
>>> WARNING: unmet direct dependencies detected for LOCKDEP
>>> Depends on [n]: DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y] && (FRAME_POINTER [=n] || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
>>> Selected by [y]:
>>> - PROVE_LOCKING [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>> - LOCK_STAT [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>> - DEBUG_LOCK_ALLOC [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>
>>> Signed-off-by: Randy Dunlap <[email protected]>
>>> Cc: Peter Zijlstra <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: Will Deacon <[email protected]>
>>> Cc: Waiman Long <[email protected]>
>>> Cc: Boqun Feng <[email protected]>
>>> Cc: Chris Zankel <[email protected]>
>>> Cc: Max Filippov <[email protected]>
>>> Cc: [email protected]
>>> Cc: Johannes Berg <[email protected]>
>>> Cc: Jeff Dike <[email protected]>
>>> Cc: Richard Weinberger <[email protected]>
>>> Cc: Anton Ivanov <[email protected]>
>>> Cc: [email protected]
>>> ---
>>> lib/Kconfig.debug | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> --- linux-next-20210514.orig/lib/Kconfig.debug
>>> +++ linux-next-20210514/lib/Kconfig.debug
>>> @@ -1383,7 +1383,7 @@ config LOCKDEP
>>> bool
>>> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
>>> select STACKTRACE
>>> - depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86
>>> + depends on FRAME_POINTER || LOCKDEP_SUPPORT
>> Ok - the FRAME_POINTER bit is weird. Are there any architectures that have
>> FRAME_POINTER defined but no LOCKDEP_SUPPORT?
>
> LOCK_DEBUGGING_SUPPORT depends on LOCKDEP_SUPPORT. So this patch is equivalent to just delete the second depends-on line.
Yes, if we disregard the FRAME_POINTER part.
> Beside LOCKDEP, LATENCYTOP also have exactly the same depends-on line.
True, but I don't get any implication that the same patch applies there.
Do you?
> So isn't FRAME_POINTER used mainly to support STACK_TRACE? However, LOCK_DEBUGGING_SUPPORT has already included STACK_TRACE_SUPPORT in its dependency. So why there is a FRAME_POINTER dependency?
FRAME_POINTER is one way but it does not seem to be required
for STACKTRACE_SUPPORT.
Do you have any patch suggestions?
thanks.
--
~Randy
On 5/24/21 2:04 PM, Waiman Long wrote:
> On 5/24/21 3:47 PM, Randy Dunlap wrote:
>> On 5/17/21 7:02 AM, Waiman Long wrote:
>>> On 5/17/21 3:11 AM, Ingo Molnar wrote:
>>>> * Randy Dunlap <[email protected]> wrote:
>>>>
>>>>> Both arch/um/ and arch/xtensa/ cause a Kconfig warning for LOCKDEP.
>>>>> These arch-es select LOCKDEP_SUPPORT but they are not listed as one
>>>>> of the arch-es that LOCKDEP depends on.
>>>>>
>>>>> Since (16) arch-es define the Kconfig symbol LOCKDEP_SUPPORT if they
>>>>> intend to have LOCKDEP support, replace the awkward list of
>>>>> arch-es that LOCKDEP depends on with the LOCKDEP_SUPPORT symbol.
>>>>>
>>>>> Fixes this kconfig warning: (for both um and xtensa)
>>>>>
>>>>> WARNING: unmet direct dependencies detected for LOCKDEP
>>>>> Depends on [n]: DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y] && (FRAME_POINTER [=n] || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
>>>>> Selected by [y]:
>>>>> - PROVE_LOCKING [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>>> - LOCK_STAT [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>>> - DEBUG_LOCK_ALLOC [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>>>
>>>>> Signed-off-by: Randy Dunlap <[email protected]>
>>>>> Cc: Peter Zijlstra <[email protected]>
>>>>> Cc: Ingo Molnar <[email protected]>
>>>>> Cc: Will Deacon <[email protected]>
>>>>> Cc: Waiman Long <[email protected]>
>>>>> Cc: Boqun Feng <[email protected]>
>>>>> Cc: Chris Zankel <[email protected]>
>>>>> Cc: Max Filippov <[email protected]>
>>>>> Cc: [email protected]
>>>>> Cc: Johannes Berg <[email protected]>
>>>>> Cc: Jeff Dike <[email protected]>
>>>>> Cc: Richard Weinberger <[email protected]>
>>>>> Cc: Anton Ivanov <[email protected]>
>>>>> Cc: [email protected]
>>>>> ---
>>>>> lib/Kconfig.debug | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> --- linux-next-20210514.orig/lib/Kconfig.debug
>>>>> +++ linux-next-20210514/lib/Kconfig.debug
>>>>> @@ -1383,7 +1383,7 @@ config LOCKDEP
>>>>> bool
>>>>> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
>>>>> select STACKTRACE
>>>>> - depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86
>>>>> + depends on FRAME_POINTER || LOCKDEP_SUPPORT
>>>> Ok - the FRAME_POINTER bit is weird. Are there any architectures that have
>>>> FRAME_POINTER defined but no LOCKDEP_SUPPORT?
>>> LOCK_DEBUGGING_SUPPORT depends on LOCKDEP_SUPPORT. So this patch is equivalent to just delete the second depends-on line.
>> Yes, if we disregard the FRAME_POINTER part.
>
> My understanding is that the 2 depends-on statements have an implicit AND. So it is like
Right (on the implicit AND).
> DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER || LOCKDEP_SUPPORT). LOCK_DEBUGGING_SUPPORT is true means the (FRAME_POINTER || LOCKDEP_SUPPORT) will always be true. FRAME_POINTER is true doesn't mean the other dependencies are true. That is why I said it is equivalent to just "DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT". IOW, FRAME_POINTER will play no part here.
>
Ack. I should have done that myself.
>>
>>> Beside LOCKDEP, LATENCYTOP also have exactly the same depends-on line.
>> True, but I don't get any implication that the same patch applies there.
>> Do you?
> It is just an observation that I stumble on. It is not related to your patch.
Got it.
>>> So isn't FRAME_POINTER used mainly to support STACK_TRACE? However, LOCK_DEBUGGING_SUPPORT has already included STACK_TRACE_SUPPORT in its dependency. So why there is a FRAME_POINTER dependency?
>> FRAME_POINTER is one way but it does not seem to be required
>> for STACKTRACE_SUPPORT.
>>
>> Do you have any patch suggestions?
>
> Is it possible to just get rid of the 2nd depends-on statement?
>
> The 2nd depends-on line was introduced by commit 7d37cb2c912d ("lib: fix kconfig dependency on ARCH_WANT_FRAME_POINTER"):
and I should have looked at that history too. Thanks.
Yes, I agree, we can just delete that line...
I'll send a v2 and copy the author of commit 7d37cb2c912d as well.
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 2779c29d9981..417c3d3e521b 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1363,7 +1363,7 @@ config LOCKDEP
> bool
> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
> select STACKTRACE
> - select FRAME_POINTER if !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE &&
> + depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM ||
> select KALLSYMS
> select KALLSYMS_ALL
>
> Since STACKTRACE is selected by lockdep, maybe we can just remove the 2nd depends-on line to see if anyone complain.
>
> Cheers,
> Longman
>
thanks.
--
~Randy
On 5/24/21 2:31 PM, Randy Dunlap wrote:
> On 5/24/21 2:04 PM, Waiman Long wrote:
>> On 5/24/21 3:47 PM, Randy Dunlap wrote:
>>> On 5/17/21 7:02 AM, Waiman Long wrote:
>>>> On 5/17/21 3:11 AM, Ingo Molnar wrote:
>>>>> * Randy Dunlap <[email protected]> wrote:
>>>>>
>>>>>> Both arch/um/ and arch/xtensa/ cause a Kconfig warning for LOCKDEP.
>>>>>> These arch-es select LOCKDEP_SUPPORT but they are not listed as one
>>>>>> of the arch-es that LOCKDEP depends on.
>>>>>>
>>>>>> Since (16) arch-es define the Kconfig symbol LOCKDEP_SUPPORT if they
>>>>>> intend to have LOCKDEP support, replace the awkward list of
>>>>>> arch-es that LOCKDEP depends on with the LOCKDEP_SUPPORT symbol.
>>>>>>
>>>>>> Fixes this kconfig warning: (for both um and xtensa)
>>>>>>
>>>>>> WARNING: unmet direct dependencies detected for LOCKDEP
>>>>>> Depends on [n]: DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y] && (FRAME_POINTER [=n] || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
>>>>>> Selected by [y]:
>>>>>> - PROVE_LOCKING [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>>>> - LOCK_STAT [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>>>> - DEBUG_LOCK_ALLOC [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>>>>
>>>>>> Signed-off-by: Randy Dunlap <[email protected]>
>>>>>> Cc: Peter Zijlstra <[email protected]>
>>>>>> Cc: Ingo Molnar <[email protected]>
>>>>>> Cc: Will Deacon <[email protected]>
>>>>>> Cc: Waiman Long <[email protected]>
>>>>>> Cc: Boqun Feng <[email protected]>
>>>>>> Cc: Chris Zankel <[email protected]>
>>>>>> Cc: Max Filippov <[email protected]>
>>>>>> Cc: [email protected]
>>>>>> Cc: Johannes Berg <[email protected]>
>>>>>> Cc: Jeff Dike <[email protected]>
>>>>>> Cc: Richard Weinberger <[email protected]>
>>>>>> Cc: Anton Ivanov <[email protected]>
>>>>>> Cc: [email protected]
>>>>>> ---
>>>>>> lib/Kconfig.debug | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> --- linux-next-20210514.orig/lib/Kconfig.debug
>>>>>> +++ linux-next-20210514/lib/Kconfig.debug
>>>>>> @@ -1383,7 +1383,7 @@ config LOCKDEP
>>>>>> bool
>>>>>> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
>>>>>> select STACKTRACE
>>>>>> - depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86
>>>>>> + depends on FRAME_POINTER || LOCKDEP_SUPPORT
>>>>> Ok - the FRAME_POINTER bit is weird. Are there any architectures that have
>>>>> FRAME_POINTER defined but no LOCKDEP_SUPPORT?
>>>> LOCK_DEBUGGING_SUPPORT depends on LOCKDEP_SUPPORT. So this patch is equivalent to just delete the second depends-on line.
>>> Yes, if we disregard the FRAME_POINTER part.
>>
>> My understanding is that the 2 depends-on statements have an implicit AND. So it is like
>
> Right (on the implicit AND).
>
>> DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER || LOCKDEP_SUPPORT). LOCK_DEBUGGING_SUPPORT is true means the (FRAME_POINTER || LOCKDEP_SUPPORT) will always be true. FRAME_POINTER is true doesn't mean the other dependencies are true. That is why I said it is equivalent to just "DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT". IOW, FRAME_POINTER will play no part here.
>>
>
> Ack. I should have done that myself.
>
>>>
>>>> Beside LOCKDEP, LATENCYTOP also have exactly the same depends-on line.
>>> True, but I don't get any implication that the same patch applies there.
>>> Do you?
>> It is just an observation that I stumble on. It is not related to your patch.
>
> Got it.
>
>>>> So isn't FRAME_POINTER used mainly to support STACK_TRACE? However, LOCK_DEBUGGING_SUPPORT has already included STACK_TRACE_SUPPORT in its dependency. So why there is a FRAME_POINTER dependency?
>>> FRAME_POINTER is one way but it does not seem to be required
>>> for STACKTRACE_SUPPORT.
>>>
>>> Do you have any patch suggestions?
>>
>> Is it possible to just get rid of the 2nd depends-on statement?
>>
>> The 2nd depends-on line was introduced by commit 7d37cb2c912d ("lib: fix kconfig dependency on ARCH_WANT_FRAME_POINTER"):
>
> and I should have looked at that history too. Thanks.
>
> Yes, I agree, we can just delete that line...
>
> I'll send a v2 and copy the author of commit 7d37cb2c912d as well.
Hm, as I review that commit, I have to wonder if the previous 'select'
was correct (if we disregard the Kconfig warning). If so, then
FRAME_POINTER is still wanted/needed for some arch-es.
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2779c29d9981..417c3d3e521b 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1363,7 +1363,7 @@ config LOCKDEP
bool
depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
select STACKTRACE
- select FRAME_POINTER if !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE && !ARC && !X86 <<<<<<<<<<<
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 2779c29d9981..417c3d3e521b 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -1363,7 +1363,7 @@ config LOCKDEP
>> bool
>> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
>> select STACKTRACE
>> - select FRAME_POINTER if !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE &&
>> + depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM ||
>> select KALLSYMS
>> select KALLSYMS_ALL
>>
>> Since STACKTRACE is selected by lockdep, maybe we can just remove the 2nd depends-on line to see if anyone complain.
--
~Randy
On 5/24/21 3:47 PM, Randy Dunlap wrote:
> On 5/17/21 7:02 AM, Waiman Long wrote:
>> On 5/17/21 3:11 AM, Ingo Molnar wrote:
>>> * Randy Dunlap <[email protected]> wrote:
>>>
>>>> Both arch/um/ and arch/xtensa/ cause a Kconfig warning for LOCKDEP.
>>>> These arch-es select LOCKDEP_SUPPORT but they are not listed as one
>>>> of the arch-es that LOCKDEP depends on.
>>>>
>>>> Since (16) arch-es define the Kconfig symbol LOCKDEP_SUPPORT if they
>>>> intend to have LOCKDEP support, replace the awkward list of
>>>> arch-es that LOCKDEP depends on with the LOCKDEP_SUPPORT symbol.
>>>>
>>>> Fixes this kconfig warning: (for both um and xtensa)
>>>>
>>>> WARNING: unmet direct dependencies detected for LOCKDEP
>>>> Depends on [n]: DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y] && (FRAME_POINTER [=n] || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
>>>> Selected by [y]:
>>>> - PROVE_LOCKING [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>> - LOCK_STAT [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>> - DEBUG_LOCK_ALLOC [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y]
>>>>
>>>> Signed-off-by: Randy Dunlap <[email protected]>
>>>> Cc: Peter Zijlstra <[email protected]>
>>>> Cc: Ingo Molnar <[email protected]>
>>>> Cc: Will Deacon <[email protected]>
>>>> Cc: Waiman Long <[email protected]>
>>>> Cc: Boqun Feng <[email protected]>
>>>> Cc: Chris Zankel <[email protected]>
>>>> Cc: Max Filippov <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: Johannes Berg <[email protected]>
>>>> Cc: Jeff Dike <[email protected]>
>>>> Cc: Richard Weinberger <[email protected]>
>>>> Cc: Anton Ivanov <[email protected]>
>>>> Cc: [email protected]
>>>> ---
>>>> lib/Kconfig.debug | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> --- linux-next-20210514.orig/lib/Kconfig.debug
>>>> +++ linux-next-20210514/lib/Kconfig.debug
>>>> @@ -1383,7 +1383,7 @@ config LOCKDEP
>>>> bool
>>>> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
>>>> select STACKTRACE
>>>> - depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86
>>>> + depends on FRAME_POINTER || LOCKDEP_SUPPORT
>>> Ok - the FRAME_POINTER bit is weird. Are there any architectures that have
>>> FRAME_POINTER defined but no LOCKDEP_SUPPORT?
>> LOCK_DEBUGGING_SUPPORT depends on LOCKDEP_SUPPORT. So this patch is equivalent to just delete the second depends-on line.
> Yes, if we disregard the FRAME_POINTER part.
My understanding is that the 2 depends-on statements have an implicit
AND. So it is like
DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER ||
LOCKDEP_SUPPORT). LOCK_DEBUGGING_SUPPORT is true means the
(FRAME_POINTER || LOCKDEP_SUPPORT) will always be true. FRAME_POINTER is
true doesn't mean the other dependencies are true. That is why I said it
is equivalent to just "DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT". IOW,
FRAME_POINTER will play no part here.
>
>> Beside LOCKDEP, LATENCYTOP also have exactly the same depends-on line.
> True, but I don't get any implication that the same patch applies there.
> Do you?
It is just an observation that I stumble on. It is not related to your
patch.
>> So isn't FRAME_POINTER used mainly to support STACK_TRACE? However, LOCK_DEBUGGING_SUPPORT has already included STACK_TRACE_SUPPORT in its dependency. So why there is a FRAME_POINTER dependency?
> FRAME_POINTER is one way but it does not seem to be required
> for STACKTRACE_SUPPORT.
>
> Do you have any patch suggestions?
Is it possible to just get rid of the 2nd depends-on statement?
The 2nd depends-on line was introduced by commit 7d37cb2c912d ("lib: fix
kconfig dependency on ARCH_WANT_FRAME_POINTER"):
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2779c29d9981..417c3d3e521b 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1363,7 +1363,7 @@ config LOCKDEP
bool
depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
select STACKTRACE
- select FRAME_POINTER if !MIPS && !PPC && !ARM && !S390 &&
!MICROBLAZE &&
+ depends on FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE ||
ARM ||
select KALLSYMS
select KALLSYMS_ALL
Since STACKTRACE is selected by lockdep, maybe we can just remove the
2nd depends-on line to see if anyone complain.
Cheers,
Longman
On 5/24/21 5:41 PM, Randy Dunlap wrote:
> On 5/24/21 2:31 PM, Randy Dunlap wrote:
>> On 5/24/21 2:04 PM, Waiman Long wrote:
>>
>>> Is it possible to just get rid of the 2nd depends-on statement?
>>>
>>> The 2nd depends-on line was introduced by commit 7d37cb2c912d ("lib: fix kconfig dependency on ARCH_WANT_FRAME_POINTER"):
>> and I should have looked at that history too. Thanks.
>>
>> Yes, I agree, we can just delete that line...
>>
>> I'll send a v2 and copy the author of commit 7d37cb2c912d as well.
> Hm, as I review that commit, I have to wonder if the previous 'select'
> was correct (if we disregard the Kconfig warning). If so, then
> FRAME_POINTER is still wanted/needed for some arch-es.
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 2779c29d9981..417c3d3e521b 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1363,7 +1363,7 @@ config LOCKDEP
> bool
> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
> select STACKTRACE
> - select FRAME_POINTER if !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE && !ARC && !X86 <<<<<<<<<<<
>
AFAICS, enabling FRAME_POINTER is a debugging aid as it enable more
precise stacktrace. However, not all archs want to enable FRAME_POINTER
because of LOCKDEP. Now you are just letting users decide if they want
FRAME_POINTER or not. Maybe you can modify the help text to mention that.
Cheers,
Longman
On 5/24/21 5:05 PM, Waiman Long wrote:
> On 5/24/21 5:41 PM, Randy Dunlap wrote:
>> On 5/24/21 2:31 PM, Randy Dunlap wrote:
>>> On 5/24/21 2:04 PM, Waiman Long wrote:
>>>
>>>> Is it possible to just get rid of the 2nd depends-on statement?
>>>>
>>>> The 2nd depends-on line was introduced by commit 7d37cb2c912d ("lib: fix kconfig dependency on ARCH_WANT_FRAME_POINTER"):
>>> and I should have looked at that history too. Thanks.
>>>
>>> Yes, I agree, we can just delete that line...
>>>
>>> I'll send a v2 and copy the author of commit 7d37cb2c912d as well.
>> Hm, as I review that commit, I have to wonder if the previous 'select'
>> was correct (if we disregard the Kconfig warning). If so, then
>> FRAME_POINTER is still wanted/needed for some arch-es.
>>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 2779c29d9981..417c3d3e521b 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -1363,7 +1363,7 @@ config LOCKDEP
>> bool
>> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
>> select STACKTRACE
>> - select FRAME_POINTER if !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE && !ARC && !X86 <<<<<<<<<<<
>>
> AFAICS, enabling FRAME_POINTER is a debugging aid as it enable more precise stacktrace. However, not all archs want to enable FRAME_POINTER because of LOCKDEP. Now you are just letting users decide if they want FRAME_POINTER or not. Maybe you can modify the help text to mention that.
LOCKDEP doesn't have any user help text.
For FRAME_POINTER, when ARCH_WANT_FRAME_POINTERS + a few other conditions
is met, it seems that the FRAME_POINTER help text is good enough IMO.
help
If you say Y here the resulting kernel image will be slightly
larger and slower, but it gives very useful debugging information
in case of kernel bugs. (precise oopses/stacktraces/warnings)
--
~Randy
On 5/25/21 12:18 AM, Randy Dunlap wrote:
> On 5/24/21 5:05 PM, Waiman Long wrote:
>> On 5/24/21 5:41 PM, Randy Dunlap wrote:
>>> On 5/24/21 2:31 PM, Randy Dunlap wrote:
>>>> On 5/24/21 2:04 PM, Waiman Long wrote:
>>>>
>>>>> Is it possible to just get rid of the 2nd depends-on statement?
>>>>>
>>>>> The 2nd depends-on line was introduced by commit 7d37cb2c912d ("lib: fix kconfig dependency on ARCH_WANT_FRAME_POINTER"):
>>>> and I should have looked at that history too. Thanks.
>>>>
>>>> Yes, I agree, we can just delete that line...
>>>>
>>>> I'll send a v2 and copy the author of commit 7d37cb2c912d as well.
>>> Hm, as I review that commit, I have to wonder if the previous 'select'
>>> was correct (if we disregard the Kconfig warning). If so, then
>>> FRAME_POINTER is still wanted/needed for some arch-es.
>>>
>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>> index 2779c29d9981..417c3d3e521b 100644
>>> --- a/lib/Kconfig.debug
>>> +++ b/lib/Kconfig.debug
>>> @@ -1363,7 +1363,7 @@ config LOCKDEP
>>> bool
>>> depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
>>> select STACKTRACE
>>> - select FRAME_POINTER if !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE && !ARC && !X86 <<<<<<<<<<<
>>>
>> AFAICS, enabling FRAME_POINTER is a debugging aid as it enable more precise stacktrace. However, not all archs want to enable FRAME_POINTER because of LOCKDEP. Now you are just letting users decide if they want FRAME_POINTER or not. Maybe you can modify the help text to mention that.
> LOCKDEP doesn't have any user help text.
>
> For FRAME_POINTER, when ARCH_WANT_FRAME_POINTERS + a few other conditions
> is met, it seems that the FRAME_POINTER help text is good enough IMO.
>
> help
> If you say Y here the resulting kernel image will be slightly
> larger and slower, but it gives very useful debugging information
> in case of kernel bugs. (precise oopses/stacktraces/warnings)
>
Right. I am fine with that.
Thanks,
Longman