Hi all,
Today's linux-next build (x86_64 allmodconfig) failed like this:
kernel/trace/trace_functions_graph.c: In function '__trace_graph_entry':
kernel/trace/trace_functions_graph.c:179: error: request for member 'a' in something not a structure or union
kernel/trace/trace_functions_graph.c: In function '__trace_graph_return':
kernel/trace/trace_functions_graph.c:243: error: request for member 'a' in something not a structure or union
Caused by commit 9288f99aa52d90a5b82573c4b769c97c55af2f56 ("this_cpu: Use
this_cpu_xx for ftrace").
I have used the version of the percpu tree from next-20091012 for today.
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/
ftrace_cpu_disabled usage in trace_functions_graph.c were left out
during this_cpu_xx conversion in commit 9288f99a causing compile
failure. Convert them.
Signed-off-by: Tejun Heo <[email protected]>
Reported-by: Stephen Rothwell <[email protected]>
Cc: Christoph Lameter <[email protected]>
---
This patch has been committed to percpu#for-next. Thanks.
kernel/trace/trace_functions_graph.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 45e6c01..90a6daa 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -176,7 +176,7 @@ static int __trace_graph_entry(struct trace_array *tr,
struct ring_buffer *buffer = tr->buffer;
struct ftrace_graph_ent_entry *entry;
- if (unlikely(local_read(&__get_cpu_var(ftrace_cpu_disabled))))
+ if (unlikely(__this_cpu_read(per_cpu_var(ftrace_cpu_disabled))))
return 0;
event = trace_buffer_lock_reserve(buffer, TRACE_GRAPH_ENT,
@@ -240,7 +240,7 @@ static void __trace_graph_return(struct trace_array *tr,
struct ring_buffer *buffer = tr->buffer;
struct ftrace_graph_ret_entry *entry;
- if (unlikely(local_read(&__get_cpu_var(ftrace_cpu_disabled))))
+ if (unlikely(__this_cpu_read(per_cpu_var(ftrace_cpu_disabled))))
return;
event = trace_buffer_lock_reserve(buffer, TRACE_GRAPH_RET,
--
1.6.4.2
On Tue, 13 Oct 2009, Tejun Heo wrote:
> ftrace_cpu_disabled usage in trace_functions_graph.c were left out
> during this_cpu_xx conversion in commit 9288f99a causing compile
> failure. Convert them.
Thanks.
Reviewed-by: Christoph Lameter <[email protected]>
On Tue, 13 Oct 2009, Tejun Heo wrote:
> Oh... one question tho. I used __this_cpu_*() as other conversions
> but I think we should be using the version without the underscores.
> The relationship between get_cpu_var() and __get_cpu_var() is
> different from the one between this_cpu_*() and __this_cpu_*().
For operations like inc/add/dec/sub you need to use the version with __
otherwise the arches that do not support these operations will have to
generate useless expensive code that disables / reenables preempt.
For this_cpu_ptr / __this_cpu_ptr it does not matter. this_cpu_ptr gives
you additional checks.
Christoph Lameter wrote:
> On Tue, 13 Oct 2009, Tejun Heo wrote:
>
>> ftrace_cpu_disabled usage in trace_functions_graph.c were left out
>> during this_cpu_xx conversion in commit 9288f99a causing compile
>> failure. Convert them.
>
> Thanks.
>
> Reviewed-by: Christoph Lameter <[email protected]>
Oh... one question tho. I used __this_cpu_*() as other conversions
but I think we should be using the version without the underscores.
The relationship between get_cpu_var() and __get_cpu_var() is
different from the one between this_cpu_*() and __this_cpu_*().
Thanks.
--
tejun
Christoph Lameter wrote:
> On Tue, 13 Oct 2009, Tejun Heo wrote:
>
>> Oh... one question tho. I used __this_cpu_*() as other conversions
>> but I think we should be using the version without the underscores.
>> The relationship between get_cpu_var() and __get_cpu_var() is
>> different from the one between this_cpu_*() and __this_cpu_*().
>
> For operations like inc/add/dec/sub you need to use the version with __
> otherwise the arches that do not support these operations will have to
> generate useless expensive code that disables / reenables preempt.
>
> For this_cpu_ptr / __this_cpu_ptr it does not matter. this_cpu_ptr gives
> you additional checks.
Yes, you're right. The naming scheme in percpu sucks really hard.
The subtle differences among [__]get_cpu_var(), [__]this_cpu_ptr() and
other this_cpu ops. Arghhhhhh.......
--
tejun
On Tue, 13 Oct 2009, Tejun Heo wrote:
> > For this_cpu_ptr / __this_cpu_ptr it does not matter. this_cpu_ptr gives
> > you additional checks.
>
> Yes, you're right. The naming scheme in percpu sucks really hard.
> The subtle differences among [__]get_cpu_var(), [__]this_cpu_ptr() and
> other this_cpu ops. Arghhhhhh.......
Yeah. __this_cpu_ptr is safe to use in preempt / irq disable sections
though the same way as __this_cpu_add/dec etc.
(__)get_cpu_var can be mostly gotten rid off through __this_cpu
operations.
We could define __get_cpu_var and get_cpu_var using this_cpu
#define get_cpu_var(x) (get_cpu(); this_cpu_read(per_cpu_var(x))
#define __get_cpu_var __this_cpu_read(per_cpu_var(x))