As the documentation explained, ftrace_test_recursion_trylock()
and ftrace_test_recursion_unlock() were supposed to disable and
enable preemption properly, however currently this work is done
outside of the function, which could be missing by mistake.
This path will make sure the preemption will b
With CONFIG_DEBUG_PREEMPT we observed reports like:
BUG: using smp_processor_id() in preemptible
caller is perf_ftrace_function_call+0x6f/0x2e0
CPU: 1 PID: 680 Comm: a.out Not tainted
Call Trace:
dump_stack_lvl+0x8d/0xcf
check_preemption_disabled+0x104/0x110
? optimize_nops.is
On 2021/10/12 下午1:39, 王贇 wrote:
> The testing show that perf_ftrace_function_call() are using
> smp_processor_id() with preemption enabled, all the checking
> on CPU could be wrong after preemption, PATCH 1/2 will fix
> that.
2/2 actually.
>
> Besides, as Peter point o
The testing show that perf_ftrace_function_call() are using
smp_processor_id() with preemption enabled, all the checking
on CPU could be wrong after preemption, PATCH 1/2 will fix
that.
Besides, as Peter point out, the testing of recursion within
the section between ftrace_test_recursion_trylock()
On 2021/10/12 下午7:20, Peter Zijlstra wrote:
> On Tue, Oct 12, 2021 at 01:40:31PM +0800, 王贇 wrote:
>
>> diff --git a/kernel/trace/trace_event_perf.c
>> b/kernel/trace/trace_event_perf.c
>> index 6aed10e..33c2f76 100644
>> --- a/kernel/trace/trace_even
On 2021/10/12 下午8:17, Steven Rostedt wrote:
> On Tue, 12 Oct 2021 13:40:08 +0800
> 王贇 wrote:
>
>> @@ -52,11 +52,6 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>> bit = ftrace_test_recursion_trylock(ip, parent_ip);
>> if (WARN_ON_ONCE(bit &
On 2021/10/12 下午8:24, Miroslav Benes wrote:
[snip]
>>
>> func = list_first_or_null_rcu(&ops->func_stack, struct klp_func,
>>stack_node);
>> @@ -120,7 +115,6 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>> klp_arch_set_pc(fregs, (unsign
On 2021/10/12 下午8:29, Steven Rostedt wrote:
> On Tue, 12 Oct 2021 14:24:43 +0200 (CEST)
> Miroslav Benes wrote:
>
>>> +++ b/kernel/livepatch/patch.c
>>> @@ -52,11 +52,6 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>>> bit = ftrace_test_recursion_trylock(ip, parent_ip);
>>>
On 2021/10/12 下午8:43, Steven Rostedt wrote:
> On Tue, 12 Oct 2021 13:40:08 +0800
> 王贇 wrote:
>
>> --- a/include/linux/trace_recursion.h
>> +++ b/include/linux/trace_recursion.h
>> @@ -214,7 +214,14 @@ static __always_inline void trace_clear_recursion(int
>>
On 2021/10/13 上午10:27, Steven Rostedt wrote:
> On Wed, 13 Oct 2021 09:50:17 +0800
> 王贇 wrote:
>
>>>> - preempt_enable_notrace();
>>>>ftrace_test_recursion_unlock(bit);
>>>> }
>>>
>>> I don't like this change much. We
On 2021/10/13 上午10:30, Steven Rostedt wrote:
> On Wed, 13 Oct 2021 10:04:52 +0800
> 王贇 wrote:
>
>> I see, while the user can still check smp_processor_id() after trylock
>> return bit 0...
>
> But preemption would have already been disabled. That's because
The testing show that perf_ftrace_function_call() are using smp_processor_id()
with preemption enabled, all the checking on CPU could be wrong after
preemption.
As Peter point out, the section between ftrace_test_recursion_trylock/unlock()
pair require the preemption to be disabled as
'Documenta
As the documentation explained, ftrace_test_recursion_trylock()
and ftrace_test_recursion_unlock() were supposed to disable and
enable preemption properly, however currently this work is done
outside of the function, which could be missing by mistake.
This path will make sure the preemption was di
With CONFIG_DEBUG_PREEMPT we observed reports like:
BUG: using smp_processor_id() in preemptible
caller is perf_ftrace_function_call+0x6f/0x2e0
CPU: 1 PID: 680 Comm: a.out Not tainted
Call Trace:
dump_stack_lvl+0x8d/0xcf
check_preemption_disabled+0x104/0x110
? optimize_nops.is
a5e-2bb9-5dbc2eda3...@linux.alibaba.com/
Ok, I'll resend it with link then.
Regards,
Michael Wang
>
> -- Steve
>
>
> On Wed, 13 Oct 2021 11:16:56 +0800
> 王贇 wrote:
>
>> The testing show that perf_ftrace_function_call() are using
>> smp_processor_id()
>
The testing show that perf_ftrace_function_call() are using smp_processor_id()
with preemption enabled, all the checking on CPU could be wrong after
preemption.
As Peter point out, the section between ftrace_test_recursion_trylock/unlock()
pair require the preemption to be disabled as
'Documenta
As the documentation explained, ftrace_test_recursion_trylock()
and ftrace_test_recursion_unlock() were supposed to disable and
enable preemption properly, however currently this work is done
outside of the function, which could be missing by mistake.
This path will make sure the preemption was di
With CONFIG_DEBUG_PREEMPT we observed reports like:
BUG: using smp_processor_id() in preemptible
caller is perf_ftrace_function_call+0x6f/0x2e0
CPU: 1 PID: 680 Comm: a.out Not tainted
Call Trace:
dump_stack_lvl+0x8d/0xcf
check_preemption_disabled+0x104/0x110
? optimize_nops.is
On 2021/10/13 下午3:55, Miroslav Benes wrote:
>> diff --git a/include/linux/trace_recursion.h
>> b/include/linux/trace_recursion.h
>> index a9f9c57..101e1fb 100644
>> --- a/include/linux/trace_recursion.h
>> +++ b/include/linux/trace_recursion.h
>> @@ -208,13 +208,29 @@ static __always_inline voi
On 2021/10/13 下午4:25, Miroslav Benes wrote:
>>> Side note... the comment will eventually conflict with peterz's
>>> https://lore.kernel.org/all/20210929152429.125997...@infradead.org/.
>>
>> Steven, would you like to share your opinion on this patch?
>>
>> If klp_synchronize_transition() will b
The testing show that perf_ftrace_function_call() are using smp_processor_id()
with preemption enabled, all the checking on CPU could be wrong after
preemption.
As Peter point out, the section between ftrace_test_recursion_trylock/unlock()
pair require the preemption to be disabled as
'Documenta
As the documentation explained, ftrace_test_recursion_trylock()
and ftrace_test_recursion_unlock() were supposed to disable and
enable preemption properly, however currently this work is done
outside of the function, which could be missing by mistake.
This path will make sure the preemption was di
With CONFIG_DEBUG_PREEMPT we observed reports like:
BUG: using smp_processor_id() in preemptible
caller is perf_ftrace_function_call+0x6f/0x2e0
CPU: 1 PID: 680 Comm: a.out Not tainted
Call Trace:
dump_stack_lvl+0x8d/0xcf
check_preemption_disabled+0x104/0x110
? optimize_nops.is
On 2021/10/14 下午5:13, Miroslav Benes wrote:
>> diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
>> index e8029ae..b8d75fb 100644
>> --- a/kernel/livepatch/patch.c
>> +++ b/kernel/livepatch/patch.c
>> @@ -49,14 +49,16 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>>
On 2021/10/14 下午11:14, Petr Mladek wrote:
[snip]
>> -return trace_test_and_set_recursion(ip, parent_ip, TRACE_FTRACE_START,
>> TRACE_FTRACE_MAX);
>> +int bit;
>> +
>> +bit = trace_test_and_set_recursion(ip, parent_ip, TRACE_FTRACE_START,
>> TRACE_FTRACE_MAX);
>> +/*
>> + *
On 2021/10/15 上午11:13, 王贇 wrote:
[snip]
>> # define do_ftrace_record_recursion(ip, pip)do { } while (0)
>> #endif
>>
>> +/*
>> + * trace_test_and_set_recursion() is called on several layers
>> + * of the ftrace code when handling the same ftrac
On 2021/10/15 下午3:28, Petr Mladek wrote:
> On Fri 2021-10-15 11:13:08, 王贇 wrote:
>>
>>
>> On 2021/10/14 下午11:14, Petr Mladek wrote:
>> [snip]
>>>> - return trace_test_and_set_recursion(ip, parent_ip, TRACE_FTRACE_START,
>>>> TRA
The testing show that perf_ftrace_function_call() are using smp_processor_id()
with preemption enabled, all the checking on CPU could be wrong after
preemption.
As Peter point out, the section between ftrace_test_recursion_trylock/unlock()
pair require the preemption to be disabled as
'Documenta
As the documentation explained, ftrace_test_recursion_trylock()
and ftrace_test_recursion_unlock() were supposed to disable and
enable preemption properly, however currently this work is done
outside of the function, which could be missing by mistake.
And since the internal using of trace_test_and
With CONFIG_DEBUG_PREEMPT we observed reports like:
BUG: using smp_processor_id() in preemptible
caller is perf_ftrace_function_call+0x6f/0x2e0
CPU: 1 PID: 680 Comm: a.out Not tainted
Call Trace:
dump_stack_lvl+0x8d/0xcf
check_preemption_disabled+0x104/0x110
? optimize_nops.is
Just a ping, to see if there are any more comments :-P
Regards,
Michael Wang
On 2021/10/18 上午11:38, 王贇 wrote:
> The testing show that perf_ftrace_function_call() are using smp_processor_id()
> with preemption enabled, all the checking on CPU could be wrong after
> preemption.
>
On 2021/10/26 上午10:42, Steven Rostedt wrote:
> On Tue, 26 Oct 2021 10:09:12 +0800
> 王贇 wrote:
>
>> Just a ping, to see if there are any more comments :-P
>
> I guess you missed what went into mainline (and your name found a bug
> in my perl script for importing
The testing show that perf_ftrace_function_call() are using smp_processor_id()
with preemption enabled, all the checking on CPU could be wrong after
preemption.
As Peter point out, the section between ftrace_test_recursion_trylock/unlock()
pair require the preemption to be disabled as
'Documenta
As the documentation explained, ftrace_test_recursion_trylock()
and ftrace_test_recursion_unlock() were supposed to disable and
enable preemption properly, however currently this work is done
outside of the function, which could be missing by mistake.
And since the internal using of trace_test_and
With CONFIG_DEBUG_PREEMPT we observed reports like:
BUG: using smp_processor_id() in preemptible
caller is perf_ftrace_function_call+0x6f/0x2e0
CPU: 1 PID: 680 Comm: a.out Not tainted
Call Trace:
dump_stack_lvl+0x8d/0xcf
check_preemption_disabled+0x104/0x110
? optimize_nops.is
Hi, Miroslav
On 2021/10/26 下午5:35, Miroslav Benes wrote:
> Hi,
>
>> diff --git a/include/linux/trace_recursion.h
>> b/include/linux/trace_recursion.h
>> index abe1a50..2bc1522 100644
>> --- a/include/linux/trace_recursion.h
>> +++ b/include/linux/trace_recursion.h
>> @@ -135,6 +135,9 @@ static _
On 2021/10/26 下午8:01, Steven Rostedt wrote:
> On Tue, 26 Oct 2021 17:48:10 +0800
> 王贇 wrote:
>
>>> The two comments should be updated too since Steven removed the "bit == 0"
>>> trick.
>>
>> Could you please give more hint on how will it b
As the documentation explained, ftrace_test_recursion_trylock()
and ftrace_test_recursion_unlock() were supposed to disable and
enable preemption properly, however currently this work is done
outside of the function, which could be missing by mistake.
And since the internal using of trace_test_and
Hi, Steven, Miroslav
Should have fixed the comments about bit value, besides, add
a warn in trace_clear_recursion() to make sure the bit < 0
abusing case will get notified.
Please let me know if there are any other issues :-)
Regards,
Michael Wang
On 2021/10/27 上午10:11, 王贇 wrote:
>
On 2021/10/27 上午10:26, Steven Rostedt wrote:
> On Wed, 27 Oct 2021 09:54:13 +0800
> 王贇 wrote:
>
>> My apologize for the stupid comments... I'll send a v6 for this patch
>> only to fix that, please let me know if this is not a good way to fix
>> few lines of
The testing show that perf_ftrace_function_call() are using smp_processor_id()
with preemption enabled, all the checking on CPU could be wrong after
preemption.
As Peter point out, the section between ftrace_test_recursion_trylock/unlock()
pair require the preemption to be disabled as
'Documenta
As the documentation explained, ftrace_test_recursion_trylock()
and ftrace_test_recursion_unlock() were supposed to disable and
enable preemption properly, however currently this work is done
outside of the function, which could be missing by mistake.
And since the internal using of trace_test_and
With CONFIG_DEBUG_PREEMPT we observed reports like:
BUG: using smp_processor_id() in preemptible
caller is perf_ftrace_function_call+0x6f/0x2e0
CPU: 1 PID: 680 Comm: a.out Not tainted
Call Trace:
dump_stack_lvl+0x8d/0xcf
check_preemption_disabled+0x104/0x110
? optimize_nops.is
On 2021/10/27 上午10:55, Steven Rostedt wrote:
> On Wed, 27 Oct 2021 10:34:13 +0800
> 王贇 wrote:
>
>> +/*
>> + * Preemption will be enabled (if it was previously enabled).
>> + */
>> static __always_inline void trace_clear_recursion(int bit)
>> {
>
The testing show that perf_ftrace_function_call() are using smp_processor_id()
with preemption enabled, all the checking on CPU could be wrong after
preemption.
As Peter point out, the section between ftrace_test_recursion_trylock/unlock()
pair require the preemption to be disabled as
'Documenta
As the documentation explained, ftrace_test_recursion_trylock()
and ftrace_test_recursion_unlock() were supposed to disable and
enable preemption properly, however currently this work is done
outside of the function, which could be missing by mistake.
And since the internal using of trace_test_and
With CONFIG_DEBUG_PREEMPT we observed reports like:
BUG: using smp_processor_id() in preemptible
caller is perf_ftrace_function_call+0x6f/0x2e0
CPU: 1 PID: 680 Comm: a.out Not tainted
Call Trace:
dump_stack_lvl+0x8d/0xcf
check_preemption_disabled+0x104/0x110
? optimize_nops.is
47 matches
Mail list logo
