* [PATCH] lockdep: call trace_hardirqs_off after clearing hardirqs_enabled
@ 2017-03-13 10:08 Qi Hou
2017-03-13 14:05 ` Steven Rostedt
2017-03-13 14:37 ` Peter Zijlstra
0 siblings, 2 replies; 4+ messages in thread
From: Qi Hou @ 2017-03-13 10:08 UTC (permalink / raw)
To: mingo, rostedt, tglx; +Cc: linux-kernel, C.Emde, paul.gortmaker, bruce.ashfield
The current order of calls within trace_hardirqs_off() would provoke an
"unannotated irqs-off" WARNING.
This warning was reported by check_flags() when it found that the hardirqs has
been disabled but the irq-flags state, hardirqs_enabled, has not been cleared.
Check_flags() is called via trace_hardirqs_off(), and trace_hardirqs_off() will
be called by functions kind of local_irq_disable() if CONFIG_TRACE_IRQFLAGS has
been enabled.
The calltrace from functions kind of local_irq_diasble() to check_flags()
simplied as below:
local_irq_disable()
|- trace_hardirqs_off()
|- trace_hardirqs_off_caller()
|- preemptirqsoff_hist()
|- __DO_TRACE()
|- rcu_dereference_sched()
|- rcu_read_lock_sched_held()
|- lock_is_held()
|- check_flags()
The order of calls within trace_hardirqs_off() can be simplified as below:
local_irq_disable()
|- raw_local_irq_disable()
|- trace_hardirqs_off()
|- trace_hardirqs_off() --> check_flags()
|- hardirqs_enabled = 0
We could find that the call to check_flags() has been made between disabling
hardirqs and clearing hardirqs_enabled. That will definitely cause an
"unannotated irqs-off" warning.
To fix this "unannotated irqs-off" warning, we must call trace_hardirqs_off()
after that we clear hardirqs_enabled.
Signed-off-by: Qi Hou <qi.hou@windriver.com>
---
kernel/locking/lockdep.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 577f026..e8b35e4 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2627,8 +2627,6 @@ __visible void trace_hardirqs_off_caller(unsigned long ip)
{
struct task_struct *curr = current;
- time_hardirqs_off(CALLER_ADDR0, ip);
-
if (unlikely(!debug_locks || current->lockdep_recursion))
return;
@@ -2649,6 +2647,8 @@ __visible void trace_hardirqs_off_caller(unsigned long ip)
debug_atomic_inc(hardirqs_off_events);
} else
debug_atomic_inc(redundant_hardirqs_off);
+
+ time_hardirqs_off(CALLER_ADDR0, ip);
}
EXPORT_SYMBOL(trace_hardirqs_off_caller);
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] lockdep: call trace_hardirqs_off after clearing hardirqs_enabled
2017-03-13 10:08 [PATCH] lockdep: call trace_hardirqs_off after clearing hardirqs_enabled Qi Hou
@ 2017-03-13 14:05 ` Steven Rostedt
2017-03-14 1:24 ` qhou
2017-03-13 14:37 ` Peter Zijlstra
1 sibling, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2017-03-13 14:05 UTC (permalink / raw)
To: Qi Hou; +Cc: mingo, tglx, linux-kernel, C.Emde, paul.gortmaker, bruce.ashfield
On Mon, 13 Mar 2017 18:08:48 +0800
Qi Hou <qi.hou@windriver.com> wrote:
> The current order of calls within trace_hardirqs_off() would provoke an
> "unannotated irqs-off" WARNING.
>
> This warning was reported by check_flags() when it found that the hardirqs has
> been disabled but the irq-flags state, hardirqs_enabled, has not been cleared.
>
> Check_flags() is called via trace_hardirqs_off(), and trace_hardirqs_off() will
> be called by functions kind of local_irq_disable() if CONFIG_TRACE_IRQFLAGS has
> been enabled.
>
> The calltrace from functions kind of local_irq_diasble() to check_flags()
> simplied as below:
>
> local_irq_disable()
> |- trace_hardirqs_off()
> |- trace_hardirqs_off_caller()
> |- preemptirqsoff_hist()
There is no preemptirqsoff_hist() function in the kernel.
-- Steve
> |- __DO_TRACE()
> |- rcu_dereference_sched()
> |- rcu_read_lock_sched_held()
> |- lock_is_held()
> |- check_flags()
>
> The order of calls within trace_hardirqs_off() can be simplified as below:
>
> local_irq_disable()
> |- raw_local_irq_disable()
> |- trace_hardirqs_off()
> |- trace_hardirqs_off() --> check_flags()
> |- hardirqs_enabled = 0
>
> We could find that the call to check_flags() has been made between disabling
> hardirqs and clearing hardirqs_enabled. That will definitely cause an
> "unannotated irqs-off" warning.
>
> To fix this "unannotated irqs-off" warning, we must call trace_hardirqs_off()
> after that we clear hardirqs_enabled.
>
> Signed-off-by: Qi Hou <qi.hou@windriver.com>
> ---
> kernel/locking/lockdep.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index 577f026..e8b35e4 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -2627,8 +2627,6 @@ __visible void trace_hardirqs_off_caller(unsigned long ip)
> {
> struct task_struct *curr = current;
>
> - time_hardirqs_off(CALLER_ADDR0, ip);
> -
> if (unlikely(!debug_locks || current->lockdep_recursion))
> return;
>
> @@ -2649,6 +2647,8 @@ __visible void trace_hardirqs_off_caller(unsigned long ip)
> debug_atomic_inc(hardirqs_off_events);
> } else
> debug_atomic_inc(redundant_hardirqs_off);
> +
> + time_hardirqs_off(CALLER_ADDR0, ip);
> }
> EXPORT_SYMBOL(trace_hardirqs_off_caller);
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] lockdep: call trace_hardirqs_off after clearing hardirqs_enabled
2017-03-13 14:05 ` Steven Rostedt
@ 2017-03-14 1:24 ` qhou
0 siblings, 0 replies; 4+ messages in thread
From: qhou @ 2017-03-14 1:24 UTC (permalink / raw)
To: Steven Rostedt
Cc: mingo, tglx, linux-kernel, C.Emde, paul.gortmaker, bruce.ashfield
On 2017年03月13日 22:05, Steven Rostedt wrote:
> On Mon, 13 Mar 2017 18:08:48 +0800
> Qi Hou <qi.hou@windriver.com> wrote:
>
>> The current order of calls within trace_hardirqs_off() would provoke an
>> "unannotated irqs-off" WARNING.
>>
>> This warning was reported by check_flags() when it found that the hardirqs has
>> been disabled but the irq-flags state, hardirqs_enabled, has not been cleared.
>>
>> Check_flags() is called via trace_hardirqs_off(), and trace_hardirqs_off() will
>> be called by functions kind of local_irq_disable() if CONFIG_TRACE_IRQFLAGS has
>> been enabled.
>>
>> The calltrace from functions kind of local_irq_diasble() to check_flags()
>> simplied as below:
>>
>> local_irq_disable()
>> |- trace_hardirqs_off()
>> |- trace_hardirqs_off_caller()
>> |- preemptirqsoff_hist()
> There is no preemptirqsoff_hist() function in the kernel.
Sorry, it's a mistake. It should have been
"trace_hardirqs_off_caller()", defined in file
kernel/locking/lockdep.c.
And it will call trace_preemptirqsoff_hist_rcuidle(),
trace_preemptirqsoff_hist_rcuidle will()
call check_flags().
I will correct it. And after carefully checking the patch, I will
re-send it.
Very very sorry for my mistake. That wasted your time.
---
Best regards,
Qi Hou
>
> -- Steve
>
>> |- __DO_TRACE()
>> |- rcu_dereference_sched()
>> |- rcu_read_lock_sched_held()
>> |- lock_is_held()
>> |- check_flags()
>>
>> The order of calls within trace_hardirqs_off() can be simplified as below:
>>
>> local_irq_disable()
>> |- raw_local_irq_disable()
>> |- trace_hardirqs_off()
>> |- trace_hardirqs_off() --> check_flags()
>> |- hardirqs_enabled = 0
>>
>> We could find that the call to check_flags() has been made between disabling
>> hardirqs and clearing hardirqs_enabled. That will definitely cause an
>> "unannotated irqs-off" warning.
>>
>> To fix this "unannotated irqs-off" warning, we must call trace_hardirqs_off()
>> after that we clear hardirqs_enabled.
>>
>> Signed-off-by: Qi Hou <qi.hou@windriver.com>
>> ---
>> kernel/locking/lockdep.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
>> index 577f026..e8b35e4 100644
>> --- a/kernel/locking/lockdep.c
>> +++ b/kernel/locking/lockdep.c
>> @@ -2627,8 +2627,6 @@ __visible void trace_hardirqs_off_caller(unsigned long ip)
>> {
>> struct task_struct *curr = current;
>>
>> - time_hardirqs_off(CALLER_ADDR0, ip);
>> -
>> if (unlikely(!debug_locks || current->lockdep_recursion))
>> return;
>>
>> @@ -2649,6 +2647,8 @@ __visible void trace_hardirqs_off_caller(unsigned long ip)
>> debug_atomic_inc(hardirqs_off_events);
>> } else
>> debug_atomic_inc(redundant_hardirqs_off);
>> +
>> + time_hardirqs_off(CALLER_ADDR0, ip);
>> }
>> EXPORT_SYMBOL(trace_hardirqs_off_caller);
>>
--
Best regards,
Qi Hou
Phone number: +86-10-8477-8608
Address: Floor 15, Building B, Wangjing Plaza, No.9 Zhong-Huan Nanlu, Chaoyang District
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] lockdep: call trace_hardirqs_off after clearing hardirqs_enabled
2017-03-13 10:08 [PATCH] lockdep: call trace_hardirqs_off after clearing hardirqs_enabled Qi Hou
2017-03-13 14:05 ` Steven Rostedt
@ 2017-03-13 14:37 ` Peter Zijlstra
1 sibling, 0 replies; 4+ messages in thread
From: Peter Zijlstra @ 2017-03-13 14:37 UTC (permalink / raw)
To: Qi Hou
Cc: mingo, rostedt, tglx, linux-kernel, C.Emde, paul.gortmaker,
bruce.ashfield
Thanks for Cc'ing me.
On Mon, Mar 13, 2017 at 06:08:48PM +0800, Qi Hou wrote:
> The current order of calls within trace_hardirqs_off() would provoke an
> "unannotated irqs-off" WARNING.
>
> This warning was reported by check_flags() when it found that the hardirqs has
> been disabled but the irq-flags state, hardirqs_enabled, has not been cleared.
>
> Check_flags() is called via trace_hardirqs_off(), and trace_hardirqs_off() will
> be called by functions kind of local_irq_disable() if CONFIG_TRACE_IRQFLAGS has
> been enabled.
>
> The calltrace from functions kind of local_irq_diasble() to check_flags()
> simplied as below:
>
> local_irq_disable()
> |- trace_hardirqs_off()
> |- trace_hardirqs_off_caller()
> |- preemptirqsoff_hist()
> |- __DO_TRACE()
> |- rcu_dereference_sched()
> |- rcu_read_lock_sched_held()
> |- lock_is_held()
> |- check_flags()
>
> The order of calls within trace_hardirqs_off() can be simplified as below:
>
> local_irq_disable()
> |- raw_local_irq_disable()
> |- trace_hardirqs_off()
> |- trace_hardirqs_off() --> check_flags()
> |- hardirqs_enabled = 0
>
> We could find that the call to check_flags() has been made between disabling
> hardirqs and clearing hardirqs_enabled. That will definitely cause an
> "unannotated irqs-off" warning.
>
> To fix this "unannotated irqs-off" warning, we must call trace_hardirqs_off()
> after that we clear hardirqs_enabled.
Here you say: "trace_hardirqs_off()"
> Signed-off-by: Qi Hou <qi.hou@windriver.com>
> ---
> kernel/locking/lockdep.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index 577f026..e8b35e4 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -2627,8 +2627,6 @@ __visible void trace_hardirqs_off_caller(unsigned long ip)
> {
> struct task_struct *curr = current;
>
> - time_hardirqs_off(CALLER_ADDR0, ip);
This is: "TIME_hardirqs_off()", notice the difference?
Care to try to write a changelog that makes sense?
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-14 1:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-13 10:08 [PATCH] lockdep: call trace_hardirqs_off after clearing hardirqs_enabled Qi Hou
2017-03-13 14:05 ` Steven Rostedt
2017-03-14 1:24 ` qhou
2017-03-13 14:37 ` Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).