* [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op
@ 2012-02-15 8:54 Ming Lei
2012-02-21 17:30 ` Tejun Heo
0 siblings, 1 reply; 7+ messages in thread
From: Ming Lei @ 2012-02-15 8:54 UTC (permalink / raw)
To: Christoph Lameter, Tejun Heo, Peter Zijlstra; +Cc: linux-kernel, Ming Lei
It doesn't make sense to trace irq off or do irq flags
lock proving inside 'this_cpu' operations, so replace local_irq_*
with raw_local_irq_* in 'this_cpu' op.
Also the patch fixes onelockdep warning[1] by the replacement, see
below:
In commit: 933393f58fef9963eac61db8093689544e29a600(percpu:
Remove irqsafe_cpu_xxx variants), local_irq_save/restore(flags) are
added inside this_cpu_inc operation, so that trace_hardirqs_off_caller
will be called by trace_hardirqs_on_caller directly because
__debug_atomic_inc is implemented as this_cpu_inc, which may trigger
the lockdep warning[1], for example in the below ARM scenary:
kernel_thread_helper /*irq disabled*/
->trace_hardirqs_on_caller /*hardirqs_enabled was set*/
->trace_hardirqs_off_caller /*hardirqs_enabled cleared*/
__this_cpu_add(redundant_hardirqs_on)
->trace_hardirqs_off_caller /*irq disabled, so call here*/
The 'unannotated irqs-on' warning will be triggered somewhere because
irq is just enabled after the irq trace in kernel_thread_helper.
[1],
[ 0.162841] ------------[ cut here ]------------
[ 0.167694] WARNING: at kernel/lockdep.c:3493 check_flags+0xc0/0x1d0()
[ 0.174468] Modules linked in:
[ 0.177703] Backtrace:
[ 0.180328] [<c00171f0>] (dump_backtrace+0x0/0x110) from [<c0412320>] (dump_stack+0x18/0x1c)
[ 0.189086] r6:c051f778 r5:00000da5 r4:00000000 r3:60000093
[ 0.195007] [<c0412308>] (dump_stack+0x0/0x1c) from [<c00410e8>] (warn_slowpath_common+0x54/0x6c)
[ 0.204223] [<c0041094>] (warn_slowpath_common+0x0/0x6c) from [<c0041124>] (warn_slowpath_null+0x24/0x2c)
[ 0.214111] r8:00000000 r7:00000000 r6:ee069598 r5:60000013 r4:ee082000
[ 0.220825] r3:00000009
[ 0.223693] [<c0041100>] (warn_slowpath_null+0x0/0x2c) from [<c0088f38>] (check_flags+0xc0/0x1d0)
[ 0.232910] [<c0088e78>] (check_flags+0x0/0x1d0) from [<c008d348>] (lock_acquire+0x4c/0x11c)
[ 0.241668] [<c008d2fc>] (lock_acquire+0x0/0x11c) from [<c0415aa4>] (_raw_spin_lock+0x3c/0x74)
[ 0.250610] [<c0415a68>] (_raw_spin_lock+0x0/0x74) from [<c010a844>] (set_task_comm+0x20/0xc0)
[ 0.259521] r6:ee069588 r5:ee0691c0 r4:ee082000
[ 0.264404] [<c010a824>] (set_task_comm+0x0/0xc0) from [<c0060780>] (kthreadd+0x28/0x108)
[ 0.272857] r8:00000000 r7:00000013 r6:c0044a08 r5:ee0691c0 r4:ee082000
[ 0.279571] r3:ee083fe0
[ 0.282470] [<c0060758>] (kthreadd+0x0/0x108) from [<c0044a08>] (do_exit+0x0/0x6dc)
[ 0.290405] r5:c0060758 r4:00000000
[ 0.294189] ---[ end trace 1b75b31a2719ed1c ]---
[ 0.299041] possible reason: unannotated irqs-on.
[ 0.303955] irq event stamp: 5
[ 0.307159] hardirqs last enabled at (4): [<c001331c>] no_work_pending+0x8/0x2c
[ 0.314880] hardirqs last disabled at (5): [<c0089b08>] trace_hardirqs_on_caller+0x60/0x26c
[ 0.323547] softirqs last enabled at (0): [<c003f754>] copy_process+0x33c/0xef4
[ 0.331207] softirqs last disabled at (0): [< (null)>] (null)
[ 0.337585] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
Acked-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
include/linux/percpu.h | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/linux/percpu.h b/include/linux/percpu.h
index 32cd1f6..67c8b69 100644
--- a/include/linux/percpu.h
+++ b/include/linux/percpu.h
@@ -348,9 +348,9 @@ do { \
#define _this_cpu_generic_to_op(pcp, val, op) \
do { \
unsigned long flags; \
- local_irq_save(flags); \
+ raw_local_irq_save(flags); \
*__this_cpu_ptr(&(pcp)) op val; \
- local_irq_restore(flags); \
+ raw_local_irq_restore(flags); \
} while (0)
#ifndef this_cpu_write
@@ -449,10 +449,10 @@ do { \
({ \
typeof(pcp) ret__; \
unsigned long flags; \
- local_irq_save(flags); \
+ raw_local_irq_save(flags); \
__this_cpu_add(pcp, val); \
ret__ = __this_cpu_read(pcp); \
- local_irq_restore(flags); \
+ raw_local_irq_restore(flags); \
ret__; \
})
@@ -479,10 +479,10 @@ do { \
#define _this_cpu_generic_xchg(pcp, nval) \
({ typeof(pcp) ret__; \
unsigned long flags; \
- local_irq_save(flags); \
+ raw_local_irq_save(flags); \
ret__ = __this_cpu_read(pcp); \
__this_cpu_write(pcp, nval); \
- local_irq_restore(flags); \
+ raw_local_irq_restore(flags); \
ret__; \
})
@@ -507,11 +507,11 @@ do { \
({ \
typeof(pcp) ret__; \
unsigned long flags; \
- local_irq_save(flags); \
+ raw_local_irq_save(flags); \
ret__ = __this_cpu_read(pcp); \
if (ret__ == (oval)) \
__this_cpu_write(pcp, nval); \
- local_irq_restore(flags); \
+ raw_local_irq_restore(flags); \
ret__; \
})
@@ -544,10 +544,10 @@ do { \
({ \
int ret__; \
unsigned long flags; \
- local_irq_save(flags); \
+ raw_local_irq_save(flags); \
ret__ = __this_cpu_generic_cmpxchg_double(pcp1, pcp2, \
oval1, oval2, nval1, nval2); \
- local_irq_restore(flags); \
+ raw_local_irq_restore(flags); \
ret__; \
})
--
1.7.9
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op
2012-02-15 8:54 [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op Ming Lei
@ 2012-02-21 17:30 ` Tejun Heo
0 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2012-02-21 17:30 UTC (permalink / raw)
To: Ming Lei; +Cc: Christoph Lameter, Peter Zijlstra, linux-kernel
On Wed, Feb 15, 2012 at 04:54:38PM +0800, Ming Lei wrote:
> It doesn't make sense to trace irq off or do irq flags
> lock proving inside 'this_cpu' operations, so replace local_irq_*
> with raw_local_irq_* in 'this_cpu' op.
>
> Also the patch fixes onelockdep warning[1] by the replacement, see
> below:
>
> In commit: 933393f58fef9963eac61db8093689544e29a600(percpu:
> Remove irqsafe_cpu_xxx variants), local_irq_save/restore(flags) are
> added inside this_cpu_inc operation, so that trace_hardirqs_off_caller
> will be called by trace_hardirqs_on_caller directly because
> __debug_atomic_inc is implemented as this_cpu_inc, which may trigger
> the lockdep warning[1], for example in the below ARM scenary:
>
> kernel_thread_helper /*irq disabled*/
> ->trace_hardirqs_on_caller /*hardirqs_enabled was set*/
> ->trace_hardirqs_off_caller /*hardirqs_enabled cleared*/
> __this_cpu_add(redundant_hardirqs_on)
> ->trace_hardirqs_off_caller /*irq disabled, so call here*/
>
> The 'unannotated irqs-on' warning will be triggered somewhere because
> irq is just enabled after the irq trace in kernel_thread_helper.
>
> [1],
> [ 0.162841] ------------[ cut here ]------------
> [ 0.167694] WARNING: at kernel/lockdep.c:3493 check_flags+0xc0/0x1d0()
> [ 0.174468] Modules linked in:
> [ 0.177703] Backtrace:
> [ 0.180328] [<c00171f0>] (dump_backtrace+0x0/0x110) from [<c0412320>] (dump_stack+0x18/0x1c)
> [ 0.189086] r6:c051f778 r5:00000da5 r4:00000000 r3:60000093
> [ 0.195007] [<c0412308>] (dump_stack+0x0/0x1c) from [<c00410e8>] (warn_slowpath_common+0x54/0x6c)
> [ 0.204223] [<c0041094>] (warn_slowpath_common+0x0/0x6c) from [<c0041124>] (warn_slowpath_null+0x24/0x2c)
> [ 0.214111] r8:00000000 r7:00000000 r6:ee069598 r5:60000013 r4:ee082000
> [ 0.220825] r3:00000009
> [ 0.223693] [<c0041100>] (warn_slowpath_null+0x0/0x2c) from [<c0088f38>] (check_flags+0xc0/0x1d0)
> [ 0.232910] [<c0088e78>] (check_flags+0x0/0x1d0) from [<c008d348>] (lock_acquire+0x4c/0x11c)
> [ 0.241668] [<c008d2fc>] (lock_acquire+0x0/0x11c) from [<c0415aa4>] (_raw_spin_lock+0x3c/0x74)
> [ 0.250610] [<c0415a68>] (_raw_spin_lock+0x0/0x74) from [<c010a844>] (set_task_comm+0x20/0xc0)
> [ 0.259521] r6:ee069588 r5:ee0691c0 r4:ee082000
> [ 0.264404] [<c010a824>] (set_task_comm+0x0/0xc0) from [<c0060780>] (kthreadd+0x28/0x108)
> [ 0.272857] r8:00000000 r7:00000013 r6:c0044a08 r5:ee0691c0 r4:ee082000
> [ 0.279571] r3:ee083fe0
> [ 0.282470] [<c0060758>] (kthreadd+0x0/0x108) from [<c0044a08>] (do_exit+0x0/0x6dc)
> [ 0.290405] r5:c0060758 r4:00000000
> [ 0.294189] ---[ end trace 1b75b31a2719ed1c ]---
> [ 0.299041] possible reason: unannotated irqs-on.
> [ 0.303955] irq event stamp: 5
> [ 0.307159] hardirqs last enabled at (4): [<c001331c>] no_work_pending+0x8/0x2c
> [ 0.314880] hardirqs last disabled at (5): [<c0089b08>] trace_hardirqs_on_caller+0x60/0x26c
> [ 0.323547] softirqs last enabled at (0): [<c003f754>] copy_process+0x33c/0xef4
> [ 0.331207] softirqs last disabled at (0): [< (null)>] (null)
> [ 0.337585] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
>
> Acked-by: Christoph Lameter <cl@linux.com>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Applied to for-3.3-fixes. Thank you.
--
tejun
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op
@ 2012-02-15 4:36 Ming Lei
2012-02-15 4:59 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Ming Lei @ 2012-02-15 4:36 UTC (permalink / raw)
To: Christoph Lameter, Tejun Heo, Peter Zijlstra; +Cc: linux-kernel, Ming Lei
It doesn't make sense to trace irq off or do irq flags
lock proving inside 'this_cpu' operations, so replace local_irq_*
with raw_local_irq_* in 'this_cpu' op.
Also the patch fixes onelockdep warning[1] by the replacement, see
below:
In commit: 933393f58fef9963eac61db8093689544e29a600(percpu:
Remove irqsafe_cpu_xxx variants), local_irq_save/restore(flags) are
added inside this_cpu_inc operation, so that trace_hardirqs_off_caller
will be called by trace_hardirqs_on_caller directly because
__debug_atomic_inc is implemented as this_cpu_inc, which may trigger
the lockdep warning[1], for example in the below ARM scenary:
kernel_thread_helper /*irq disabled*/
->trace_hardirqs_on_caller /*hardirqs_enabled was set*/
->trace_hardirqs_off_caller /*hardirqs_enabled cleared*/
__this_cpu_add(redundant_hardirqs_on)
->trace_hardirqs_off_caller /*irq disabled, so call here*/
The 'unannotated irqs-on' warning will be triggered somewhere because
irq is just enabled after the irq trace in kernel_thread_helper.
[1],
[ 0.162841] ------------[ cut here ]------------
[ 0.167694] WARNING: at kernel/lockdep.c:3493 check_flags+0xc0/0x1d0()
[ 0.174468] Modules linked in:
[ 0.177703] Backtrace:
[ 0.180328] [<c00171f0>] (dump_backtrace+0x0/0x110) from [<c0412320>] (dump_stack+0x18/0x1c)
[ 0.189086] r6:c051f778 r5:00000da5 r4:00000000 r3:60000093
[ 0.195007] [<c0412308>] (dump_stack+0x0/0x1c) from [<c00410e8>] (warn_slowpath_common+0x54/0x6c)
[ 0.204223] [<c0041094>] (warn_slowpath_common+0x0/0x6c) from [<c0041124>] (warn_slowpath_null+0x24/0x2c)
[ 0.214111] r8:00000000 r7:00000000 r6:ee069598 r5:60000013 r4:ee082000
[ 0.220825] r3:00000009
[ 0.223693] [<c0041100>] (warn_slowpath_null+0x0/0x2c) from [<c0088f38>] (check_flags+0xc0/0x1d0)
[ 0.232910] [<c0088e78>] (check_flags+0x0/0x1d0) from [<c008d348>] (lock_acquire+0x4c/0x11c)
[ 0.241668] [<c008d2fc>] (lock_acquire+0x0/0x11c) from [<c0415aa4>] (_raw_spin_lock+0x3c/0x74)
[ 0.250610] [<c0415a68>] (_raw_spin_lock+0x0/0x74) from [<c010a844>] (set_task_comm+0x20/0xc0)
[ 0.259521] r6:ee069588 r5:ee0691c0 r4:ee082000
[ 0.264404] [<c010a824>] (set_task_comm+0x0/0xc0) from [<c0060780>] (kthreadd+0x28/0x108)
[ 0.272857] r8:00000000 r7:00000013 r6:c0044a08 r5:ee0691c0 r4:ee082000
[ 0.279571] r3:ee083fe0
[ 0.282470] [<c0060758>] (kthreadd+0x0/0x108) from [<c0044a08>] (do_exit+0x0/0x6dc)
[ 0.290405] r5:c0060758 r4:00000000
[ 0.294189] ---[ end trace 1b75b31a2719ed1c ]---
[ 0.299041] possible reason: unannotated irqs-on.
[ 0.303955] irq event stamp: 5
[ 0.307159] hardirqs last enabled at (4): [<c001331c>] no_work_pending+0x8/0x2c
[ 0.314880] hardirqs last disabled at (5): [<c0089b08>] trace_hardirqs_on_caller+0x60/0x26c
[ 0.323547] softirqs last enabled at (0): [<c003f754>] copy_process+0x33c/0xef4
[ 0.331207] softirqs last disabled at (0): [< (null)>] (null)
[ 0.337585] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
Acked-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
include/linux/percpu.h | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/linux/percpu.h b/include/linux/percpu.h
index 6e68d05..5ed1e38 100644
--- a/include/linux/percpu.h
+++ b/include/linux/percpu.h
@@ -294,9 +294,9 @@ do { \
#define _this_cpu_generic_to_op(pcp, val, op) \
do { \
unsigned long flags; \
- local_irq_save(flags); \
+ raw_local_irq_save(flags); \
*__this_cpu_ptr(&(pcp)) op val; \
- local_irq_restore(flags); \
+ raw_local_irq_restore(flags); \
} while (0)
#ifndef this_cpu_write
@@ -395,10 +395,10 @@ do { \
({ \
typeof(pcp) ret__; \
unsigned long flags; \
- local_irq_save(flags); \
+ raw_local_irq_save(flags); \
__this_cpu_add(pcp, val); \
ret__ = __this_cpu_read(pcp); \
- local_irq_restore(flags); \
+ raw_local_irq_restore(flags); \
ret__; \
})
@@ -425,10 +425,10 @@ do { \
#define _this_cpu_generic_xchg(pcp, nval) \
({ typeof(pcp) ret__; \
unsigned long flags; \
- local_irq_save(flags); \
+ raw_local_irq_save(flags); \
ret__ = __this_cpu_read(pcp); \
__this_cpu_write(pcp, nval); \
- local_irq_restore(flags); \
+ raw_local_irq_restore(flags); \
ret__; \
})
@@ -453,11 +453,11 @@ do { \
({ \
typeof(pcp) ret__; \
unsigned long flags; \
- local_irq_save(flags); \
+ raw_local_irq_save(flags); \
ret__ = __this_cpu_read(pcp); \
if (ret__ == (oval)) \
__this_cpu_write(pcp, nval); \
- local_irq_restore(flags); \
+ raw_local_irq_restore(flags); \
ret__; \
})
@@ -490,10 +490,10 @@ do { \
({ \
int ret__; \
unsigned long flags; \
- local_irq_save(flags); \
+ raw_local_irq_save(flags); \
ret__ = __this_cpu_generic_cmpxchg_double(pcp1, pcp2, \
oval1, oval2, nval1, nval2); \
- local_irq_restore(flags); \
+ raw_local_irq_restore(flags); \
ret__; \
})
--
1.7.9
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op
2012-02-15 4:36 Ming Lei
@ 2012-02-15 4:59 ` Eric Dumazet
2012-02-15 5:08 ` Ming Lei
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2012-02-15 4:59 UTC (permalink / raw)
To: Ming Lei; +Cc: Christoph Lameter, Tejun Heo, Peter Zijlstra, linux-kernel
Le mercredi 15 février 2012 à 12:36 +0800, Ming Lei a écrit :
> It doesn't make sense to trace irq off or do irq flags
> lock proving inside 'this_cpu' operations, so replace local_irq_*
> with raw_local_irq_* in 'this_cpu' op.
...
> Acked-by: Christoph Lameter <cl@linux.com>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
> include/linux/percpu.h | 20 ++++++++++----------
> 1 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/percpu.h b/include/linux/percpu.h
> index 6e68d05..5ed1e38 100644
> --- a/include/linux/percpu.h
> +++ b/include/linux/percpu.h
> @@ -294,9 +294,9 @@ do { \
> #define _this_cpu_generic_to_op(pcp, val, op) \
> do { \
> unsigned long flags; \
> - local_irq_save(flags); \
> + raw_local_irq_save(flags); \
> *__this_cpu_ptr(&(pcp)) op val; \
> - local_irq_restore(flags); \
> + raw_local_irq_restore(flags); \
> } while (0)
>
Could you check the alignement of trailing '\' ?
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op
2012-02-15 4:59 ` Eric Dumazet
@ 2012-02-15 5:08 ` Ming Lei
2012-02-15 6:03 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Ming Lei @ 2012-02-15 5:08 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Christoph Lameter, Tejun Heo, Peter Zijlstra, linux-kernel
On Wed, Feb 15, 2012 at 12:59 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mercredi 15 février 2012 à 12:36 +0800, Ming Lei a écrit :
>> It doesn't make sense to trace irq off or do irq flags
>> lock proving inside 'this_cpu' operations, so replace local_irq_*
>> with raw_local_irq_* in 'this_cpu' op.
> ...
>> Acked-by: Christoph Lameter <cl@linux.com>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>> ---
>> include/linux/percpu.h | 20 ++++++++++----------
>> 1 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/linux/percpu.h b/include/linux/percpu.h
>> index 6e68d05..5ed1e38 100644
>> --- a/include/linux/percpu.h
>> +++ b/include/linux/percpu.h
>> @@ -294,9 +294,9 @@ do { \
>> #define _this_cpu_generic_to_op(pcp, val, op) \
>> do { \
>> unsigned long flags; \
>> - local_irq_save(flags); \
>> + raw_local_irq_save(flags); \
>> *__this_cpu_ptr(&(pcp)) op val; \
>> - local_irq_restore(flags); \
>> + raw_local_irq_restore(flags); \
>> } while (0)
>>
>
> Could you check the alignement of trailing '\' ?
Just several warnings, no errors. Also the 5 warnings are introduced
by previous commit, sounds nothing to do with motivation of this patch.
You may prepare a standalone patch to fix all the code style in this file, :-)
./script/checkpatch.pl
WARNING: line over 80 characters
......
total: 0 errors, 5 warnings, 60 lines checked
thanks,
--
Ming Lei
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op
2012-02-15 5:08 ` Ming Lei
@ 2012-02-15 6:03 ` Eric Dumazet
2012-02-15 8:56 ` Ming Lei
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2012-02-15 6:03 UTC (permalink / raw)
To: Ming Lei; +Cc: Christoph Lameter, Tejun Heo, Peter Zijlstra, linux-kernel
Le mercredi 15 février 2012 à 13:08 +0800, Ming Lei a écrit :
> Just several warnings, no errors. Also the 5 warnings are introduced
> by previous commit, sounds nothing to do with motivation of this patch.
> You may prepare a standalone patch to fix all the code style in this file, :-)
>
Oh well, I dont use checkpatch but spoke about trailing \
Cant you see them on your screen ?
We had :
#define _this_cpu_generic_to_op(pcp, val, op) \
do { \
unsigned long flags; \
local_irq_save(flags); \
*__this_cpu_ptr(&(pcp)) op val; \
local_irq_restore(flags); \
} while (0)
You want :
#define _this_cpu_generic_to_op(pcp, val, op) \
do { \
unsigned long flags; \
raw_local_irq_save(flags); \
*__this_cpu_ptr(&(pcp)) op val; \
raw_local_irq_restore(flags); \
} while (0)
I suggest :
#define _this_cpu_generic_to_op(pcp, val, op) \
do { \
unsigned long flags; \
raw_local_irq_save(flags); \
*__this_cpu_ptr(&(pcp)) op val; \
raw_local_irq_restore(flags); \
} while (0)
Is it clear now ?
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op
2012-02-15 6:03 ` Eric Dumazet
@ 2012-02-15 8:56 ` Ming Lei
0 siblings, 0 replies; 7+ messages in thread
From: Ming Lei @ 2012-02-15 8:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Christoph Lameter, Tejun Heo, Peter Zijlstra, linux-kernel
On Wed, Feb 15, 2012 at 2:03 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mercredi 15 février 2012 à 13:08 +0800, Ming Lei a écrit :
>
>> Just several warnings, no errors. Also the 5 warnings are introduced
>> by previous commit, sounds nothing to do with motivation of this patch.
>> You may prepare a standalone patch to fix all the code style in this file, :-)
>>
>
> Oh well, I dont use checkpatch but spoke about trailing \
>
> Cant you see them on your screen ?
>
> We had :
>
> #define _this_cpu_generic_to_op(pcp, val, op) \
> do { \
> unsigned long flags; \
> local_irq_save(flags); \
> *__this_cpu_ptr(&(pcp)) op val; \
> local_irq_restore(flags); \
> } while (0)
>
>
> You want :
>
> #define _this_cpu_generic_to_op(pcp, val, op) \
> do { \
> unsigned long flags; \
> raw_local_irq_save(flags); \
> *__this_cpu_ptr(&(pcp)) op val; \
> raw_local_irq_restore(flags); \
> } while (0)
>
>
> I suggest :
>
> #define _this_cpu_generic_to_op(pcp, val, op) \
> do { \
> unsigned long flags; \
> raw_local_irq_save(flags); \
> *__this_cpu_ptr(&(pcp)) op val; \
> raw_local_irq_restore(flags); \
> } while (0)
>
>
> Is it clear now ?
Thanks for your pointing it out.
Yes, I have updated the patch and sent it out.
thanks,
--
Ming Lei
ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-02-21 17:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-15 8:54 [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op Ming Lei
2012-02-21 17:30 ` Tejun Heo
-- strict thread matches above, loose matches on Subject: below --
2012-02-15 4:36 Ming Lei
2012-02-15 4:59 ` Eric Dumazet
2012-02-15 5:08 ` Ming Lei
2012-02-15 6:03 ` Eric Dumazet
2012-02-15 8:56 ` Ming Lei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox