* [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
[not found] <20131011175518.634285474@linux.com>
@ 2013-10-11 17:54 ` Christoph Lameter
0 siblings, 0 replies; 33+ messages in thread
From: Christoph Lameter @ 2013-10-11 17:54 UTC (permalink / raw)
To: Tejun Heo
Cc: akpm, Steven Rostedt, linux-kernel, Ingo Molnar, Peter Zijlstra,
Thomas Gleixner
V3->V4:
- Drop CONFIG_DEBUG_THIS_CPU_OPERATIONS
- Add support for logging the exact operation that caused the issue.
We define a check function in order to avoid trouble with the
include files. Then the higher level __this_cpu macros are
modified to invoke the check before __this_cpu operation
Signed-off-by: Christoph Lameter <cl@linux.com>
Index: linux/include/linux/percpu.h
===================================================================
--- linux.orig/include/linux/percpu.h 2013-10-10 11:30:17.111743444 -0500
+++ linux/include/linux/percpu.h 2013-10-10 10:30:26.817316787 -0500
@@ -175,6 +175,12 @@ extern phys_addr_t per_cpu_ptr_to_phys(v
extern void __bad_size_call_parameter(void);
+#ifdef CONFIG_DEBUG_PREEMPT
+extern void __this_cpu_preempt_check(const char *op);
+#else
+static inline void __this_cpu_preempt_check(const char *op) { }
+#endif
+
#define __pcpu_size_call_return(stem, variable) \
({ typeof(variable) pscr_ret__; \
__verify_pcpu_ptr(&(variable)); \
@@ -538,7 +544,8 @@ do { \
# ifndef __this_cpu_read_8
# define __this_cpu_read_8(pcp) (*__this_cpu_ptr(&(pcp)))
# endif
-# define __this_cpu_read(pcp) __pcpu_size_call_return(__this_cpu_read_, (pcp))
+# define __this_cpu_read(pcp) \
+ (__this_cpu_preempt_check("read"),__pcpu_size_call_return(__this_cpu_read_, (pcp)))
#endif
#define __this_cpu_generic_to_op(pcp, val, op) \
@@ -559,7 +566,12 @@ do { \
# ifndef __this_cpu_write_8
# define __this_cpu_write_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), =)
# endif
-# define __this_cpu_write(pcp, val) __pcpu_size_call(__this_cpu_write_, (pcp), (val))
+
+# define __this_cpu_write(pcp, val) \
+do { __this_cpu_preempt_check("write"); \
+ __pcpu_size_call(__this_cpu_write_, (pcp), (val)); \
+} while (0)
+
#endif
#ifndef __this_cpu_add
@@ -575,7 +587,12 @@ do { \
# ifndef __this_cpu_add_8
# define __this_cpu_add_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), +=)
# endif
-# define __this_cpu_add(pcp, val) __pcpu_size_call(__this_cpu_add_, (pcp), (val))
+
+# define __this_cpu_add(pcp, val) \
+do { __this_cpu_preempt_check("add"); \
+ __pcpu_size_call(__this_cpu_add_, (pcp), (val)); \
+} while (0)
+
#endif
#ifndef __this_cpu_sub
@@ -603,7 +620,12 @@ do { \
# ifndef __this_cpu_and_8
# define __this_cpu_and_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), &=)
# endif
-# define __this_cpu_and(pcp, val) __pcpu_size_call(__this_cpu_and_, (pcp), (val))
+
+# define __this_cpu_and(pcp, val) \
+do { __this_cpu_preempt_check("and"); \
+ __pcpu_size_call(__this_cpu_and_, (pcp), (val)); \
+} while (0)
+
#endif
#ifndef __this_cpu_or
@@ -619,7 +641,12 @@ do { \
# ifndef __this_cpu_or_8
# define __this_cpu_or_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), |=)
# endif
-# define __this_cpu_or(pcp, val) __pcpu_size_call(__this_cpu_or_, (pcp), (val))
+
+# define __this_cpu_or(pcp, val) \
+do { __this_cpu_preempt_check("or"); \
+ __pcpu_size_call(__this_cpu_or_, (pcp), (val)); \
+} while (0)
+
#endif
#ifndef __this_cpu_xor
@@ -635,7 +662,12 @@ do { \
# ifndef __this_cpu_xor_8
# define __this_cpu_xor_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), ^=)
# endif
-# define __this_cpu_xor(pcp, val) __pcpu_size_call(__this_cpu_xor_, (pcp), (val))
+
+# define __this_cpu_xor(pcp, val) \
+do { __this_cpu_preempt_check("xor"); \
+ __pcpu_size_call(__this_cpu_xor_, (pcp), (val)); \
+} while (0)
+
#endif
#define __this_cpu_generic_add_return(pcp, val) \
@@ -658,7 +690,7 @@ do { \
# define __this_cpu_add_return_8(pcp, val) __this_cpu_generic_add_return(pcp, val)
# endif
# define __this_cpu_add_return(pcp, val) \
- __pcpu_size_call_return2(__this_cpu_add_return_, pcp, val)
+ (__this_cpu_preempt_check("add_return"),__pcpu_size_call_return2(__this_cpu_add_return_, pcp, val))
#endif
#define __this_cpu_sub_return(pcp, val) __this_cpu_add_return(pcp, -(val))
@@ -686,7 +718,7 @@ do { \
# define __this_cpu_xchg_8(pcp, nval) __this_cpu_generic_xchg(pcp, nval)
# endif
# define __this_cpu_xchg(pcp, nval) \
- __pcpu_size_call_return2(__this_cpu_xchg_, (pcp), nval)
+ (__this_cpu_preempt_check("xchg"),__pcpu_size_call_return2(__this_cpu_xchg_, (pcp), nval))
#endif
#define __this_cpu_generic_cmpxchg(pcp, oval, nval) \
@@ -712,7 +744,7 @@ do { \
# define __this_cpu_cmpxchg_8(pcp, oval, nval) __this_cpu_generic_cmpxchg(pcp, oval, nval)
# endif
# define __this_cpu_cmpxchg(pcp, oval, nval) \
- __pcpu_size_call_return2(__this_cpu_cmpxchg_, pcp, oval, nval)
+ (__this_cpu_preempt_check("cmpxchg"),__pcpu_size_call_return2(__this_cpu_cmpxchg_, pcp, oval, nval))
#endif
#define __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
@@ -745,7 +777,7 @@ do { \
__this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
# endif
# define __this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __pcpu_double_call_return_bool(__this_cpu_cmpxchg_double_, (pcp1), (pcp2), (oval1), (oval2), (nval1), (nval2))
+ (__this_cpu_preempt_check("cmpxchg_double"),__pcpu_double_call_return_bool(__this_cpu_cmpxchg_double_, (pcp1), (pcp2), (oval1), (oval2), (nval1), (nval2)))
#endif
/*
Index: linux/lib/smp_processor_id.c
===================================================================
--- linux.orig/lib/smp_processor_id.c 2013-10-10 11:30:17.111743444 -0500
+++ linux/lib/smp_processor_id.c 2013-10-10 10:32:16.046357108 -0500
@@ -7,7 +7,7 @@
#include <linux/kallsyms.h>
#include <linux/sched.h>
-notrace unsigned int debug_smp_processor_id(void)
+notrace static unsigned int check_preemption_disabled(char *what)
{
unsigned long preempt_count = preempt_count();
int this_cpu = raw_smp_processor_id();
@@ -39,9 +39,9 @@ notrace unsigned int debug_smp_processor
if (!printk_ratelimit())
goto out_enable;
- printk(KERN_ERR "BUG: using smp_processor_id() in preemptible [%08x] "
- "code: %s/%d\n",
- preempt_count() - 1, current->comm, current->pid);
+ printk(KERN_ERR "%s in preemptible [%08x] code: %s/%d\n",
+ what, preempt_count() - 1, current->comm, current->pid);
+
print_symbol("caller is %s\n", (long)__builtin_return_address(0));
dump_stack();
@@ -51,5 +51,18 @@ out:
return this_cpu;
}
+notrace unsigned int debug_smp_processor_id(void)
+{
+ return check_preemption_disabled("BUG: using smp_processor_id()");
+}
EXPORT_SYMBOL(debug_smp_processor_id);
+notrace void __this_cpu_preempt_check(const char *op)
+{
+ char text[40];
+
+ snprintf(text, sizeof(text), "__this_cpu_%s operation", op);
+ check_preemption_disabled(text);
+}
+EXPORT_SYMBOL(__this_cpu_preempt_check);
+
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 0/6] percpu: Implement Preemption checks for __this_cpu operations V4b
@ 2013-10-15 17:47 Christoph Lameter
2013-10-15 17:47 ` [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update Christoph Lameter
` (5 more replies)
0 siblings, 6 replies; 33+ messages in thread
From: Christoph Lameter @ 2013-10-15 17:47 UTC (permalink / raw)
To: Tejun Heo
Cc: akpm, rostedt, linux-kernel, Ingo Molnar, Peter Zijlstra,
Thomas Gleixner
** Resending using Comcast instead of Amazon SES.
This patchset introduces preemption checks for __this_cpu operations.
First we add new raw_cpu operations that perform this cpu operations
without preempt checks in the future.
Then those raw_cpu operations are used in a number of locations to avoid
false positives.
The last patch then adds the preemption checks by modifying the
__this_cpu macros in include/linux/percpu.h
V3->v4:
- Drop CONFIG_DEBUG_THIS_CPU_OPERATIONS
- Detail operation triggering the log entry.
- Use quilt 0.60/ [PATCH] prefix.
- Clean up subject lines.
- Include raw_cpu_ops conversion/fixes that were tested on
a Ubuntu 13.04 desktop.
- Traces
V2->V3:
- Subject line in the raw_cpu_ops patch had ; instead of :.
Guess I am getting old.
- Improve descriptions and variable names.
- Run tests again with kvm to verify that it still works.
A) No warnings with just the patches applied
B) Lots of warnings with CONFIG_DEBUG_THIS_CPU_OPERATIONS enabled
C) No warnings with 3 core patches applied that simply convert
__this_cpu operations to raw_cpu_ops.
V1->V2:
- Reuse preemption check logic in lib/smp_processor_id.c
- Add CONFIG_DEBUG_THIS_CPU_OPERATIONS
- Remove conversions to the use of raw_cpu_ops since
these may require some discussion first.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update
2013-10-15 17:47 [PATCH 0/6] percpu: Implement Preemption checks for __this_cpu operations V4b Christoph Lameter
@ 2013-10-15 17:47 ` Christoph Lameter
2013-10-15 18:36 ` Eric Dumazet
2013-10-16 8:35 ` Peter Zijlstra
2013-10-15 17:47 ` [PATCH 2/6] percpu: Add raw_cpu_ops Christoph Lameter
` (4 subsequent siblings)
5 siblings, 2 replies; 33+ messages in thread
From: Christoph Lameter @ 2013-10-15 17:47 UTC (permalink / raw)
To: Tejun Heo
Cc: akpm, rostedt, linux-kernel, Ingo Molnar, Peter Zijlstra,
Thomas Gleixner, Eric Dumazet, David Miller
[-- Attachment #1: fix_snmp --]
[-- Type: text/plain, Size: 2161 bytes --]
ip4_datagram_connect is called with BH processing enabled. Therefore
we cannot use IP_INC_STATS_BH but must use IP_INC_STATS which disables
BH handling before incrementing the counter.
The following trace is triggered without this patch:
[ 9293.806634] __this_cpu_add operation in preemptible [00000000] code: ntpd/2150
[ 9293.813894] caller is __this_cpu_preempt_check+0x38/0x60
[ 9293.813896] CPU: 2 PID: 2150 Comm: ntpd Tainted: GF 3.12.0-rc4+ #178
[ 9293.813897] Hardware name: Hewlett-Packard HP Z230 SFF Workstation/1906, BIOS L51 v01.10 07/05/2013
[ 9293.813898] 0000000000000002 ffff8803f33fddc8 ffffffff816d57c4 ffff8803f33fdfd8
[ 9293.813903] ffff8803f33fddf8 ffffffff8137341c ffff8800366b0380 00000000ffffff9b
[ 9293.813905] ffff8800366b0680 ffffffffffffff9b ffff8803f33fde38 ffffffff81373478
[ 9293.813916] Call Trace:
[ 9293.813920] [<ffffffff816d57c4>] dump_stack+0x4e/0x82
[ 9293.813921] [<ffffffff8137341c>] check_preemption_disabled+0xec/0x110
[ 9293.813923] [<ffffffff81373478>] __this_cpu_preempt_check+0x38/0x60
[ 9293.813926] [<ffffffff8163cd5d>] ip4_datagram_connect+0x2cd/0x2f0
[ 9293.813929] [<ffffffff8164b73e>] inet_dgram_connect+0x2e/0x80
[ 9293.813934] [<ffffffff815c766b>] SYSC_connect+0xdb/0x100
[ 9293.813938] [<ffffffff81006e6f>] ? mask_8259A+0x2f/0x30
[ 9293.813940] [<ffffffff81010505>] ? syscall_trace_enter+0x155/0x270
[ 9293.813942] [<ffffffff815c7b2e>] SyS_connect+0xe/0x10
[ 9293.813944] [<ffffffff816e4d13>] tracesys+0xe1/0xe6
Cc: Eric Dumazet <edumazet@google.com>
Cc: netdev@vger.kernel.org
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Christoph Lameter <cl@linux.com>
Index: linux/net/ipv4/datagram.c
===================================================================
--- linux.orig/net/ipv4/datagram.c 2013-08-08 02:55:00.900983548 -0500
+++ linux/net/ipv4/datagram.c 2013-10-09 15:02:51.999123322 -0500
@@ -57,7 +57,7 @@ int ip4_datagram_connect(struct sock *sk
if (IS_ERR(rt)) {
err = PTR_ERR(rt);
if (err == -ENETUNREACH)
- IP_INC_STATS_BH(sock_net(sk), IPSTATS_MIB_OUTNOROUTES);
+ IP_INC_STATS(sock_net(sk), IPSTATS_MIB_OUTNOROUTES);
goto out;
}
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/6] percpu: Add raw_cpu_ops
2013-10-15 17:47 [PATCH 0/6] percpu: Implement Preemption checks for __this_cpu operations V4b Christoph Lameter
2013-10-15 17:47 ` [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update Christoph Lameter
@ 2013-10-15 17:47 ` Christoph Lameter
2013-10-15 17:47 ` [PATCH 3/6] mm: Use raw_cpu ops for determining current NUMA node Christoph Lameter
` (3 subsequent siblings)
5 siblings, 0 replies; 33+ messages in thread
From: Christoph Lameter @ 2013-10-15 17:47 UTC (permalink / raw)
To: Tejun Heo
Cc: akpm, rostedt, linux-kernel, Ingo Molnar, Peter Zijlstra,
Thomas Gleixner
[-- Attachment #1: raw_cpu_ops --]
[-- Type: text/plain, Size: 4290 bytes --]
The following patches will add preemption checks to __this_cpu ops so we
need to have an alternative way to use this cpu operations without
preemption checks.
raw_cpu ops rely on the __this_cpu_xxx_1/2/4/8 operations having no
preemption checks. We therefore do not have to duplicate these functions
but can affort to only duplicate the higher level macros.
Also add raw_cpu_ptr for symmetries sake.
Signed-off-by: Christoph Lameter <cl@linux.com>
Index: linux/include/linux/percpu.h
===================================================================
--- linux.orig/include/linux/percpu.h 2013-09-23 10:20:05.050535801 -0500
+++ linux/include/linux/percpu.h 2013-09-23 10:20:05.050535801 -0500
@@ -139,6 +139,9 @@ extern int __init pcpu_page_first_chunk(
pcpu_fc_populate_pte_fn_t populate_pte_fn);
#endif
+/* For symmetries sake. */
+#define raw_cpu_ptr __this_cpu_ptr
+
/*
* Use this to get to a cpu's version of the per-cpu object
* dynamically allocated. Non-atomic access to the current CPU's
@@ -520,17 +523,7 @@ do { \
/*
* Generic percpu operations for context that are safe from preemption/interrupts.
- * Either we do not care about races or the caller has the
- * responsibility of handling preemption/interrupt issues. Arch code can still
- * override these instructions since the arch per cpu code may be more
- * efficient and may actually get race freeness for free (that is the
- * case for x86 for example).
- *
- * If there is no other protection through preempt disable and/or
- * disabling interupts then one of these RMW operations can show unexpected
- * behavior because the execution thread was rescheduled on another processor
- * or an interrupt occurred and the same percpu variable was modified from
- * the interrupt context.
+ * These functions verify that preemption has been disabled.
*/
#ifndef __this_cpu_read
# ifndef __this_cpu_read_1
@@ -755,4 +748,74 @@ do { \
__pcpu_double_call_return_bool(__this_cpu_cmpxchg_double_, (pcp1), (pcp2), (oval1), (oval2), (nval1), (nval2))
#endif
+/*
+ * Generic percpu operations for context where we do not want to do
+ * any checks for preemptiosn.
+ *
+ * If there is no other protection through preempt disable and/or
+ * disabling interupts then one of these RMW operations can show unexpected
+ * behavior because the execution thread was rescheduled on another processor
+ * or an interrupt occurred and the same percpu variable was modified from
+ * the interrupt context.
+ */
+#ifndef raw_cpu_read
+# define raw_cpu_read(pcp) __pcpu_size_call_return(__this_cpu_read_, (pcp))
+#endif
+
+#ifndef raw_cpu_write
+# define raw_cpu_write(pcp, val) __pcpu_size_call(__this_cpu_write_, (pcp), (val))
+#endif
+
+#ifndef raw_cpu_add
+# define raw_cpu_add(pcp, val) __pcpu_size_call(__this_cpu_add_, (pcp), (val))
+#endif
+
+#ifndef raw_cpu_sub
+# define raw_cpu_sub(pcp, val) raw_cpu_add((pcp), -(val))
+#endif
+
+#ifndef raw_cpu_inc
+# define raw_cpu_inc(pcp) raw_cpu_add((pcp), 1)
+#endif
+
+#ifndef raw_cpu_dec
+# define raw_cpu_dec(pcp) raw_cpu_sub((pcp), 1)
+#endif
+
+#ifndef raw_cpu_and
+# define raw_cpu_and(pcp, val) __pcpu_size_call(__this_cpu_and_, (pcp), (val))
+#endif
+
+#ifndef raw_cpu_or
+# define raw_cpu_or(pcp, val) __pcpu_size_call(__this_cpu_or_, (pcp), (val))
+#endif
+
+#ifndef raw_cpu_xor
+# define raw_cpu_xor(pcp, val) __pcpu_size_call(__this_cpu_xor_, (pcp), (val))
+#endif
+
+#ifndef raw_cpu_add_return
+# define raw_cpu_add_return(pcp, val) \
+ __pcpu_size_call_return2(__this_cpu_add_return_, pcp, val)
+#endif
+
+#define raw_cpu_sub_return(pcp, val) raw_cpu_add_return(pcp, -(val))
+#define raw_cpu_inc_return(pcp) raw_cpu_add_return(pcp, 1)
+#define raw_cpu_dec_return(pcp) raw_cpu_add_return(pcp, -1)
+
+#ifndef raw_cpu_xchg
+# define raw_cpu_xchg(pcp, nval) \
+ __pcpu_size_call_return2(__this_cpu_xchg_, (pcp), nval)
+#endif
+
+#ifndef raw_cpu_cmpxchg
+# define raw_cpu_cmpxchg(pcp, oval, nval) \
+ __pcpu_size_call_return2(__this_cpu_cmpxchg_, pcp, oval, nval)
+#endif
+
+#ifndef raw_cpu_cmpxchg_double
+# define raw_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ __pcpu_double_call_return_bool(__this_cpu_cmpxchg_double_, (pcp1), (pcp2), (oval1), (oval2), (nval1), (nval2))
+#endif
+
#endif /* __LINUX_PERCPU_H */
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 3/6] mm: Use raw_cpu ops for determining current NUMA node
2013-10-15 17:47 [PATCH 0/6] percpu: Implement Preemption checks for __this_cpu operations V4b Christoph Lameter
2013-10-15 17:47 ` [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update Christoph Lameter
2013-10-15 17:47 ` [PATCH 2/6] percpu: Add raw_cpu_ops Christoph Lameter
@ 2013-10-15 17:47 ` Christoph Lameter
2013-10-16 8:38 ` Peter Zijlstra
2013-10-15 17:47 ` [PATCH 4/6] Use raw_cpu_write for initialization of per cpu refcount Christoph Lameter
` (2 subsequent siblings)
5 siblings, 1 reply; 33+ messages in thread
From: Christoph Lameter @ 2013-10-15 17:47 UTC (permalink / raw)
To: Tejun Heo
Cc: akpm, rostedt, linux-kernel, Ingo Molnar, Peter Zijlstra,
Thomas Gleixner, Alex Shi
[-- Attachment #1: preempt_fix_numa_node --]
[-- Type: text/plain, Size: 4423 bytes --]
With the preempt checking logic for __this_cpu_ops we will get false
positives from locations in the code that use numa_node_id.
Before the __this_cpu ops where introduced there were
no checks for preemption present either. smp_raw_processor_id()
was used. See http://www.spinics.net/lists/linux-numa/msg00641.html
Therefore we need to use raw_cpu_read here to avoid false postives.
Note that this issue has been discussed in prior years.
If the process changes nodes after retrieving the current numa node then
that is acceptable since most uses of numa_node etc are for optimization
and not for correctness.
Some sample traces:
__this_cpu_read operation in preemptible [00000000] code: login/1456
caller is __this_cpu_preempt_check+0x2b/0x2d
CPU: 0 PID: 1456 Comm: login Not tainted 3.12.0-rc4-cl-00062-g2fe80d3-dirty #185
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
000000000000013c ffff88001f31ba58 ffffffff8147cf5e ffff88001f31bfd8
ffff88001f31ba88 ffffffff8127eea9 0000000000000000 ffff88001f3975c0
00000000f7707000 ffff88001f3975c0 ffff88001f31bac0 ffffffff8127eeef
Call Trace:
[<ffffffff8147cf5e>] dump_stack+0x4e/0x82
[<ffffffff8127eea9>] check_preemption_disabled+0xc5/0xe0
[<ffffffff8127eeef>] __this_cpu_preempt_check+0x2b/0x2d
[<ffffffff81030ff5>] ? show_stack+0x3b/0x3d
[<ffffffff810ebee3>] get_task_policy+0x1d/0x49
[<ffffffff810ed705>] get_vma_policy+0x14/0x76
[<ffffffff810ed8ff>] alloc_pages_vma+0x35/0xff
[<ffffffff810dad97>] handle_mm_fault+0x290/0x73b
[<ffffffff810503da>] __do_page_fault+0x3fe/0x44d
[<ffffffff8109b360>] ? trace_hardirqs_on_caller+0x142/0x19e
[<ffffffff8109b3c9>] ? trace_hardirqs_on+0xd/0xf
[<ffffffff81278bed>] ? trace_hardirqs_off_thunk+0x3a/0x3c
[<ffffffff810be97f>] ? find_get_pages_contig+0x18e/0x18e
[<ffffffff810be97f>] ? find_get_pages_contig+0x18e/0x18e
[<ffffffff81050451>] do_page_fault+0x9/0xc
[<ffffffff81483602>] page_fault+0x22/0x30
[<ffffffff810be97f>] ? find_get_pages_contig+0x18e/0x18e
[<ffffffff810be97f>] ? find_get_pages_contig+0x18e/0x18e
[<ffffffff810be4c3>] ? file_read_actor+0x3a/0x15a
[<ffffffff810be97f>] ? find_get_pages_contig+0x18e/0x18e
[<ffffffff810bffab>] generic_file_aio_read+0x38e/0x624
[<ffffffff810f6d69>] do_sync_read+0x54/0x73
[<ffffffff810f7890>] vfs_read+0x9d/0x12a
[<ffffffff810f7a59>] SyS_read+0x47/0x7e
[<ffffffff81484f21>] cstar_dispatch+0x7/0x23
caller is __this_cpu_preempt_check+0x2b/0x2d
CPU: 0 PID: 1456 Comm: login Not tainted 3.12.0-rc4-cl-00062-g2fe80d3-dirty #185
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
00000000000000e8 ffff88001f31bbf8 ffffffff8147cf5e ffff88001f31bfd8
ffff88001f31bc28 ffffffff8127eea9 ffffffff823c5c40 00000000000213da
0000000000000000 0000000000000000 ffff88001f31bc60 ffffffff8127eeef
Call Trace:
[<ffffffff8147cf5e>] dump_stack+0x4e/0x82
[<ffffffff8127eea9>] check_preemption_disabled+0xc5/0xe0
[<ffffffff8127eeef>] __this_cpu_preempt_check+0x2b/0x2d
[<ffffffff810e006e>] ? install_special_mapping+0x11/0xe4
[<ffffffff810ec8a8>] alloc_pages_current+0x8f/0xbc
[<ffffffff810bec6b>] __page_cache_alloc+0xb/0xd
[<ffffffff810c7e90>] __do_page_cache_readahead+0xf4/0x219
[<ffffffff810c7e0e>] ? __do_page_cache_readahead+0x72/0x219
[<ffffffff810c827c>] ra_submit+0x1c/0x20
[<ffffffff810c850c>] ondemand_readahead+0x28c/0x2b4
[<ffffffff810c85e9>] page_cache_sync_readahead+0x38/0x3a
[<ffffffff810bfe7e>] generic_file_aio_read+0x261/0x624
[<ffffffff810f6d69>] do_sync_read+0x54/0x73
[<ffffffff810f7890>] vfs_read+0x9d/0x12a
[<ffffffff810f7a59>] SyS_read+0x47/0x7e
[<ffffffff81484f21>] cstar_dispatch+0x7/0x23
CC: linux-mm@kvack.org
Cc: Alex Shi <alex.shi@intel.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Index: linux/include/linux/topology.h
===================================================================
--- linux.orig/include/linux/topology.h 2013-09-24 11:29:51.000000000 -0500
+++ linux/include/linux/topology.h 2013-09-24 11:30:18.893831971 -0500
@@ -182,7 +182,7 @@ DECLARE_PER_CPU(int, numa_node);
/* Returns the number of the current Node. */
static inline int numa_node_id(void)
{
- return __this_cpu_read(numa_node);
+ return raw_cpu_read(numa_node);
}
#endif
@@ -239,7 +239,7 @@ static inline void set_numa_mem(int node
/* Returns the number of the nearest Node with memory */
static inline int numa_mem_id(void)
{
- return __this_cpu_read(_numa_mem_);
+ return raw_cpu_read(_numa_mem_);
}
#endif
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 4/6] Use raw_cpu_write for initialization of per cpu refcount.
2013-10-15 17:47 [PATCH 0/6] percpu: Implement Preemption checks for __this_cpu operations V4b Christoph Lameter
` (2 preceding siblings ...)
2013-10-15 17:47 ` [PATCH 3/6] mm: Use raw_cpu ops for determining current NUMA node Christoph Lameter
@ 2013-10-15 17:47 ` Christoph Lameter
2013-10-16 8:43 ` Peter Zijlstra
2013-10-15 17:47 ` [PATCH 5/6] net: __this_cpu_inc in route.c Christoph Lameter
2013-10-15 17:47 ` [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops Christoph Lameter
5 siblings, 1 reply; 33+ messages in thread
From: Christoph Lameter @ 2013-10-15 17:47 UTC (permalink / raw)
To: Tejun Heo
Cc: akpm, rostedt, linux-kernel, Ingo Molnar, Peter Zijlstra,
Thomas Gleixner, Rusty Russell
[-- Attachment #1: preempt_module --]
[-- Type: text/plain, Size: 1967 bytes --]
The initialization of a structure is not subject to synchronization.
The use of __this_cpu would trigger a false positive with the
additional preemption checks for __this_cpu ops.
So simply disable the check through the use of raw_cpu ops.
Trace:
[ 0.668066] __this_cpu_write operation in preemptible [00000000] code: modprobe/286
[ 0.668108] caller is __this_cpu_preempt_check+0x38/0x60
[ 0.668111] CPU: 3 PID: 286 Comm: modprobe Tainted: GF 3.12.0-rc4+ #187
[ 0.668112] Hardware name: FUJITSU CELSIUS W530 Power/D3227-A1, BIOS V4.6.5.4 R1.10.0 for D3227-A1x 09/16/2013
[ 0.668113] 0000000000000003 ffff8807edda1d18 ffffffff816d5a57 ffff8807edda1fd8
[ 0.668117] ffff8807edda1d48 ffffffff8137359c ffff8807edda1ef8 ffffffffa0002178
[ 0.668121] ffffc90000067730 ffff8807edda1e48 ffff8807edda1d88 ffffffff813735f8
[ 0.668124] Call Trace:
[ 0.668129] [<ffffffff816d5a57>] dump_stack+0x4e/0x82
[ 0.668132] [<ffffffff8137359c>] check_preemption_disabled+0xec/0x110
[ 0.668135] [<ffffffff813735f8>] __this_cpu_preempt_check+0x38/0x60
[ 0.668139] [<ffffffff810c24fd>] load_module+0xcfd/0x2650
[ 0.668143] [<ffffffff816dd922>] ? page_fault+0x22/0x30
[ 0.668146] [<ffffffff810c3ef6>] SyS_init_module+0xa6/0xd0
[ 0.668150] [<ffffffff816e4fd3>] tracesys+0xe1/0xe6
Cc: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Christoph Lameter <cl@linux.com>
Index: linux/kernel/module.c
===================================================================
--- linux.orig/kernel/module.c 2013-09-05 13:43:30.557687773 -0500
+++ linux/kernel/module.c 2013-10-07 12:33:43.732059759 -0500
@@ -643,7 +643,7 @@ static int module_unload_init(struct mod
INIT_LIST_HEAD(&mod->target_list);
/* Hold reference count during initialization. */
- __this_cpu_write(mod->refptr->incs, 1);
+ raw_cpu_write(mod->refptr->incs, 1);
/* Backwards compatibility macros put refcount during init. */
mod->waiter = current;
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 5/6] net: __this_cpu_inc in route.c
2013-10-15 17:47 [PATCH 0/6] percpu: Implement Preemption checks for __this_cpu operations V4b Christoph Lameter
` (3 preceding siblings ...)
2013-10-15 17:47 ` [PATCH 4/6] Use raw_cpu_write for initialization of per cpu refcount Christoph Lameter
@ 2013-10-15 17:47 ` Christoph Lameter
2013-10-16 8:46 ` Peter Zijlstra
2013-10-15 17:47 ` [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops Christoph Lameter
5 siblings, 1 reply; 33+ messages in thread
From: Christoph Lameter @ 2013-10-15 17:47 UTC (permalink / raw)
To: Tejun Heo
Cc: akpm, rostedt, linux-kernel, Ingo Molnar, Peter Zijlstra,
Thomas Gleixner, Eric Dumazet
[-- Attachment #1: preempt_rt_cache_stat --]
[-- Type: text/plain, Size: 3343 bytes --]
The RT_CACHE_STAT_INC macro triggers the new preemption checks
for __this_cpu ops
I do not see any other synchronization that would allow the use
of a __this_cpu operation here however in commit
dbd2915ce87e811165da0717f8e159276ebb803e Andrew justifies
the use of raw_smp_processor_id() here because "we do not care"
about races.
So lets use raw_cpu ops here and hope for the best. The use of
__this_cpu op improves the situation already from what commit
dbd2915ce87e811165da0717f8e159276ebb803e did since the single instruction
emitted on x86 does not allow the race to occur anymore. However,
non x86 platforms could still experience a race here.
Trace:
[ 1277.189084] __this_cpu_add operation in preemptible [00000000] code: avahi-daemon/1193
[ 1277.189085] caller is __this_cpu_preempt_check+0x38/0x60
[ 1277.189086] CPU: 1 PID: 1193 Comm: avahi-daemon Tainted: GF 3.12.0-rc4+ #187
[ 1277.189087] Hardware name: FUJITSU CELSIUS W530 Power/D3227-A1, BIOS V4.6.5.4 R1.10.0 for D3227-A1x 09/16/2013
[ 1277.189088] 0000000000000001 ffff8807ef78fa00 ffffffff816d5a57 ffff8807ef78ffd8
[ 1277.189089] ffff8807ef78fa30 ffffffff8137359c ffff8807ef78fba0 ffff88079f822b40
[ 1277.189091] 0000000020000000 ffff8807ee32c800 ffff8807ef78fa70 ffffffff813735f8
[ 1277.189093] Call Trace:
[ 1277.189094] [<ffffffff816d5a57>] dump_stack+0x4e/0x82
[ 1277.189096] [<ffffffff8137359c>] check_preemption_disabled+0xec/0x110
[ 1277.189097] [<ffffffff813735f8>] __this_cpu_preempt_check+0x38/0x60
[ 1277.189098] [<ffffffff81610d65>] __ip_route_output_key+0x575/0x8c0
[ 1277.189100] [<ffffffff816110d7>] ip_route_output_flow+0x27/0x70
[ 1277.189101] [<ffffffff81616c80>] ? ip_copy_metadata+0x1a0/0x1a0
[ 1277.189102] [<ffffffff81640b15>] udp_sendmsg+0x825/0xa20
[ 1277.189104] [<ffffffff811b4aa9>] ? do_sys_poll+0x449/0x5d0
[ 1277.189105] [<ffffffff8164c695>] inet_sendmsg+0x85/0xc0
[ 1277.189106] [<ffffffff815c6e3c>] sock_sendmsg+0x9c/0xd0
[ 1277.189108] [<ffffffff813735f8>] ? __this_cpu_preempt_check+0x38/0x60
[ 1277.189109] [<ffffffff815c7550>] ? move_addr_to_kernel+0x40/0xa0
[ 1277.189111] [<ffffffff815c71ec>] ___sys_sendmsg+0x37c/0x390
[ 1277.189112] [<ffffffff8136613a>] ? string.isra.3+0x3a/0xd0
[ 1277.189113] [<ffffffff8136613a>] ? string.isra.3+0x3a/0xd0
[ 1277.189115] [<ffffffff81367b54>] ? vsnprintf+0x364/0x650
[ 1277.189116] [<ffffffff81367ee9>] ? snprintf+0x39/0x40
[ 1277.189118] [<ffffffff813735f8>] ? __this_cpu_preempt_check+0x38/0x60
[ 1277.189119] [<ffffffff815c7ff9>] __sys_sendmsg+0x49/0x90
[ 1277.189121] [<ffffffff815c8052>] SyS_sendmsg+0x12/0x20
[ 1277.189122] [<ffffffff816e4fd3>] tracesys+0xe1/0xe6
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Index: linux/net/ipv4/route.c
===================================================================
--- linux.orig/net/ipv4/route.c 2013-10-07 10:27:25.000000000 -0500
+++ linux/net/ipv4/route.c 2013-10-07 12:41:23.993823743 -0500
@@ -197,7 +197,7 @@ const __u8 ip_tos2prio[16] = {
EXPORT_SYMBOL(ip_tos2prio);
static DEFINE_PER_CPU(struct rt_cache_stat, rt_cache_stat);
-#define RT_CACHE_STAT_INC(field) __this_cpu_inc(rt_cache_stat.field)
+#define RT_CACHE_STAT_INC(field) raw_cpu_inc(rt_cache_stat.field)
#ifdef CONFIG_PROC_FS
static void *rt_cache_seq_start(struct seq_file *seq, loff_t *pos)
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-15 17:47 [PATCH 0/6] percpu: Implement Preemption checks for __this_cpu operations V4b Christoph Lameter
` (4 preceding siblings ...)
2013-10-15 17:47 ` [PATCH 5/6] net: __this_cpu_inc in route.c Christoph Lameter
@ 2013-10-15 17:47 ` Christoph Lameter
2013-10-16 8:49 ` Peter Zijlstra
5 siblings, 1 reply; 33+ messages in thread
From: Christoph Lameter @ 2013-10-15 17:47 UTC (permalink / raw)
To: Tejun Heo
Cc: akpm, rostedt, linux-kernel, Ingo Molnar, Peter Zijlstra,
Thomas Gleixner
[-- Attachment #1: preempt_check_this_cpu_ops --]
[-- Type: text/plain, Size: 6721 bytes --]
V3->V4:
- Drop CONFIG_DEBUG_THIS_CPU_OPERATIONS
- Add support for logging the exact operation that caused the issue.
We define a check function in order to avoid trouble with the
include files. Then the higher level __this_cpu macros are
modified to invoke the check before __this_cpu operation
Signed-off-by: Christoph Lameter <cl@linux.com>
Index: linux/include/linux/percpu.h
===================================================================
--- linux.orig/include/linux/percpu.h 2013-10-10 11:30:17.111743444 -0500
+++ linux/include/linux/percpu.h 2013-10-10 10:30:26.817316787 -0500
@@ -175,6 +175,12 @@ extern phys_addr_t per_cpu_ptr_to_phys(v
extern void __bad_size_call_parameter(void);
+#ifdef CONFIG_DEBUG_PREEMPT
+extern void __this_cpu_preempt_check(const char *op);
+#else
+static inline void __this_cpu_preempt_check(const char *op) { }
+#endif
+
#define __pcpu_size_call_return(stem, variable) \
({ typeof(variable) pscr_ret__; \
__verify_pcpu_ptr(&(variable)); \
@@ -538,7 +544,8 @@ do { \
# ifndef __this_cpu_read_8
# define __this_cpu_read_8(pcp) (*__this_cpu_ptr(&(pcp)))
# endif
-# define __this_cpu_read(pcp) __pcpu_size_call_return(__this_cpu_read_, (pcp))
+# define __this_cpu_read(pcp) \
+ (__this_cpu_preempt_check("read"),__pcpu_size_call_return(__this_cpu_read_, (pcp)))
#endif
#define __this_cpu_generic_to_op(pcp, val, op) \
@@ -559,7 +566,12 @@ do { \
# ifndef __this_cpu_write_8
# define __this_cpu_write_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), =)
# endif
-# define __this_cpu_write(pcp, val) __pcpu_size_call(__this_cpu_write_, (pcp), (val))
+
+# define __this_cpu_write(pcp, val) \
+do { __this_cpu_preempt_check("write"); \
+ __pcpu_size_call(__this_cpu_write_, (pcp), (val)); \
+} while (0)
+
#endif
#ifndef __this_cpu_add
@@ -575,7 +587,12 @@ do { \
# ifndef __this_cpu_add_8
# define __this_cpu_add_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), +=)
# endif
-# define __this_cpu_add(pcp, val) __pcpu_size_call(__this_cpu_add_, (pcp), (val))
+
+# define __this_cpu_add(pcp, val) \
+do { __this_cpu_preempt_check("add"); \
+ __pcpu_size_call(__this_cpu_add_, (pcp), (val)); \
+} while (0)
+
#endif
#ifndef __this_cpu_sub
@@ -603,7 +620,12 @@ do { \
# ifndef __this_cpu_and_8
# define __this_cpu_and_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), &=)
# endif
-# define __this_cpu_and(pcp, val) __pcpu_size_call(__this_cpu_and_, (pcp), (val))
+
+# define __this_cpu_and(pcp, val) \
+do { __this_cpu_preempt_check("and"); \
+ __pcpu_size_call(__this_cpu_and_, (pcp), (val)); \
+} while (0)
+
#endif
#ifndef __this_cpu_or
@@ -619,7 +641,12 @@ do { \
# ifndef __this_cpu_or_8
# define __this_cpu_or_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), |=)
# endif
-# define __this_cpu_or(pcp, val) __pcpu_size_call(__this_cpu_or_, (pcp), (val))
+
+# define __this_cpu_or(pcp, val) \
+do { __this_cpu_preempt_check("or"); \
+ __pcpu_size_call(__this_cpu_or_, (pcp), (val)); \
+} while (0)
+
#endif
#ifndef __this_cpu_xor
@@ -635,7 +662,12 @@ do { \
# ifndef __this_cpu_xor_8
# define __this_cpu_xor_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), ^=)
# endif
-# define __this_cpu_xor(pcp, val) __pcpu_size_call(__this_cpu_xor_, (pcp), (val))
+
+# define __this_cpu_xor(pcp, val) \
+do { __this_cpu_preempt_check("xor"); \
+ __pcpu_size_call(__this_cpu_xor_, (pcp), (val)); \
+} while (0)
+
#endif
#define __this_cpu_generic_add_return(pcp, val) \
@@ -658,7 +690,7 @@ do { \
# define __this_cpu_add_return_8(pcp, val) __this_cpu_generic_add_return(pcp, val)
# endif
# define __this_cpu_add_return(pcp, val) \
- __pcpu_size_call_return2(__this_cpu_add_return_, pcp, val)
+ (__this_cpu_preempt_check("add_return"),__pcpu_size_call_return2(__this_cpu_add_return_, pcp, val))
#endif
#define __this_cpu_sub_return(pcp, val) __this_cpu_add_return(pcp, -(val))
@@ -686,7 +718,7 @@ do { \
# define __this_cpu_xchg_8(pcp, nval) __this_cpu_generic_xchg(pcp, nval)
# endif
# define __this_cpu_xchg(pcp, nval) \
- __pcpu_size_call_return2(__this_cpu_xchg_, (pcp), nval)
+ (__this_cpu_preempt_check("xchg"),__pcpu_size_call_return2(__this_cpu_xchg_, (pcp), nval))
#endif
#define __this_cpu_generic_cmpxchg(pcp, oval, nval) \
@@ -712,7 +744,7 @@ do { \
# define __this_cpu_cmpxchg_8(pcp, oval, nval) __this_cpu_generic_cmpxchg(pcp, oval, nval)
# endif
# define __this_cpu_cmpxchg(pcp, oval, nval) \
- __pcpu_size_call_return2(__this_cpu_cmpxchg_, pcp, oval, nval)
+ (__this_cpu_preempt_check("cmpxchg"),__pcpu_size_call_return2(__this_cpu_cmpxchg_, pcp, oval, nval))
#endif
#define __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
@@ -745,7 +777,7 @@ do { \
__this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
# endif
# define __this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __pcpu_double_call_return_bool(__this_cpu_cmpxchg_double_, (pcp1), (pcp2), (oval1), (oval2), (nval1), (nval2))
+ (__this_cpu_preempt_check("cmpxchg_double"),__pcpu_double_call_return_bool(__this_cpu_cmpxchg_double_, (pcp1), (pcp2), (oval1), (oval2), (nval1), (nval2)))
#endif
/*
Index: linux/lib/smp_processor_id.c
===================================================================
--- linux.orig/lib/smp_processor_id.c 2013-10-10 11:30:17.111743444 -0500
+++ linux/lib/smp_processor_id.c 2013-10-10 10:32:16.046357108 -0500
@@ -7,7 +7,7 @@
#include <linux/kallsyms.h>
#include <linux/sched.h>
-notrace unsigned int debug_smp_processor_id(void)
+notrace static unsigned int check_preemption_disabled(char *what)
{
unsigned long preempt_count = preempt_count();
int this_cpu = raw_smp_processor_id();
@@ -39,9 +39,9 @@ notrace unsigned int debug_smp_processor
if (!printk_ratelimit())
goto out_enable;
- printk(KERN_ERR "BUG: using smp_processor_id() in preemptible [%08x] "
- "code: %s/%d\n",
- preempt_count() - 1, current->comm, current->pid);
+ printk(KERN_ERR "%s in preemptible [%08x] code: %s/%d\n",
+ what, preempt_count() - 1, current->comm, current->pid);
+
print_symbol("caller is %s\n", (long)__builtin_return_address(0));
dump_stack();
@@ -51,5 +51,18 @@ out:
return this_cpu;
}
+notrace unsigned int debug_smp_processor_id(void)
+{
+ return check_preemption_disabled("BUG: using smp_processor_id()");
+}
EXPORT_SYMBOL(debug_smp_processor_id);
+notrace void __this_cpu_preempt_check(const char *op)
+{
+ char text[40];
+
+ snprintf(text, sizeof(text), "__this_cpu_%s operation", op);
+ check_preemption_disabled(text);
+}
+EXPORT_SYMBOL(__this_cpu_preempt_check);
+
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update
2013-10-15 17:47 ` [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update Christoph Lameter
@ 2013-10-15 18:36 ` Eric Dumazet
2013-10-16 6:09 ` Ingo Molnar
2013-10-16 8:35 ` Peter Zijlstra
1 sibling, 1 reply; 33+ messages in thread
From: Eric Dumazet @ 2013-10-15 18:36 UTC (permalink / raw)
To: Christoph Lameter
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Peter Zijlstra, Thomas Gleixner, Eric Dumazet, David Miller
On Tue, 2013-10-15 at 12:47 -0500, Christoph Lameter wrote:
> plain text document attachment (fix_snmp)
> ip4_datagram_connect is called with BH processing enabled. Therefore
> we cannot use IP_INC_STATS_BH but must use IP_INC_STATS which disables
> BH handling before incrementing the counter.
>
> The following trace is triggered without this patch:
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: netdev@vger.kernel.org
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Christoph Lameter <cl@linux.com>
Strange, I thought I already Acked it...
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update
2013-10-15 18:36 ` Eric Dumazet
@ 2013-10-16 6:09 ` Ingo Molnar
0 siblings, 0 replies; 33+ messages in thread
From: Ingo Molnar @ 2013-10-16 6:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: Christoph Lameter, Tejun Heo, akpm, rostedt, linux-kernel,
Peter Zijlstra, Thomas Gleixner, Eric Dumazet, David Miller
* Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2013-10-15 at 12:47 -0500, Christoph Lameter wrote:
> > plain text document attachment (fix_snmp)
> > ip4_datagram_connect is called with BH processing enabled. Therefore
> > we cannot use IP_INC_STATS_BH but must use IP_INC_STATS which disables
> > BH handling before incrementing the counter.
> >
> > The following trace is triggered without this patch:
>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: netdev@vger.kernel.org
> > Cc: David Miller <davem@davemloft.net>
> > Signed-off-by: Christoph Lameter <cl@linux.com>
>
> Strange, I thought I already Acked it...
>
> Acked-by: Eric Dumazet <edumazet@google.com>
Christoph, _please_ pick up people's Acked-by's in the future ... This is
basic courtesy towards reviewers.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update
2013-10-15 17:47 ` [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update Christoph Lameter
2013-10-15 18:36 ` Eric Dumazet
@ 2013-10-16 8:35 ` Peter Zijlstra
2013-10-16 9:14 ` Eric Dumazet
1 sibling, 1 reply; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-16 8:35 UTC (permalink / raw)
To: Christoph Lameter
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner, Eric Dumazet, David Miller
On Tue, Oct 15, 2013 at 12:47:23PM -0500, Christoph Lameter wrote:
> ip4_datagram_connect is called with BH processing enabled. Therefore
> we cannot use IP_INC_STATS_BH but must use IP_INC_STATS which disables
> BH handling before incrementing the counter.
>
> The following trace is triggered without this patch:
>
> [ 9293.806634] __this_cpu_add operation in preemptible [00000000] code: ntpd/2150
You lost the BUG there; that really needs to be there:
- BUG makes people pay attention
- This was an actual BUG wasn't it?
Sure there can be false positives, but in all cases people should amend
the code. Sometimes with a comment explaining why the raw primitive
should be used; sometimes to fix an actual bug, but a patch needs to be
written. Therefore BUG!
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/6] mm: Use raw_cpu ops for determining current NUMA node
2013-10-15 17:47 ` [PATCH 3/6] mm: Use raw_cpu ops for determining current NUMA node Christoph Lameter
@ 2013-10-16 8:38 ` Peter Zijlstra
2013-10-16 14:22 ` Christoph Lameter
0 siblings, 1 reply; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-16 8:38 UTC (permalink / raw)
To: Christoph Lameter
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner, Alex Shi
On Tue, Oct 15, 2013 at 12:47:25PM -0500, Christoph Lameter wrote:
> Index: linux/include/linux/topology.h
> ===================================================================
> --- linux.orig/include/linux/topology.h 2013-09-24 11:29:51.000000000 -0500
> +++ linux/include/linux/topology.h 2013-09-24 11:30:18.893831971 -0500
> @@ -182,7 +182,7 @@ DECLARE_PER_CPU(int, numa_node);
> /* Returns the number of the current Node. */
> static inline int numa_node_id(void)
> {
> - return __this_cpu_read(numa_node);
> + return raw_cpu_read(numa_node);
> }
> #endif
>
> @@ -239,7 +239,7 @@ static inline void set_numa_mem(int node
> /* Returns the number of the nearest Node with memory */
> static inline int numa_mem_id(void)
> {
> - return __this_cpu_read(_numa_mem_);
> + return raw_cpu_read(_numa_mem_);
> }
> #endif
NAK; smp_processor_id() has the preemption checks; for consistently
numa_node_id() should have them too, for the very same reason. Who's to
say the node id is still valid when you return from this function? If
we're preemptable we could've just been migrated away to another node.
So please introduce raw_numa_node_id() and use that; all fully analogous
to smp_processor_id().
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 4/6] Use raw_cpu_write for initialization of per cpu refcount.
2013-10-15 17:47 ` [PATCH 4/6] Use raw_cpu_write for initialization of per cpu refcount Christoph Lameter
@ 2013-10-16 8:43 ` Peter Zijlstra
0 siblings, 0 replies; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-16 8:43 UTC (permalink / raw)
To: Christoph Lameter
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner, Rusty Russell
On Tue, Oct 15, 2013 at 12:47:26PM -0500, Christoph Lameter wrote:
> The initialization of a structure is not subject to synchronization.
> The use of __this_cpu would trigger a false positive with the
> additional preemption checks for __this_cpu ops.
>
> So simply disable the check through the use of raw_cpu ops.
>
> Trace:
>
> [ 0.668066] __this_cpu_write operation in preemptible [00000000] code: modprobe/286
> [ 0.668108] caller is __this_cpu_preempt_check+0x38/0x60
> [ 0.668111] CPU: 3 PID: 286 Comm: modprobe Tainted: GF 3.12.0-rc4+ #187
> [ 0.668112] Hardware name: FUJITSU CELSIUS W530 Power/D3227-A1, BIOS V4.6.5.4 R1.10.0 for D3227-A1x 09/16/2013
> [ 0.668113] 0000000000000003 ffff8807edda1d18 ffffffff816d5a57 ffff8807edda1fd8
> [ 0.668117] ffff8807edda1d48 ffffffff8137359c ffff8807edda1ef8 ffffffffa0002178
> [ 0.668121] ffffc90000067730 ffff8807edda1e48 ffff8807edda1d88 ffffffff813735f8
> [ 0.668124] Call Trace:
> [ 0.668129] [<ffffffff816d5a57>] dump_stack+0x4e/0x82
> [ 0.668132] [<ffffffff8137359c>] check_preemption_disabled+0xec/0x110
> [ 0.668135] [<ffffffff813735f8>] __this_cpu_preempt_check+0x38/0x60
> [ 0.668139] [<ffffffff810c24fd>] load_module+0xcfd/0x2650
> [ 0.668143] [<ffffffff816dd922>] ? page_fault+0x22/0x30
> [ 0.668146] [<ffffffff810c3ef6>] SyS_init_module+0xa6/0xd0
> [ 0.668150] [<ffffffff816e4fd3>] tracesys+0xe1/0xe6
>
>
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Signed-off-by: Christoph Lameter <cl@linux.com>
>
> Index: linux/kernel/module.c
> ===================================================================
> --- linux.orig/kernel/module.c 2013-09-05 13:43:30.557687773 -0500
> +++ linux/kernel/module.c 2013-10-07 12:33:43.732059759 -0500
> @@ -643,7 +643,7 @@ static int module_unload_init(struct mod
> INIT_LIST_HEAD(&mod->target_list);
>
> /* Hold reference count during initialization. */
> - __this_cpu_write(mod->refptr->incs, 1);
> + raw_cpu_write(mod->refptr->incs, 1);
Just add: Safe to use raw because we're the only user. So we all
instantly know.
> /* Backwards compatibility macros put refcount during init. */
> mod->waiter = current;
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/6] net: __this_cpu_inc in route.c
2013-10-15 17:47 ` [PATCH 5/6] net: __this_cpu_inc in route.c Christoph Lameter
@ 2013-10-16 8:46 ` Peter Zijlstra
2013-10-16 9:22 ` Eric Dumazet
0 siblings, 1 reply; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-16 8:46 UTC (permalink / raw)
To: Christoph Lameter
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner, Eric Dumazet
On Tue, Oct 15, 2013 at 12:47:27PM -0500, Christoph Lameter wrote:
> The RT_CACHE_STAT_INC macro triggers the new preemption checks
> for __this_cpu ops
>
> I do not see any other synchronization that would allow the use
> of a __this_cpu operation here however in commit
> dbd2915ce87e811165da0717f8e159276ebb803e Andrew justifies
> the use of raw_smp_processor_id() here because "we do not care"
> about races.
>
> So lets use raw_cpu ops here and hope for the best. The use of
> __this_cpu op improves the situation already from what commit
> dbd2915ce87e811165da0717f8e159276ebb803e did since the single instruction
> emitted on x86 does not allow the race to occur anymore. However,
> non x86 platforms could still experience a race here.
>
Are we sure all !x86 implementations will DTRT in that it will increment
some CPU and not get horribly confused? I suppose it would; but is
that a guarantee given someplace?
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-15 17:47 ` [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops Christoph Lameter
@ 2013-10-16 8:49 ` Peter Zijlstra
2013-10-16 15:09 ` Christoph Lameter
0 siblings, 1 reply; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-16 8:49 UTC (permalink / raw)
To: Christoph Lameter
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Tue, Oct 15, 2013 at 12:47:28PM -0500, Christoph Lameter wrote:
> V3->V4:
> - Drop CONFIG_DEBUG_THIS_CPU_OPERATIONS
> - Add support for logging the exact operation that caused the issue.
>
> We define a check function in order to avoid trouble with the
> include files. Then the higher level __this_cpu macros are
> modified to invoke the check before __this_cpu operation
>
> Signed-off-by: Christoph Lameter <cl@linux.com>
>
> Index: linux/include/linux/percpu.h
> ===================================================================
> --- linux.orig/include/linux/percpu.h 2013-10-10 11:30:17.111743444 -0500
> +++ linux/include/linux/percpu.h 2013-10-10 10:30:26.817316787 -0500
> @@ -175,6 +175,12 @@ extern phys_addr_t per_cpu_ptr_to_phys(v
>
> extern void __bad_size_call_parameter(void);
>
> +#ifdef CONFIG_DEBUG_PREEMPT
> +extern void __this_cpu_preempt_check(const char *op);
> +#else
> +static inline void __this_cpu_preempt_check(const char *op) { }
> +#endif
> +
> #define __pcpu_size_call_return(stem, variable) \
> ({ typeof(variable) pscr_ret__; \
> __verify_pcpu_ptr(&(variable)); \
> @@ -538,7 +544,8 @@ do { \
> # ifndef __this_cpu_read_8
> # define __this_cpu_read_8(pcp) (*__this_cpu_ptr(&(pcp)))
> # endif
> -# define __this_cpu_read(pcp) __pcpu_size_call_return(__this_cpu_read_, (pcp))
> +# define __this_cpu_read(pcp) \
> + (__this_cpu_preempt_check("read"),__pcpu_size_call_return(__this_cpu_read_, (pcp)))
> #endif
>
So I didn't understand what was wrong with:
#define __this_cpu_read(pcp) \
(__this_cpu_preempt_check("read"), raw_this_cpu_read(pcp))
And idem for all others. This is 1) shorter to write; and 2) makes it
blindingly obvious that the implementations are actually the same.
> Index: linux/lib/smp_processor_id.c
> ===================================================================
> --- linux.orig/lib/smp_processor_id.c 2013-10-10 11:30:17.111743444 -0500
> +++ linux/lib/smp_processor_id.c 2013-10-10 10:32:16.046357108 -0500
> @@ -7,7 +7,7 @@
> #include <linux/kallsyms.h>
> #include <linux/sched.h>
>
> -notrace unsigned int debug_smp_processor_id(void)
> +notrace static unsigned int check_preemption_disabled(char *what)
> {
> unsigned long preempt_count = preempt_count();
> int this_cpu = raw_smp_processor_id();
> @@ -39,9 +39,9 @@ notrace unsigned int debug_smp_processor
> if (!printk_ratelimit())
> goto out_enable;
>
> - printk(KERN_ERR "BUG: using smp_processor_id() in preemptible [%08x] "
> - "code: %s/%d\n",
> - preempt_count() - 1, current->comm, current->pid);
> + printk(KERN_ERR "%s in preemptible [%08x] code: %s/%d\n",
> + what, preempt_count() - 1, current->comm, current->pid);
> +
Like already said; NAK until that "BUG" remains.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update
2013-10-16 8:35 ` Peter Zijlstra
@ 2013-10-16 9:14 ` Eric Dumazet
2013-10-16 9:26 ` Ingo Molnar
0 siblings, 1 reply; 33+ messages in thread
From: Eric Dumazet @ 2013-10-16 9:14 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Christoph Lameter, Tejun Heo, akpm, rostedt, linux-kernel,
Ingo Molnar, Thomas Gleixner, Eric Dumazet, David Miller
On Wed, 2013-10-16 at 10:35 +0200, Peter Zijlstra wrote:
> On Tue, Oct 15, 2013 at 12:47:23PM -0500, Christoph Lameter wrote:
> > ip4_datagram_connect is called with BH processing enabled. Therefore
> > we cannot use IP_INC_STATS_BH but must use IP_INC_STATS which disables
> > BH handling before incrementing the counter.
> >
> > The following trace is triggered without this patch:
> >
> > [ 9293.806634] __this_cpu_add operation in preemptible [00000000] code: ntpd/2150
>
> You lost the BUG there; that really needs to be there:
>
> - BUG makes people pay attention
> - This was an actual BUG wasn't it?
>
> Sure there can be false positives, but in all cases people should amend
> the code. Sometimes with a comment explaining why the raw primitive
> should be used; sometimes to fix an actual bug, but a patch needs to be
> written. Therefore BUG!
Yep this is a real BUG for linux 2.6.36+ on 32bit arches,
The effect of this bug is that on 32bit arches, we might corrupt
a seqcount : Later, we can spin forever on it.
In linux 2.6.36 we converted IP mib from 32 to 64 bits, therefore this
fix should be backported up to 2.6.36
Prior to 2.6.36, the bug was that some increments of SNMP stat could be
lost, because two cpus could access the same location, hardly a
problem...
Thanks
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/6] net: __this_cpu_inc in route.c
2013-10-16 8:46 ` Peter Zijlstra
@ 2013-10-16 9:22 ` Eric Dumazet
2013-10-16 10:25 ` Peter Zijlstra
0 siblings, 1 reply; 33+ messages in thread
From: Eric Dumazet @ 2013-10-16 9:22 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Christoph Lameter, Tejun Heo, akpm, rostedt, linux-kernel,
Ingo Molnar, Thomas Gleixner, Eric Dumazet
On Wed, 2013-10-16 at 10:46 +0200, Peter Zijlstra wrote:
> Are we sure all !x86 implementations will DTRT in that it will increment
> some CPU and not get horribly confused? I suppose it would; but is
> that a guarantee given someplace?
I think we should be fine, these are only stats exposed
on /proc/net/stat/rt_cache
Those 32bit counters were useful when we still had IP route cache.
Most of the fields are no longer incremented.
We eventually could remove them.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update
2013-10-16 9:14 ` Eric Dumazet
@ 2013-10-16 9:26 ` Ingo Molnar
2013-10-16 14:27 ` Christoph Lameter
0 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2013-10-16 9:26 UTC (permalink / raw)
To: Eric Dumazet
Cc: Peter Zijlstra, Christoph Lameter, Tejun Heo, akpm, rostedt,
linux-kernel, Thomas Gleixner, Eric Dumazet, David Miller
* Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2013-10-16 at 10:35 +0200, Peter Zijlstra wrote:
> > On Tue, Oct 15, 2013 at 12:47:23PM -0500, Christoph Lameter wrote:
> > > ip4_datagram_connect is called with BH processing enabled. Therefore
> > > we cannot use IP_INC_STATS_BH but must use IP_INC_STATS which disables
> > > BH handling before incrementing the counter.
> > >
> > > The following trace is triggered without this patch:
> > >
> > > [ 9293.806634] __this_cpu_add operation in preemptible [00000000] code: ntpd/2150
> >
> > You lost the BUG there; that really needs to be there:
> >
> > - BUG makes people pay attention
> > - This was an actual BUG wasn't it?
> >
> > Sure there can be false positives, but in all cases people should
> > amend the code. Sometimes with a comment explaining why the raw
> > primitive should be used; sometimes to fix an actual bug, but a patch
> > needs to be written. Therefore BUG!
>
> Yep this is a real BUG for linux 2.6.36+ on 32bit arches,
>
> The effect of this bug is that on 32bit arches, we might corrupt a
> seqcount : Later, we can spin forever on it.
Ouch, that's a pretty serious bug ...
The patch title should reflect this fact.
> In linux 2.6.36 we converted IP mib from 32 to 64 bits, therefore this
> fix should be backported up to 2.6.36
>
> Prior to 2.6.36, the bug was that some increments of SNMP stat could be
> lost, because two cpus could access the same location, hardly a
> problem...
Acked-by: Ingo Molnar <mingo@kernel.org>
Thanks,
Ingo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/6] net: __this_cpu_inc in route.c
2013-10-16 9:22 ` Eric Dumazet
@ 2013-10-16 10:25 ` Peter Zijlstra
2013-10-16 15:07 ` Christoph Lameter
0 siblings, 1 reply; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-16 10:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: Christoph Lameter, Tejun Heo, akpm, rostedt, linux-kernel,
Ingo Molnar, Thomas Gleixner, Eric Dumazet
On Wed, Oct 16, 2013 at 02:22:49AM -0700, Eric Dumazet wrote:
> On Wed, 2013-10-16 at 10:46 +0200, Peter Zijlstra wrote:
>
> > Are we sure all !x86 implementations will DTRT in that it will increment
> > some CPU and not get horribly confused? I suppose it would; but is
> > that a guarantee given someplace?
>
> I think we should be fine, these are only stats exposed
> on /proc/net/stat/rt_cache
My concern was really that an unprotected __this_cpu op would complete
coherent and not corrupt state. If we cannot guarantee this it should
always be a full BUG to use it without proper protection.
For x86 its fairly easy to see its correct this way; but for load-store
archs this isn't immediately obvious to me.
Suppose; r1 contains our per-cpu pointer:
LOAD r2, per-cpu-base
ADD r1, r2
LOAD r2, $(r1) # load value
INC r2
STORE $(r1), r2 # store value
If such a thing is done without preempt disable; we could be
preempted/migrated at any place along that chain. In that case the STORE
could be to another CPUs memory (we get migrated near the INC) and could
conflict with a per-cpu operation on that CPU corrupting state.
If I look at percpu.h; the generic __this_cpu versions look like they
generate the above for such archs.
In that case; I don't see how even for statistics (where we really don't
care what cpu the op happens on, as long as it happens to a cpu,
coherently) it is correct to use the raw_this_cpu stuff without
preemption protection.
In fact; I think the comment near __this_cpu_read actually alludes to
this.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/6] mm: Use raw_cpu ops for determining current NUMA node
2013-10-16 8:38 ` Peter Zijlstra
@ 2013-10-16 14:22 ` Christoph Lameter
0 siblings, 0 replies; 33+ messages in thread
From: Christoph Lameter @ 2013-10-16 14:22 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner, Alex Shi
On Wed, 16 Oct 2013, Peter Zijlstra wrote:
> NAK; smp_processor_id() has the preemption checks; for consistently
> numa_node_id() should have them too, for the very same reason. Who's to
> say the node id is still valid when you return from this function? If
> we're preemptable we could've just been migrated away to another node.
>
> So please introduce raw_numa_node_id() and use that; all fully analogous
> to smp_processor_id().
The code that was here before the use of this_cpu ops did use
raw_smp_processor_id.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update
2013-10-16 9:26 ` Ingo Molnar
@ 2013-10-16 14:27 ` Christoph Lameter
2013-10-16 14:37 ` Eric Dumazet
0 siblings, 1 reply; 33+ messages in thread
From: Christoph Lameter @ 2013-10-16 14:27 UTC (permalink / raw)
To: Ingo Molnar
Cc: Eric Dumazet, Peter Zijlstra, Tejun Heo, akpm, rostedt,
linux-kernel, Thomas Gleixner, Eric Dumazet, David Miller
Same issue in ipv6 code.
Subject: net: Fix statistics update in ip6
ipv6 does the same as ipv4. IP6_INC_STATS must be used outside
of BH sections.
Signed-off-by: Christoph Lameter <cl@linux.com>
Index: linux/net/ipv6/ip6_output.c
===================================================================
--- linux.orig/net/ipv6/ip6_output.c 2013-10-10 11:01:01.119771564 -0500
+++ linux/net/ipv6/ip6_output.c 2013-10-16 08:48:19.474562100 -0500
@@ -909,7 +909,7 @@ static int ip6_dst_lookup_tail(struct so
out_err_release:
if (err == -ENETUNREACH)
- IP6_INC_STATS_BH(net, NULL, IPSTATS_MIB_OUTNOROUTES);
+ IP6_INC_STATS(net, NULL, IPSTATS_MIB_OUTNOROUTES);
dst_release(*dst);
*dst = NULL;
return err;
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update
2013-10-16 14:27 ` Christoph Lameter
@ 2013-10-16 14:37 ` Eric Dumazet
0 siblings, 0 replies; 33+ messages in thread
From: Eric Dumazet @ 2013-10-16 14:37 UTC (permalink / raw)
To: Christoph Lameter
Cc: Ingo Molnar, Peter Zijlstra, Tejun Heo, akpm, rostedt,
linux-kernel, Thomas Gleixner, Eric Dumazet, David Miller
On Wed, 2013-10-16 at 14:27 +0000, Christoph Lameter wrote:
> Same issue in ipv6 code.
>
>
> Subject: net: Fix statistics update in ip6
>
> ipv6 does the same as ipv4. IP6_INC_STATS must be used outside
> of BH sections.
>
> Signed-off-by: Christoph Lameter <cl@linux.com>
>
Missing separator "---"
> Index: linux/net/ipv6/ip6_output.c
> ===================================================================
> --- linux.orig/net/ipv6/ip6_output.c 2013-10-10 11:01:01.119771564 -0500
> +++ linux/net/ipv6/ip6_output.c 2013-10-16 08:48:19.474562100 -0500
> @@ -909,7 +909,7 @@ static int ip6_dst_lookup_tail(struct so
>
> out_err_release:
> if (err == -ENETUNREACH)
> - IP6_INC_STATS_BH(net, NULL, IPSTATS_MIB_OUTNOROUTES);
> + IP6_INC_STATS(net, NULL, IPSTATS_MIB_OUTNOROUTES);
> dst_release(*dst);
> *dst = NULL;
> return err;
Yes, bug added in commit ca46f9c834913fc5
("[IPv6] SNMP: Increment OutNoRoutes when connecting to unreachable
network")
Your inability to cc netdev for netdev patches is really irritating,
you do know that, right ?
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/6] net: __this_cpu_inc in route.c
2013-10-16 10:25 ` Peter Zijlstra
@ 2013-10-16 15:07 ` Christoph Lameter
0 siblings, 0 replies; 33+ messages in thread
From: Christoph Lameter @ 2013-10-16 15:07 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Eric Dumazet, Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner, Eric Dumazet
On Wed, 16 Oct 2013, Peter Zijlstra wrote:
> For x86 its fairly easy to see its correct this way; but for load-store
> archs this isn't immediately obvious to me.
>
> Suppose; r1 contains our per-cpu pointer:
>
> LOAD r2, per-cpu-base
> ADD r1, r2
> LOAD r2, $(r1) # load value
> INC r2
> STORE $(r1), r2 # store value
>
> If such a thing is done without preempt disable; we could be
> preempted/migrated at any place along that chain. In that case the STORE
> could be to another CPUs memory (we get migrated near the INC) and could
> conflict with a per-cpu operation on that CPU corrupting state.
>
> If I look at percpu.h; the generic __this_cpu versions look like they
> generate the above for such archs.
Yes they do.
> In that case; I don't see how even for statistics (where we really don't
> care what cpu the op happens on, as long as it happens to a cpu,
> coherently) it is correct to use the raw_this_cpu stuff without
> preemption protection.
>
> In fact; I think the comment near __this_cpu_read actually alludes to
> this.
Indeed. This is well known. For some statistics this has been
judged to be acceptable in the past when the alternative was disabling
interrupts since the overhead was so much higher and the statistics
operations had to be used in very performance critical sections.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-16 8:49 ` Peter Zijlstra
@ 2013-10-16 15:09 ` Christoph Lameter
2013-10-16 15:36 ` Peter Zijlstra
0 siblings, 1 reply; 33+ messages in thread
From: Christoph Lameter @ 2013-10-16 15:09 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Wed, 16 Oct 2013, Peter Zijlstra wrote:
> So I didn't understand what was wrong with:
>
> #define __this_cpu_read(pcp) \
> (__this_cpu_preempt_check("read"), raw_this_cpu_read(pcp))
>
> And idem for all others. This is 1) shorter to write; and 2) makes it
> blindingly obvious that the implementations are actually the same.
Nothing wrong with that. It just increases the scope of this patch to
require modifications to arch code and I already have trouble enough
following through on all the issues that were raised so far.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-16 15:09 ` Christoph Lameter
@ 2013-10-16 15:36 ` Peter Zijlstra
2013-10-16 15:55 ` Christoph Lameter
0 siblings, 1 reply; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-16 15:36 UTC (permalink / raw)
To: Christoph Lameter
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Wed, Oct 16, 2013 at 03:09:13PM +0000, Christoph Lameter wrote:
> On Wed, 16 Oct 2013, Peter Zijlstra wrote:
>
> > So I didn't understand what was wrong with:
> >
> > #define __this_cpu_read(pcp) \
> > (__this_cpu_preempt_check("read"), raw_this_cpu_read(pcp))
> >
> > And idem for all others. This is 1) shorter to write; and 2) makes it
> > blindingly obvious that the implementations are actually the same.
>
> Nothing wrong with that. It just increases the scope of this patch to
> require modifications to arch code and I already have trouble enough
> following through on all the issues that were raised so far.
>
But non of your raw ops touch arch code... /me confused.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-16 15:36 ` Peter Zijlstra
@ 2013-10-16 15:55 ` Christoph Lameter
2013-10-16 16:25 ` Peter Zijlstra
0 siblings, 1 reply; 33+ messages in thread
From: Christoph Lameter @ 2013-10-16 15:55 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Wed, 16 Oct 2013, Peter Zijlstra wrote:
> On Wed, Oct 16, 2013 at 03:09:13PM +0000, Christoph Lameter wrote:
> > On Wed, 16 Oct 2013, Peter Zijlstra wrote:
> >
> > > So I didn't understand what was wrong with:
> > >
> > > #define __this_cpu_read(pcp) \
> > > (__this_cpu_preempt_check("read"), raw_this_cpu_read(pcp))
> > >
> > > And idem for all others. This is 1) shorter to write; and 2) makes it
> > > blindingly obvious that the implementations are actually the same.
> >
> > Nothing wrong with that. It just increases the scope of this patch to
> > require modifications to arch code and I already have trouble enough
> > following through on all the issues that were raised so far.
> >
>
> But non of your raw ops touch arch code... /me confused.
Yes that is intentional to limit scope. Renaming would require arch
changes.
The __this_cpu_xxxs are defined in arch code. Look at x86 arch
implementations for example. arch/x86/include/asm/percpu.h. s390 also has
this. Not sure if other arches do.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-16 15:55 ` Christoph Lameter
@ 2013-10-16 16:25 ` Peter Zijlstra
2013-10-16 16:52 ` Steven Rostedt
2013-10-16 18:38 ` Peter Zijlstra
0 siblings, 2 replies; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-16 16:25 UTC (permalink / raw)
To: Christoph Lameter
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Wed, Oct 16, 2013 at 03:55:47PM +0000, Christoph Lameter wrote:
> On Wed, 16 Oct 2013, Peter Zijlstra wrote:
>
> > On Wed, Oct 16, 2013 at 03:09:13PM +0000, Christoph Lameter wrote:
> > > On Wed, 16 Oct 2013, Peter Zijlstra wrote:
> > >
> > > > So I didn't understand what was wrong with:
> > > >
> > > > #define __this_cpu_read(pcp) \
> > > > (__this_cpu_preempt_check("read"), raw_this_cpu_read(pcp))
> > > >
> > > > And idem for all others. This is 1) shorter to write; and 2) makes it
> > > > blindingly obvious that the implementations are actually the same.
> > >
> > > Nothing wrong with that. It just increases the scope of this patch to
> > > require modifications to arch code and I already have trouble enough
> > > following through on all the issues that were raised so far.
> > >
> >
> > But non of your raw ops touch arch code... /me confused.
>
> Yes that is intentional to limit scope. Renaming would require arch
> changes.
>
> The __this_cpu_xxxs are defined in arch code. Look at x86 arch
> implementations for example. arch/x86/include/asm/percpu.h. s390 also has
> this. Not sure if other arches do.
I can only find x86 that has per arch __this_cpu thingies; and then only
the __this_cpu_$op_$n variants, the actual __this_cpu_$op stuff comes
from linux/percpu.h:
# define __this_cpu_read(pcp) __pcpu_size_call_return(__this_cpu_read_, (pcp))
With exception of __this_cpu_ptr.
# git grep "#[[:space:]]*define[[:space:]]*__this_cpu" | grep -v "__this_cpu[[:alpha:]_]*[0-9]"
arch/x86/include/asm/percpu.h:#define __this_cpu_ptr(ptr) \
include/asm-generic/percpu.h:#define __this_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset)
include/asm-generic/percpu.h:#define __this_cpu_ptr(ptr) this_cpu_ptr(ptr)
include/linux/percpu.h:# define __this_cpu_read(pcp) __pcpu_size_call_return(__this_cpu_read_, (pcp))
include/linux/percpu.h:#define __this_cpu_generic_to_op(pcp, val, op) \
include/linux/percpu.h:# define __this_cpu_write(pcp, val) __pcpu_size_call(__this_cpu_write_, (pcp), (val))
include/linux/percpu.h:# define __this_cpu_add(pcp, val) __pcpu_size_call(__this_cpu_add_, (pcp), (val))
include/linux/percpu.h:# define __this_cpu_sub(pcp, val) __this_cpu_add((pcp), -(val))
include/linux/percpu.h:# define __this_cpu_inc(pcp) __this_cpu_add((pcp), 1)
include/linux/percpu.h:# define __this_cpu_dec(pcp) __this_cpu_sub((pcp), 1)
include/linux/percpu.h:# define __this_cpu_and(pcp, val) __pcpu_size_call(__this_cpu_and_, (pcp), (val))
include/linux/percpu.h:# define __this_cpu_or(pcp, val) __pcpu_size_call(__this_cpu_or_, (pcp), (val))
include/linux/percpu.h:# define __this_cpu_xor(pcp, val) __pcpu_size_call(__this_cpu_xor_, (pcp), (val))
include/linux/percpu.h:#define __this_cpu_generic_add_return(pcp, val) \
include/linux/percpu.h:# define __this_cpu_add_return(pcp, val) \
include/linux/percpu.h:#define __this_cpu_sub_return(pcp, val) __this_cpu_add_return(pcp, -(val))
include/linux/percpu.h:#define __this_cpu_inc_return(pcp) __this_cpu_add_return(pcp, 1)
include/linux/percpu.h:#define __this_cpu_dec_return(pcp) __this_cpu_add_return(pcp, -1)
include/linux/percpu.h:#define __this_cpu_generic_xchg(pcp, nval) \
include/linux/percpu.h:# define __this_cpu_xchg(pcp, nval) \
include/linux/percpu.h:#define __this_cpu_generic_cmpxchg(pcp, oval, nval) \
include/linux/percpu.h:# define __this_cpu_cmpxchg(pcp, oval, nval) \
include/linux/percpu.h:#define __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
include/linux/percpu.h:# define __this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
But yes, the way its set-up an arch could indeed provide __this_cup_$op
itself -- without providing the _$n variants; in which case the
raw_cpu_$op provided by you is broken.
Can't we have a 'simple' coccinelle script rename the entire __this_cpu*
implementation over to raw_cpu* and then provide generic __this_cpu* ->
raw_cpu maps?
>From what I can gather the __this_cpu_$op_$n variants aren't an official
API anyway.
Maybe something simple like this:
git grep -l "#[[:space:]]*define[[:space:]]*__this_cpu" | while read file; do sed -ie 's/__this_cpu/raw_cpu/g' $file; done
To rename __this_cpu to raw_cpu; it would then need a little manual
fixup and generic __this_cpu -> raW_cpu map, which can add the
preemption check.
---
arch/x86/include/asm/percpu.h | 84 ++++++-------
include/asm-generic/percpu.h | 10 +-
include/linux/percpu.h | 282 +++++++++++++++++++++---------------------
3 files changed, 188 insertions(+), 188 deletions(-)
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 0da5200ee79d..7a9fbff3d191 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -52,7 +52,7 @@
* Compared to the generic __my_cpu_offset version, the following
* saves one instruction and avoids clobbering a temp register.
*/
-#define __this_cpu_ptr(ptr) \
+#define raw_cpu_ptr(ptr) \
({ \
unsigned long tcp_ptr__; \
__verify_pcpu_ptr(ptr); \
@@ -361,28 +361,28 @@ do { \
*/
#define this_cpu_read_stable(var) percpu_from_op("mov", var, "p" (&(var)))
-#define __this_cpu_read_1(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
-#define __this_cpu_read_2(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
-#define __this_cpu_read_4(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
-
-#define __this_cpu_write_1(pcp, val) percpu_to_op("mov", (pcp), val)
-#define __this_cpu_write_2(pcp, val) percpu_to_op("mov", (pcp), val)
-#define __this_cpu_write_4(pcp, val) percpu_to_op("mov", (pcp), val)
-#define __this_cpu_add_1(pcp, val) percpu_add_op((pcp), val)
-#define __this_cpu_add_2(pcp, val) percpu_add_op((pcp), val)
-#define __this_cpu_add_4(pcp, val) percpu_add_op((pcp), val)
-#define __this_cpu_and_1(pcp, val) percpu_to_op("and", (pcp), val)
-#define __this_cpu_and_2(pcp, val) percpu_to_op("and", (pcp), val)
-#define __this_cpu_and_4(pcp, val) percpu_to_op("and", (pcp), val)
-#define __this_cpu_or_1(pcp, val) percpu_to_op("or", (pcp), val)
-#define __this_cpu_or_2(pcp, val) percpu_to_op("or", (pcp), val)
-#define __this_cpu_or_4(pcp, val) percpu_to_op("or", (pcp), val)
-#define __this_cpu_xor_1(pcp, val) percpu_to_op("xor", (pcp), val)
-#define __this_cpu_xor_2(pcp, val) percpu_to_op("xor", (pcp), val)
-#define __this_cpu_xor_4(pcp, val) percpu_to_op("xor", (pcp), val)
-#define __this_cpu_xchg_1(pcp, val) percpu_xchg_op(pcp, val)
-#define __this_cpu_xchg_2(pcp, val) percpu_xchg_op(pcp, val)
-#define __this_cpu_xchg_4(pcp, val) percpu_xchg_op(pcp, val)
+#define raw_cpu_read_1(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
+#define raw_cpu_read_2(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
+#define raw_cpu_read_4(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
+
+#define raw_cpu_write_1(pcp, val) percpu_to_op("mov", (pcp), val)
+#define raw_cpu_write_2(pcp, val) percpu_to_op("mov", (pcp), val)
+#define raw_cpu_write_4(pcp, val) percpu_to_op("mov", (pcp), val)
+#define raw_cpu_add_1(pcp, val) percpu_add_op((pcp), val)
+#define raw_cpu_add_2(pcp, val) percpu_add_op((pcp), val)
+#define raw_cpu_add_4(pcp, val) percpu_add_op((pcp), val)
+#define raw_cpu_and_1(pcp, val) percpu_to_op("and", (pcp), val)
+#define raw_cpu_and_2(pcp, val) percpu_to_op("and", (pcp), val)
+#define raw_cpu_and_4(pcp, val) percpu_to_op("and", (pcp), val)
+#define raw_cpu_or_1(pcp, val) percpu_to_op("or", (pcp), val)
+#define raw_cpu_or_2(pcp, val) percpu_to_op("or", (pcp), val)
+#define raw_cpu_or_4(pcp, val) percpu_to_op("or", (pcp), val)
+#define raw_cpu_xor_1(pcp, val) percpu_to_op("xor", (pcp), val)
+#define raw_cpu_xor_2(pcp, val) percpu_to_op("xor", (pcp), val)
+#define raw_cpu_xor_4(pcp, val) percpu_to_op("xor", (pcp), val)
+#define raw_cpu_xchg_1(pcp, val) percpu_xchg_op(pcp, val)
+#define raw_cpu_xchg_2(pcp, val) percpu_xchg_op(pcp, val)
+#define raw_cpu_xchg_4(pcp, val) percpu_xchg_op(pcp, val)
#define this_cpu_read_1(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
#define this_cpu_read_2(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
@@ -406,12 +406,12 @@ do { \
#define this_cpu_xchg_2(pcp, nval) percpu_xchg_op(pcp, nval)
#define this_cpu_xchg_4(pcp, nval) percpu_xchg_op(pcp, nval)
-#define __this_cpu_add_return_1(pcp, val) percpu_add_return_op(pcp, val)
-#define __this_cpu_add_return_2(pcp, val) percpu_add_return_op(pcp, val)
-#define __this_cpu_add_return_4(pcp, val) percpu_add_return_op(pcp, val)
-#define __this_cpu_cmpxchg_1(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
-#define __this_cpu_cmpxchg_2(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
-#define __this_cpu_cmpxchg_4(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
+#define raw_cpu_add_return_1(pcp, val) percpu_add_return_op(pcp, val)
+#define raw_cpu_add_return_2(pcp, val) percpu_add_return_op(pcp, val)
+#define raw_cpu_add_return_4(pcp, val) percpu_add_return_op(pcp, val)
+#define raw_cpu_cmpxchg_1(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
+#define raw_cpu_cmpxchg_2(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
+#define raw_cpu_cmpxchg_4(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
#define this_cpu_add_return_1(pcp, val) percpu_add_return_op(pcp, val)
#define this_cpu_add_return_2(pcp, val) percpu_add_return_op(pcp, val)
@@ -432,7 +432,7 @@ do { \
__ret; \
})
-#define __this_cpu_cmpxchg_double_4 percpu_cmpxchg8b_double
+#define raw_cpu_cmpxchg_double_4 percpu_cmpxchg8b_double
#define this_cpu_cmpxchg_double_4 percpu_cmpxchg8b_double
#endif /* CONFIG_X86_CMPXCHG64 */
@@ -441,15 +441,15 @@ do { \
* 32 bit must fall back to generic operations.
*/
#ifdef CONFIG_X86_64
-#define __this_cpu_read_8(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
-#define __this_cpu_write_8(pcp, val) percpu_to_op("mov", (pcp), val)
-#define __this_cpu_add_8(pcp, val) percpu_add_op((pcp), val)
-#define __this_cpu_and_8(pcp, val) percpu_to_op("and", (pcp), val)
-#define __this_cpu_or_8(pcp, val) percpu_to_op("or", (pcp), val)
-#define __this_cpu_xor_8(pcp, val) percpu_to_op("xor", (pcp), val)
-#define __this_cpu_add_return_8(pcp, val) percpu_add_return_op(pcp, val)
-#define __this_cpu_xchg_8(pcp, nval) percpu_xchg_op(pcp, nval)
-#define __this_cpu_cmpxchg_8(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
+#define raw_cpu_read_8(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
+#define raw_cpu_write_8(pcp, val) percpu_to_op("mov", (pcp), val)
+#define raw_cpu_add_8(pcp, val) percpu_add_op((pcp), val)
+#define raw_cpu_and_8(pcp, val) percpu_to_op("and", (pcp), val)
+#define raw_cpu_or_8(pcp, val) percpu_to_op("or", (pcp), val)
+#define raw_cpu_xor_8(pcp, val) percpu_to_op("xor", (pcp), val)
+#define raw_cpu_add_return_8(pcp, val) percpu_add_return_op(pcp, val)
+#define raw_cpu_xchg_8(pcp, nval) percpu_xchg_op(pcp, nval)
+#define raw_cpu_cmpxchg_8(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
#define this_cpu_read_8(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
#define this_cpu_write_8(pcp, val) percpu_to_op("mov", (pcp), val)
@@ -481,7 +481,7 @@ do { \
__ret; \
})
-#define __this_cpu_cmpxchg_double_8 percpu_cmpxchg16b_double
+#define raw_cpu_cmpxchg_double_8 percpu_cmpxchg16b_double
#define this_cpu_cmpxchg_double_8 percpu_cmpxchg16b_double
#endif
@@ -502,9 +502,9 @@ static __always_inline int x86_this_cpu_constant_test_bit(unsigned int nr,
unsigned long __percpu *a = (unsigned long *)addr + nr / BITS_PER_LONG;
#ifdef CONFIG_X86_64
- return ((1UL << (nr % BITS_PER_LONG)) & __this_cpu_read_8(*a)) != 0;
+ return ((1UL << (nr % BITS_PER_LONG)) & raw_cpu_read_8(*a)) != 0;
#else
- return ((1UL << (nr % BITS_PER_LONG)) & __this_cpu_read_4(*a)) != 0;
+ return ((1UL << (nr % BITS_PER_LONG)) & raw_cpu_read_4(*a)) != 0;
#endif
}
diff --git a/include/asm-generic/percpu.h b/include/asm-generic/percpu.h
index d17784ea37ff..c6d86ab3aa67 100644
--- a/include/asm-generic/percpu.h
+++ b/include/asm-generic/percpu.h
@@ -56,17 +56,17 @@ extern unsigned long __per_cpu_offset[NR_CPUS];
#define per_cpu(var, cpu) \
(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
-#ifndef __this_cpu_ptr
-#define __this_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset)
+#ifndef raw_cpu_ptr
+#define raw_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset)
#endif
#ifdef CONFIG_DEBUG_PREEMPT
#define this_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, my_cpu_offset)
#else
-#define this_cpu_ptr(ptr) __this_cpu_ptr(ptr)
+#define this_cpu_ptr(ptr) raw_cpu_ptr(ptr)
#endif
#define __get_cpu_var(var) (*this_cpu_ptr(&(var)))
-#define __raw_get_cpu_var(var) (*__this_cpu_ptr(&(var)))
+#define __raw_get_cpu_var(var) (*raw_cpu_ptr(&(var)))
#ifdef CONFIG_HAVE_SETUP_PER_CPU_AREA
extern void setup_per_cpu_areas(void);
@@ -83,7 +83,7 @@ extern void setup_per_cpu_areas(void);
#define __get_cpu_var(var) (*VERIFY_PERCPU_PTR(&(var)))
#define __raw_get_cpu_var(var) (*VERIFY_PERCPU_PTR(&(var)))
#define this_cpu_ptr(ptr) per_cpu_ptr(ptr, 0)
-#define __this_cpu_ptr(ptr) this_cpu_ptr(ptr)
+#define raw_cpu_ptr(ptr) this_cpu_ptr(ptr)
#endif /* SMP */
diff --git a/include/linux/percpu.h b/include/linux/percpu.h
index cc88172c7d9a..c9ba66e58acf 100644
--- a/include/linux/percpu.h
+++ b/include/linux/percpu.h
@@ -295,7 +295,7 @@ do { \
do { \
unsigned long flags; \
raw_local_irq_save(flags); \
- *__this_cpu_ptr(&(pcp)) op val; \
+ *raw_cpu_ptr(&(pcp)) op val; \
raw_local_irq_restore(flags); \
} while (0)
@@ -396,8 +396,8 @@ do { \
typeof(pcp) ret__; \
unsigned long flags; \
raw_local_irq_save(flags); \
- __this_cpu_add(pcp, val); \
- ret__ = __this_cpu_read(pcp); \
+ raw_cpu_add(pcp, val); \
+ ret__ = raw_cpu_read(pcp); \
raw_local_irq_restore(flags); \
ret__; \
})
@@ -426,8 +426,8 @@ do { \
({ typeof(pcp) ret__; \
unsigned long flags; \
raw_local_irq_save(flags); \
- ret__ = __this_cpu_read(pcp); \
- __this_cpu_write(pcp, nval); \
+ ret__ = raw_cpu_read(pcp); \
+ raw_cpu_write(pcp, nval); \
raw_local_irq_restore(flags); \
ret__; \
})
@@ -454,9 +454,9 @@ do { \
typeof(pcp) ret__; \
unsigned long flags; \
raw_local_irq_save(flags); \
- ret__ = __this_cpu_read(pcp); \
+ ret__ = raw_cpu_read(pcp); \
if (ret__ == (oval)) \
- __this_cpu_write(pcp, nval); \
+ raw_cpu_write(pcp, nval); \
raw_local_irq_restore(flags); \
ret__; \
})
@@ -491,7 +491,7 @@ do { \
int ret__; \
unsigned long flags; \
raw_local_irq_save(flags); \
- ret__ = __this_cpu_generic_cmpxchg_double(pcp1, pcp2, \
+ ret__ = raw_cpu_generic_cmpxchg_double(pcp1, pcp2, \
oval1, oval2, nval1, nval2); \
raw_local_irq_restore(flags); \
ret__; \
@@ -532,227 +532,227 @@ do { \
* or an interrupt occurred and the same percpu variable was modified from
* the interrupt context.
*/
-#ifndef __this_cpu_read
-# ifndef __this_cpu_read_1
-# define __this_cpu_read_1(pcp) (*__this_cpu_ptr(&(pcp)))
+#ifndef raw_cpu_read
+# ifndef raw_cpu_read_1
+# define raw_cpu_read_1(pcp) (*raw_cpu_ptr(&(pcp)))
# endif
-# ifndef __this_cpu_read_2
-# define __this_cpu_read_2(pcp) (*__this_cpu_ptr(&(pcp)))
+# ifndef raw_cpu_read_2
+# define raw_cpu_read_2(pcp) (*raw_cpu_ptr(&(pcp)))
# endif
-# ifndef __this_cpu_read_4
-# define __this_cpu_read_4(pcp) (*__this_cpu_ptr(&(pcp)))
+# ifndef raw_cpu_read_4
+# define raw_cpu_read_4(pcp) (*raw_cpu_ptr(&(pcp)))
# endif
-# ifndef __this_cpu_read_8
-# define __this_cpu_read_8(pcp) (*__this_cpu_ptr(&(pcp)))
+# ifndef raw_cpu_read_8
+# define raw_cpu_read_8(pcp) (*raw_cpu_ptr(&(pcp)))
# endif
-# define __this_cpu_read(pcp) __pcpu_size_call_return(__this_cpu_read_, (pcp))
+# define raw_cpu_read(pcp) __pcpu_size_call_return(raw_cpu_read_, (pcp))
#endif
-#define __this_cpu_generic_to_op(pcp, val, op) \
+#define raw_cpu_generic_to_op(pcp, val, op) \
do { \
- *__this_cpu_ptr(&(pcp)) op val; \
+ *raw_cpu_ptr(&(pcp)) op val; \
} while (0)
-#ifndef __this_cpu_write
-# ifndef __this_cpu_write_1
-# define __this_cpu_write_1(pcp, val) __this_cpu_generic_to_op((pcp), (val), =)
+#ifndef raw_cpu_write
+# ifndef raw_cpu_write_1
+# define raw_cpu_write_1(pcp, val) raw_cpu_generic_to_op((pcp), (val), =)
# endif
-# ifndef __this_cpu_write_2
-# define __this_cpu_write_2(pcp, val) __this_cpu_generic_to_op((pcp), (val), =)
+# ifndef raw_cpu_write_2
+# define raw_cpu_write_2(pcp, val) raw_cpu_generic_to_op((pcp), (val), =)
# endif
-# ifndef __this_cpu_write_4
-# define __this_cpu_write_4(pcp, val) __this_cpu_generic_to_op((pcp), (val), =)
+# ifndef raw_cpu_write_4
+# define raw_cpu_write_4(pcp, val) raw_cpu_generic_to_op((pcp), (val), =)
# endif
-# ifndef __this_cpu_write_8
-# define __this_cpu_write_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), =)
+# ifndef raw_cpu_write_8
+# define raw_cpu_write_8(pcp, val) raw_cpu_generic_to_op((pcp), (val), =)
# endif
-# define __this_cpu_write(pcp, val) __pcpu_size_call(__this_cpu_write_, (pcp), (val))
+# define raw_cpu_write(pcp, val) __pcpu_size_call(raw_cpu_write_, (pcp), (val))
#endif
-#ifndef __this_cpu_add
-# ifndef __this_cpu_add_1
-# define __this_cpu_add_1(pcp, val) __this_cpu_generic_to_op((pcp), (val), +=)
+#ifndef raw_cpu_add
+# ifndef raw_cpu_add_1
+# define raw_cpu_add_1(pcp, val) raw_cpu_generic_to_op((pcp), (val), +=)
# endif
-# ifndef __this_cpu_add_2
-# define __this_cpu_add_2(pcp, val) __this_cpu_generic_to_op((pcp), (val), +=)
+# ifndef raw_cpu_add_2
+# define raw_cpu_add_2(pcp, val) raw_cpu_generic_to_op((pcp), (val), +=)
# endif
-# ifndef __this_cpu_add_4
-# define __this_cpu_add_4(pcp, val) __this_cpu_generic_to_op((pcp), (val), +=)
+# ifndef raw_cpu_add_4
+# define raw_cpu_add_4(pcp, val) raw_cpu_generic_to_op((pcp), (val), +=)
# endif
-# ifndef __this_cpu_add_8
-# define __this_cpu_add_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), +=)
+# ifndef raw_cpu_add_8
+# define raw_cpu_add_8(pcp, val) raw_cpu_generic_to_op((pcp), (val), +=)
# endif
-# define __this_cpu_add(pcp, val) __pcpu_size_call(__this_cpu_add_, (pcp), (val))
+# define raw_cpu_add(pcp, val) __pcpu_size_call(raw_cpu_add_, (pcp), (val))
#endif
-#ifndef __this_cpu_sub
-# define __this_cpu_sub(pcp, val) __this_cpu_add((pcp), -(val))
+#ifndef raw_cpu_sub
+# define raw_cpu_sub(pcp, val) raw_cpu_add((pcp), -(val))
#endif
-#ifndef __this_cpu_inc
-# define __this_cpu_inc(pcp) __this_cpu_add((pcp), 1)
+#ifndef raw_cpu_inc
+# define raw_cpu_inc(pcp) raw_cpu_add((pcp), 1)
#endif
-#ifndef __this_cpu_dec
-# define __this_cpu_dec(pcp) __this_cpu_sub((pcp), 1)
+#ifndef raw_cpu_dec
+# define raw_cpu_dec(pcp) raw_cpu_sub((pcp), 1)
#endif
-#ifndef __this_cpu_and
-# ifndef __this_cpu_and_1
-# define __this_cpu_and_1(pcp, val) __this_cpu_generic_to_op((pcp), (val), &=)
+#ifndef raw_cpu_and
+# ifndef raw_cpu_and_1
+# define raw_cpu_and_1(pcp, val) raw_cpu_generic_to_op((pcp), (val), &=)
# endif
-# ifndef __this_cpu_and_2
-# define __this_cpu_and_2(pcp, val) __this_cpu_generic_to_op((pcp), (val), &=)
+# ifndef raw_cpu_and_2
+# define raw_cpu_and_2(pcp, val) raw_cpu_generic_to_op((pcp), (val), &=)
# endif
-# ifndef __this_cpu_and_4
-# define __this_cpu_and_4(pcp, val) __this_cpu_generic_to_op((pcp), (val), &=)
+# ifndef raw_cpu_and_4
+# define raw_cpu_and_4(pcp, val) raw_cpu_generic_to_op((pcp), (val), &=)
# endif
-# ifndef __this_cpu_and_8
-# define __this_cpu_and_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), &=)
+# ifndef raw_cpu_and_8
+# define raw_cpu_and_8(pcp, val) raw_cpu_generic_to_op((pcp), (val), &=)
# endif
-# define __this_cpu_and(pcp, val) __pcpu_size_call(__this_cpu_and_, (pcp), (val))
+# define raw_cpu_and(pcp, val) __pcpu_size_call(raw_cpu_and_, (pcp), (val))
#endif
-#ifndef __this_cpu_or
-# ifndef __this_cpu_or_1
-# define __this_cpu_or_1(pcp, val) __this_cpu_generic_to_op((pcp), (val), |=)
+#ifndef raw_cpu_or
+# ifndef raw_cpu_or_1
+# define raw_cpu_or_1(pcp, val) raw_cpu_generic_to_op((pcp), (val), |=)
# endif
-# ifndef __this_cpu_or_2
-# define __this_cpu_or_2(pcp, val) __this_cpu_generic_to_op((pcp), (val), |=)
+# ifndef raw_cpu_or_2
+# define raw_cpu_or_2(pcp, val) raw_cpu_generic_to_op((pcp), (val), |=)
# endif
-# ifndef __this_cpu_or_4
-# define __this_cpu_or_4(pcp, val) __this_cpu_generic_to_op((pcp), (val), |=)
+# ifndef raw_cpu_or_4
+# define raw_cpu_or_4(pcp, val) raw_cpu_generic_to_op((pcp), (val), |=)
# endif
-# ifndef __this_cpu_or_8
-# define __this_cpu_or_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), |=)
+# ifndef raw_cpu_or_8
+# define raw_cpu_or_8(pcp, val) raw_cpu_generic_to_op((pcp), (val), |=)
# endif
-# define __this_cpu_or(pcp, val) __pcpu_size_call(__this_cpu_or_, (pcp), (val))
+# define raw_cpu_or(pcp, val) __pcpu_size_call(raw_cpu_or_, (pcp), (val))
#endif
-#ifndef __this_cpu_xor
-# ifndef __this_cpu_xor_1
-# define __this_cpu_xor_1(pcp, val) __this_cpu_generic_to_op((pcp), (val), ^=)
+#ifndef raw_cpu_xor
+# ifndef raw_cpu_xor_1
+# define raw_cpu_xor_1(pcp, val) raw_cpu_generic_to_op((pcp), (val), ^=)
# endif
-# ifndef __this_cpu_xor_2
-# define __this_cpu_xor_2(pcp, val) __this_cpu_generic_to_op((pcp), (val), ^=)
+# ifndef raw_cpu_xor_2
+# define raw_cpu_xor_2(pcp, val) raw_cpu_generic_to_op((pcp), (val), ^=)
# endif
-# ifndef __this_cpu_xor_4
-# define __this_cpu_xor_4(pcp, val) __this_cpu_generic_to_op((pcp), (val), ^=)
+# ifndef raw_cpu_xor_4
+# define raw_cpu_xor_4(pcp, val) raw_cpu_generic_to_op((pcp), (val), ^=)
# endif
-# ifndef __this_cpu_xor_8
-# define __this_cpu_xor_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), ^=)
+# ifndef raw_cpu_xor_8
+# define raw_cpu_xor_8(pcp, val) raw_cpu_generic_to_op((pcp), (val), ^=)
# endif
-# define __this_cpu_xor(pcp, val) __pcpu_size_call(__this_cpu_xor_, (pcp), (val))
+# define raw_cpu_xor(pcp, val) __pcpu_size_call(raw_cpu_xor_, (pcp), (val))
#endif
-#define __this_cpu_generic_add_return(pcp, val) \
+#define raw_cpu_generic_add_return(pcp, val) \
({ \
- __this_cpu_add(pcp, val); \
- __this_cpu_read(pcp); \
+ raw_cpu_add(pcp, val); \
+ raw_cpu_read(pcp); \
})
-#ifndef __this_cpu_add_return
-# ifndef __this_cpu_add_return_1
-# define __this_cpu_add_return_1(pcp, val) __this_cpu_generic_add_return(pcp, val)
+#ifndef raw_cpu_add_return
+# ifndef raw_cpu_add_return_1
+# define raw_cpu_add_return_1(pcp, val) raw_cpu_generic_add_return(pcp, val)
# endif
-# ifndef __this_cpu_add_return_2
-# define __this_cpu_add_return_2(pcp, val) __this_cpu_generic_add_return(pcp, val)
+# ifndef raw_cpu_add_return_2
+# define raw_cpu_add_return_2(pcp, val) raw_cpu_generic_add_return(pcp, val)
# endif
-# ifndef __this_cpu_add_return_4
-# define __this_cpu_add_return_4(pcp, val) __this_cpu_generic_add_return(pcp, val)
+# ifndef raw_cpu_add_return_4
+# define raw_cpu_add_return_4(pcp, val) raw_cpu_generic_add_return(pcp, val)
# endif
-# ifndef __this_cpu_add_return_8
-# define __this_cpu_add_return_8(pcp, val) __this_cpu_generic_add_return(pcp, val)
+# ifndef raw_cpu_add_return_8
+# define raw_cpu_add_return_8(pcp, val) raw_cpu_generic_add_return(pcp, val)
# endif
-# define __this_cpu_add_return(pcp, val) \
- __pcpu_size_call_return2(__this_cpu_add_return_, pcp, val)
+# define raw_cpu_add_return(pcp, val) \
+ __pcpu_size_call_return2(raw_cpu_add_return_, pcp, val)
#endif
-#define __this_cpu_sub_return(pcp, val) __this_cpu_add_return(pcp, -(val))
-#define __this_cpu_inc_return(pcp) __this_cpu_add_return(pcp, 1)
-#define __this_cpu_dec_return(pcp) __this_cpu_add_return(pcp, -1)
+#define raw_cpu_sub_return(pcp, val) raw_cpu_add_return(pcp, -(val))
+#define raw_cpu_inc_return(pcp) raw_cpu_add_return(pcp, 1)
+#define raw_cpu_dec_return(pcp) raw_cpu_add_return(pcp, -1)
-#define __this_cpu_generic_xchg(pcp, nval) \
+#define raw_cpu_generic_xchg(pcp, nval) \
({ typeof(pcp) ret__; \
- ret__ = __this_cpu_read(pcp); \
- __this_cpu_write(pcp, nval); \
+ ret__ = raw_cpu_read(pcp); \
+ raw_cpu_write(pcp, nval); \
ret__; \
})
-#ifndef __this_cpu_xchg
-# ifndef __this_cpu_xchg_1
-# define __this_cpu_xchg_1(pcp, nval) __this_cpu_generic_xchg(pcp, nval)
+#ifndef raw_cpu_xchg
+# ifndef raw_cpu_xchg_1
+# define raw_cpu_xchg_1(pcp, nval) raw_cpu_generic_xchg(pcp, nval)
# endif
-# ifndef __this_cpu_xchg_2
-# define __this_cpu_xchg_2(pcp, nval) __this_cpu_generic_xchg(pcp, nval)
+# ifndef raw_cpu_xchg_2
+# define raw_cpu_xchg_2(pcp, nval) raw_cpu_generic_xchg(pcp, nval)
# endif
-# ifndef __this_cpu_xchg_4
-# define __this_cpu_xchg_4(pcp, nval) __this_cpu_generic_xchg(pcp, nval)
+# ifndef raw_cpu_xchg_4
+# define raw_cpu_xchg_4(pcp, nval) raw_cpu_generic_xchg(pcp, nval)
# endif
-# ifndef __this_cpu_xchg_8
-# define __this_cpu_xchg_8(pcp, nval) __this_cpu_generic_xchg(pcp, nval)
+# ifndef raw_cpu_xchg_8
+# define raw_cpu_xchg_8(pcp, nval) raw_cpu_generic_xchg(pcp, nval)
# endif
-# define __this_cpu_xchg(pcp, nval) \
- __pcpu_size_call_return2(__this_cpu_xchg_, (pcp), nval)
+# define raw_cpu_xchg(pcp, nval) \
+ __pcpu_size_call_return2(raw_cpu_xchg_, (pcp), nval)
#endif
-#define __this_cpu_generic_cmpxchg(pcp, oval, nval) \
+#define raw_cpu_generic_cmpxchg(pcp, oval, nval) \
({ \
typeof(pcp) ret__; \
- ret__ = __this_cpu_read(pcp); \
+ ret__ = raw_cpu_read(pcp); \
if (ret__ == (oval)) \
- __this_cpu_write(pcp, nval); \
+ raw_cpu_write(pcp, nval); \
ret__; \
})
-#ifndef __this_cpu_cmpxchg
-# ifndef __this_cpu_cmpxchg_1
-# define __this_cpu_cmpxchg_1(pcp, oval, nval) __this_cpu_generic_cmpxchg(pcp, oval, nval)
+#ifndef raw_cpu_cmpxchg
+# ifndef raw_cpu_cmpxchg_1
+# define raw_cpu_cmpxchg_1(pcp, oval, nval) raw_cpu_generic_cmpxchg(pcp, oval, nval)
# endif
-# ifndef __this_cpu_cmpxchg_2
-# define __this_cpu_cmpxchg_2(pcp, oval, nval) __this_cpu_generic_cmpxchg(pcp, oval, nval)
+# ifndef raw_cpu_cmpxchg_2
+# define raw_cpu_cmpxchg_2(pcp, oval, nval) raw_cpu_generic_cmpxchg(pcp, oval, nval)
# endif
-# ifndef __this_cpu_cmpxchg_4
-# define __this_cpu_cmpxchg_4(pcp, oval, nval) __this_cpu_generic_cmpxchg(pcp, oval, nval)
+# ifndef raw_cpu_cmpxchg_4
+# define raw_cpu_cmpxchg_4(pcp, oval, nval) raw_cpu_generic_cmpxchg(pcp, oval, nval)
# endif
-# ifndef __this_cpu_cmpxchg_8
-# define __this_cpu_cmpxchg_8(pcp, oval, nval) __this_cpu_generic_cmpxchg(pcp, oval, nval)
+# ifndef raw_cpu_cmpxchg_8
+# define raw_cpu_cmpxchg_8(pcp, oval, nval) raw_cpu_generic_cmpxchg(pcp, oval, nval)
# endif
-# define __this_cpu_cmpxchg(pcp, oval, nval) \
- __pcpu_size_call_return2(__this_cpu_cmpxchg_, pcp, oval, nval)
+# define raw_cpu_cmpxchg(pcp, oval, nval) \
+ __pcpu_size_call_return2(raw_cpu_cmpxchg_, pcp, oval, nval)
#endif
-#define __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+#define raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
({ \
int __ret = 0; \
- if (__this_cpu_read(pcp1) == (oval1) && \
- __this_cpu_read(pcp2) == (oval2)) { \
- __this_cpu_write(pcp1, (nval1)); \
- __this_cpu_write(pcp2, (nval2)); \
+ if (raw_cpu_read(pcp1) == (oval1) && \
+ raw_cpu_read(pcp2) == (oval2)) { \
+ raw_cpu_write(pcp1, (nval1)); \
+ raw_cpu_write(pcp2, (nval2)); \
__ret = 1; \
} \
(__ret); \
})
-#ifndef __this_cpu_cmpxchg_double
-# ifndef __this_cpu_cmpxchg_double_1
-# define __this_cpu_cmpxchg_double_1(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
+#ifndef raw_cpu_cmpxchg_double
+# ifndef raw_cpu_cmpxchg_double_1
+# define raw_cpu_cmpxchg_double_1(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
# endif
-# ifndef __this_cpu_cmpxchg_double_2
-# define __this_cpu_cmpxchg_double_2(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
+# ifndef raw_cpu_cmpxchg_double_2
+# define raw_cpu_cmpxchg_double_2(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
# endif
-# ifndef __this_cpu_cmpxchg_double_4
-# define __this_cpu_cmpxchg_double_4(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
+# ifndef raw_cpu_cmpxchg_double_4
+# define raw_cpu_cmpxchg_double_4(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
# endif
-# ifndef __this_cpu_cmpxchg_double_8
-# define __this_cpu_cmpxchg_double_8(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
+# ifndef raw_cpu_cmpxchg_double_8
+# define raw_cpu_cmpxchg_double_8(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
# endif
-# define __this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __pcpu_double_call_return_bool(__this_cpu_cmpxchg_double_, (pcp1), (pcp2), (oval1), (oval2), (nval1), (nval2))
+# define raw_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ __pcpu_double_call_return_bool(raw_cpu_cmpxchg_double_, (pcp1), (pcp2), (oval1), (oval2), (nval1), (nval2))
#endif
#endif /* __LINUX_PERCPU_H */
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-16 16:25 ` Peter Zijlstra
@ 2013-10-16 16:52 ` Steven Rostedt
2013-10-16 17:11 ` Peter Zijlstra
2013-10-16 18:38 ` Peter Zijlstra
1 sibling, 1 reply; 33+ messages in thread
From: Steven Rostedt @ 2013-10-16 16:52 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Christoph Lameter, Tejun Heo, akpm, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Wed, 16 Oct 2013 18:25:37 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
> But yes, the way its set-up an arch could indeed provide __this_cup_$op
> itself -- without providing the _$n variants; in which case the
> raw_cpu_$op provided by you is broken.
>
> Can't we have a 'simple' coccinelle script rename the entire __this_cpu*
> implementation over to raw_cpu* and then provide generic __this_cpu* ->
> raw_cpu maps?
>
Perhaps we should match the way spinlocks are.
this_cpu*() be the normal use.
raw_this_cpu() could perhaps not do the checks?
arch_this_cpu() be the architecture specific version of this_cpu*
-- Steve
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-16 16:52 ` Steven Rostedt
@ 2013-10-16 17:11 ` Peter Zijlstra
2013-10-16 17:39 ` Steven Rostedt
0 siblings, 1 reply; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-16 17:11 UTC (permalink / raw)
To: Steven Rostedt
Cc: Christoph Lameter, Tejun Heo, akpm, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Wed, Oct 16, 2013 at 12:52:38PM -0400, Steven Rostedt wrote:
> On Wed, 16 Oct 2013 18:25:37 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
>
> > But yes, the way its set-up an arch could indeed provide __this_cup_$op
> > itself -- without providing the _$n variants; in which case the
> > raw_cpu_$op provided by you is broken.
> >
> > Can't we have a 'simple' coccinelle script rename the entire __this_cpu*
> > implementation over to raw_cpu* and then provide generic __this_cpu* ->
> > raw_cpu maps?
> >
>
> Perhaps we should match the way spinlocks are.
>
>
> this_cpu*() be the normal use.
>
> raw_this_cpu() could perhaps not do the checks?
>
> arch_this_cpu() be the architecture specific version of this_cpu*
In that case we'd need to do something like:
this_cpu_$op -> this_cpu_$op_irq (disables irqs itself)
__this_cpu_$op -> this_cpu_$op (with check)
-> raw_cpu_$op (without the check)
I don't think the arch bits feature heavily for percpu; normally an arch
provides __this_cpu_$op_$n; raw_cpu_$op_$n in my latest proposal.
Anyway; I don't think the spinlock pattern matches too good and I don't
mind the proposed:
this_cpu_$op (disables IRQs itself)
__this_cpu_$op (with preemption check)
raw_cpu_$op (without preemption check)
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-16 17:11 ` Peter Zijlstra
@ 2013-10-16 17:39 ` Steven Rostedt
0 siblings, 0 replies; 33+ messages in thread
From: Steven Rostedt @ 2013-10-16 17:39 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Christoph Lameter, Tejun Heo, akpm, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Wed, 16 Oct 2013 19:11:53 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
>
> Anyway; I don't think the spinlock pattern matches too good and I don't
> mind the proposed:
>
> this_cpu_$op (disables IRQs itself)
> __this_cpu_$op (with preemption check)
> raw_cpu_$op (without preemption check)
>
That sounds fine too.
-- Steve
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-16 16:25 ` Peter Zijlstra
2013-10-16 16:52 ` Steven Rostedt
@ 2013-10-16 18:38 ` Peter Zijlstra
2013-10-17 19:22 ` Christoph Lameter
1 sibling, 1 reply; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-16 18:38 UTC (permalink / raw)
To: Christoph Lameter
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Wed, Oct 16, 2013 at 06:25:37PM +0200, Peter Zijlstra wrote:
> To rename __this_cpu to raw_cpu; it would then need a little manual
> fixup and generic __this_cpu -> raW_cpu map, which can add the
> preemption check.
Something like so perhaps... only lightly compile tested
(x86_64-defconfig).
---
arch/x86/include/asm/percpu.h | 108 ++++++------
arch/x86/include/asm/preempt.h | 16 +-
include/asm-generic/percpu.h | 15 +-
include/linux/percpu.h | 385 +++++++++++++++++++++++++----------------
lib/smp_processor_id.c | 19 +-
5 files changed, 319 insertions(+), 224 deletions(-)
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 0da5200..a053103 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -52,7 +52,7 @@
* Compared to the generic __my_cpu_offset version, the following
* saves one instruction and avoids clobbering a temp register.
*/
-#define __this_cpu_ptr(ptr) \
+#define raw_cpu_ptr(ptr) \
({ \
unsigned long tcp_ptr__; \
__verify_pcpu_ptr(ptr); \
@@ -361,28 +361,28 @@ do { \
*/
#define this_cpu_read_stable(var) percpu_from_op("mov", var, "p" (&(var)))
-#define __this_cpu_read_1(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
-#define __this_cpu_read_2(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
-#define __this_cpu_read_4(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
-
-#define __this_cpu_write_1(pcp, val) percpu_to_op("mov", (pcp), val)
-#define __this_cpu_write_2(pcp, val) percpu_to_op("mov", (pcp), val)
-#define __this_cpu_write_4(pcp, val) percpu_to_op("mov", (pcp), val)
-#define __this_cpu_add_1(pcp, val) percpu_add_op((pcp), val)
-#define __this_cpu_add_2(pcp, val) percpu_add_op((pcp), val)
-#define __this_cpu_add_4(pcp, val) percpu_add_op((pcp), val)
-#define __this_cpu_and_1(pcp, val) percpu_to_op("and", (pcp), val)
-#define __this_cpu_and_2(pcp, val) percpu_to_op("and", (pcp), val)
-#define __this_cpu_and_4(pcp, val) percpu_to_op("and", (pcp), val)
-#define __this_cpu_or_1(pcp, val) percpu_to_op("or", (pcp), val)
-#define __this_cpu_or_2(pcp, val) percpu_to_op("or", (pcp), val)
-#define __this_cpu_or_4(pcp, val) percpu_to_op("or", (pcp), val)
-#define __this_cpu_xor_1(pcp, val) percpu_to_op("xor", (pcp), val)
-#define __this_cpu_xor_2(pcp, val) percpu_to_op("xor", (pcp), val)
-#define __this_cpu_xor_4(pcp, val) percpu_to_op("xor", (pcp), val)
-#define __this_cpu_xchg_1(pcp, val) percpu_xchg_op(pcp, val)
-#define __this_cpu_xchg_2(pcp, val) percpu_xchg_op(pcp, val)
-#define __this_cpu_xchg_4(pcp, val) percpu_xchg_op(pcp, val)
+#define raw_cpu_read_1(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
+#define raw_cpu_read_2(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
+#define raw_cpu_read_4(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
+
+#define raw_cpu_write_1(pcp, val) percpu_to_op("mov", (pcp), val)
+#define raw_cpu_write_2(pcp, val) percpu_to_op("mov", (pcp), val)
+#define raw_cpu_write_4(pcp, val) percpu_to_op("mov", (pcp), val)
+#define raw_cpu_add_1(pcp, val) percpu_add_op((pcp), val)
+#define raw_cpu_add_2(pcp, val) percpu_add_op((pcp), val)
+#define raw_cpu_add_4(pcp, val) percpu_add_op((pcp), val)
+#define raw_cpu_and_1(pcp, val) percpu_to_op("and", (pcp), val)
+#define raw_cpu_and_2(pcp, val) percpu_to_op("and", (pcp), val)
+#define raw_cpu_and_4(pcp, val) percpu_to_op("and", (pcp), val)
+#define raw_cpu_or_1(pcp, val) percpu_to_op("or", (pcp), val)
+#define raw_cpu_or_2(pcp, val) percpu_to_op("or", (pcp), val)
+#define raw_cpu_or_4(pcp, val) percpu_to_op("or", (pcp), val)
+#define raw_cpu_xor_1(pcp, val) percpu_to_op("xor", (pcp), val)
+#define raw_cpu_xor_2(pcp, val) percpu_to_op("xor", (pcp), val)
+#define raw_cpu_xor_4(pcp, val) percpu_to_op("xor", (pcp), val)
+#define raw_cpu_xchg_1(pcp, val) percpu_xchg_op(pcp, val)
+#define raw_cpu_xchg_2(pcp, val) percpu_xchg_op(pcp, val)
+#define raw_cpu_xchg_4(pcp, val) percpu_xchg_op(pcp, val)
#define this_cpu_read_1(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
#define this_cpu_read_2(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
@@ -406,16 +406,16 @@ do { \
#define this_cpu_xchg_2(pcp, nval) percpu_xchg_op(pcp, nval)
#define this_cpu_xchg_4(pcp, nval) percpu_xchg_op(pcp, nval)
-#define __this_cpu_add_return_1(pcp, val) percpu_add_return_op(pcp, val)
-#define __this_cpu_add_return_2(pcp, val) percpu_add_return_op(pcp, val)
-#define __this_cpu_add_return_4(pcp, val) percpu_add_return_op(pcp, val)
-#define __this_cpu_cmpxchg_1(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
-#define __this_cpu_cmpxchg_2(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
-#define __this_cpu_cmpxchg_4(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
+#define raw_cpu_add_return_1(pcp, val) percpu_add_return_op(pcp, val)
+#define raw_cpu_add_return_2(pcp, val) percpu_add_return_op(pcp, val)
+#define raw_cpu_add_return_4(pcp, val) percpu_add_return_op(pcp, val)
+#define raw_cpu_cmpxchg_1(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
+#define raw_cpu_cmpxchg_2(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
+#define raw_cpu_cmpxchg_4(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
-#define this_cpu_add_return_1(pcp, val) percpu_add_return_op(pcp, val)
-#define this_cpu_add_return_2(pcp, val) percpu_add_return_op(pcp, val)
-#define this_cpu_add_return_4(pcp, val) percpu_add_return_op(pcp, val)
+#define this_cpu_add_return_1(pcp, val) percpu_add_return_op(pcp, val)
+#define this_cpu_add_return_2(pcp, val) percpu_add_return_op(pcp, val)
+#define this_cpu_add_return_4(pcp, val) percpu_add_return_op(pcp, val)
#define this_cpu_cmpxchg_1(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
#define this_cpu_cmpxchg_2(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
#define this_cpu_cmpxchg_4(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
@@ -432,7 +432,7 @@ do { \
__ret; \
})
-#define __this_cpu_cmpxchg_double_4 percpu_cmpxchg8b_double
+#define raw_cpu_cmpxchg_double_4 percpu_cmpxchg8b_double
#define this_cpu_cmpxchg_double_4 percpu_cmpxchg8b_double
#endif /* CONFIG_X86_CMPXCHG64 */
@@ -441,24 +441,24 @@ do { \
* 32 bit must fall back to generic operations.
*/
#ifdef CONFIG_X86_64
-#define __this_cpu_read_8(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
-#define __this_cpu_write_8(pcp, val) percpu_to_op("mov", (pcp), val)
-#define __this_cpu_add_8(pcp, val) percpu_add_op((pcp), val)
-#define __this_cpu_and_8(pcp, val) percpu_to_op("and", (pcp), val)
-#define __this_cpu_or_8(pcp, val) percpu_to_op("or", (pcp), val)
-#define __this_cpu_xor_8(pcp, val) percpu_to_op("xor", (pcp), val)
-#define __this_cpu_add_return_8(pcp, val) percpu_add_return_op(pcp, val)
-#define __this_cpu_xchg_8(pcp, nval) percpu_xchg_op(pcp, nval)
-#define __this_cpu_cmpxchg_8(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
-
-#define this_cpu_read_8(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
-#define this_cpu_write_8(pcp, val) percpu_to_op("mov", (pcp), val)
-#define this_cpu_add_8(pcp, val) percpu_add_op((pcp), val)
-#define this_cpu_and_8(pcp, val) percpu_to_op("and", (pcp), val)
-#define this_cpu_or_8(pcp, val) percpu_to_op("or", (pcp), val)
-#define this_cpu_xor_8(pcp, val) percpu_to_op("xor", (pcp), val)
-#define this_cpu_add_return_8(pcp, val) percpu_add_return_op(pcp, val)
-#define this_cpu_xchg_8(pcp, nval) percpu_xchg_op(pcp, nval)
+#define raw_cpu_read_8(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
+#define raw_cpu_write_8(pcp, val) percpu_to_op("mov", (pcp), val)
+#define raw_cpu_add_8(pcp, val) percpu_add_op((pcp), val)
+#define raw_cpu_and_8(pcp, val) percpu_to_op("and", (pcp), val)
+#define raw_cpu_or_8(pcp, val) percpu_to_op("or", (pcp), val)
+#define raw_cpu_xor_8(pcp, val) percpu_to_op("xor", (pcp), val)
+#define raw_cpu_add_return_8(pcp, val) percpu_add_return_op(pcp, val)
+#define raw_cpu_xchg_8(pcp, nval) percpu_xchg_op(pcp, nval)
+#define raw_cpu_cmpxchg_8(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
+
+#define this_cpu_read_8(pcp) percpu_from_op("mov", (pcp), "m"(pcp))
+#define this_cpu_write_8(pcp, val) percpu_to_op("mov", (pcp), val)
+#define this_cpu_add_8(pcp, val) percpu_add_op((pcp), val)
+#define this_cpu_and_8(pcp, val) percpu_to_op("and", (pcp), val)
+#define this_cpu_or_8(pcp, val) percpu_to_op("or", (pcp), val)
+#define this_cpu_xor_8(pcp, val) percpu_to_op("xor", (pcp), val)
+#define this_cpu_add_return_8(pcp, val) percpu_add_return_op(pcp, val)
+#define this_cpu_xchg_8(pcp, nval) percpu_xchg_op(pcp, nval)
#define this_cpu_cmpxchg_8(pcp, oval, nval) percpu_cmpxchg_op(pcp, oval, nval)
/*
@@ -481,7 +481,7 @@ do { \
__ret; \
})
-#define __this_cpu_cmpxchg_double_8 percpu_cmpxchg16b_double
+#define raw_cpu_cmpxchg_double_8 percpu_cmpxchg16b_double
#define this_cpu_cmpxchg_double_8 percpu_cmpxchg16b_double
#endif
@@ -502,9 +502,9 @@ static __always_inline int x86_this_cpu_constant_test_bit(unsigned int nr,
unsigned long __percpu *a = (unsigned long *)addr + nr / BITS_PER_LONG;
#ifdef CONFIG_X86_64
- return ((1UL << (nr % BITS_PER_LONG)) & __this_cpu_read_8(*a)) != 0;
+ return ((1UL << (nr % BITS_PER_LONG)) & raw_cpu_read_8(*a)) != 0;
#else
- return ((1UL << (nr % BITS_PER_LONG)) & __this_cpu_read_4(*a)) != 0;
+ return ((1UL << (nr % BITS_PER_LONG)) & raw_cpu_read_4(*a)) != 0;
#endif
}
diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
index 8729723..95e3532 100644
--- a/arch/x86/include/asm/preempt.h
+++ b/arch/x86/include/asm/preempt.h
@@ -13,12 +13,12 @@ DECLARE_PER_CPU(int, __preempt_count);
*/
static __always_inline int preempt_count(void)
{
- return __this_cpu_read_4(__preempt_count) & ~PREEMPT_NEED_RESCHED;
+ return raw_cpu_read_4(__preempt_count) & ~PREEMPT_NEED_RESCHED;
}
static __always_inline void preempt_count_set(int pc)
{
- __this_cpu_write_4(__preempt_count, pc);
+ raw_cpu_write_4(__preempt_count, pc);
}
/*
@@ -47,17 +47,17 @@ static __always_inline void preempt_count_set(int pc)
static __always_inline void set_preempt_need_resched(void)
{
- __this_cpu_and_4(__preempt_count, ~PREEMPT_NEED_RESCHED);
+ raw_cpu_and_4(__preempt_count, ~PREEMPT_NEED_RESCHED);
}
static __always_inline void clear_preempt_need_resched(void)
{
- __this_cpu_or_4(__preempt_count, PREEMPT_NEED_RESCHED);
+ raw_cpu_or_4(__preempt_count, PREEMPT_NEED_RESCHED);
}
static __always_inline bool test_preempt_need_resched(void)
{
- return !(__this_cpu_read_4(__preempt_count) & PREEMPT_NEED_RESCHED);
+ return !(raw_cpu_read_4(__preempt_count) & PREEMPT_NEED_RESCHED);
}
/*
@@ -66,12 +66,12 @@ static __always_inline bool test_preempt_need_resched(void)
static __always_inline void __preempt_count_add(int val)
{
- __this_cpu_add_4(__preempt_count, val);
+ raw_cpu_add_4(__preempt_count, val);
}
static __always_inline void __preempt_count_sub(int val)
{
- __this_cpu_add_4(__preempt_count, -val);
+ raw_cpu_add_4(__preempt_count, -val);
}
static __always_inline bool __preempt_count_dec_and_test(void)
@@ -84,7 +84,7 @@ static __always_inline bool __preempt_count_dec_and_test(void)
*/
static __always_inline bool should_resched(void)
{
- return unlikely(!__this_cpu_read_4(__preempt_count));
+ return unlikely(!raw_cpu_read_4(__preempt_count));
}
#ifdef CONFIG_PREEMPT
diff --git a/include/asm-generic/percpu.h b/include/asm-generic/percpu.h
index d17784e..b5aca54 100644
--- a/include/asm-generic/percpu.h
+++ b/include/asm-generic/percpu.h
@@ -56,17 +56,18 @@ extern unsigned long __per_cpu_offset[NR_CPUS];
#define per_cpu(var, cpu) \
(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
-#ifndef __this_cpu_ptr
-#define __this_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset)
+#ifndef raw_cpu_ptr
+#define raw_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset)
#endif
+
#ifdef CONFIG_DEBUG_PREEMPT
#define this_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, my_cpu_offset)
#else
-#define this_cpu_ptr(ptr) __this_cpu_ptr(ptr)
+#define this_cpu_ptr(ptr) raw_cpu_ptr(ptr)
#endif
-#define __get_cpu_var(var) (*this_cpu_ptr(&(var)))
-#define __raw_get_cpu_var(var) (*__this_cpu_ptr(&(var)))
+#define __get_cpu_var(var) (*this_cpu_ptr(&(var)))
+#define __raw_get_cpu_var(var) (*raw_cpu_ptr(&(var)))
#ifdef CONFIG_HAVE_SETUP_PER_CPU_AREA
extern void setup_per_cpu_areas(void);
@@ -83,10 +84,12 @@ extern void setup_per_cpu_areas(void);
#define __get_cpu_var(var) (*VERIFY_PERCPU_PTR(&(var)))
#define __raw_get_cpu_var(var) (*VERIFY_PERCPU_PTR(&(var)))
#define this_cpu_ptr(ptr) per_cpu_ptr(ptr, 0)
-#define __this_cpu_ptr(ptr) this_cpu_ptr(ptr)
+#define raw_cpu_ptr(ptr) this_cpu_ptr(ptr)
#endif /* SMP */
+#define __this_cpu_ptr(ptr) this_cpu_ptr(ptr)
+
#ifndef PER_CPU_BASE_SECTION
#ifdef CONFIG_SMP
#define PER_CPU_BASE_SECTION ".data..percpu"
diff --git a/include/linux/percpu.h b/include/linux/percpu.h
index cc88172..b88f03f 100644
--- a/include/linux/percpu.h
+++ b/include/linux/percpu.h
@@ -295,7 +295,7 @@ do { \
do { \
unsigned long flags; \
raw_local_irq_save(flags); \
- *__this_cpu_ptr(&(pcp)) op val; \
+ *raw_cpu_ptr(&(pcp)) op val; \
raw_local_irq_restore(flags); \
} while (0)
@@ -396,8 +396,8 @@ do { \
typeof(pcp) ret__; \
unsigned long flags; \
raw_local_irq_save(flags); \
- __this_cpu_add(pcp, val); \
- ret__ = __this_cpu_read(pcp); \
+ raw_cpu_add(pcp, val); \
+ ret__ = raw_cpu_read(pcp); \
raw_local_irq_restore(flags); \
ret__; \
})
@@ -426,8 +426,8 @@ do { \
({ typeof(pcp) ret__; \
unsigned long flags; \
raw_local_irq_save(flags); \
- ret__ = __this_cpu_read(pcp); \
- __this_cpu_write(pcp, nval); \
+ ret__ = raw_cpu_read(pcp); \
+ raw_cpu_write(pcp, nval); \
raw_local_irq_restore(flags); \
ret__; \
})
@@ -454,9 +454,9 @@ do { \
typeof(pcp) ret__; \
unsigned long flags; \
raw_local_irq_save(flags); \
- ret__ = __this_cpu_read(pcp); \
+ ret__ = raw_cpu_read(pcp); \
if (ret__ == (oval)) \
- __this_cpu_write(pcp, nval); \
+ raw_cpu_write(pcp, nval); \
raw_local_irq_restore(flags); \
ret__; \
})
@@ -491,7 +491,7 @@ do { \
int ret__; \
unsigned long flags; \
raw_local_irq_save(flags); \
- ret__ = __this_cpu_generic_cmpxchg_double(pcp1, pcp2, \
+ ret__ = raw_cpu_generic_cmpxchg_double(pcp1, pcp2, \
oval1, oval2, nval1, nval2); \
raw_local_irq_restore(flags); \
ret__; \
@@ -519,240 +519,321 @@ do { \
#endif
/*
- * Generic percpu operations for context that are safe from preemption/interrupts.
- * Either we do not care about races or the caller has the
- * responsibility of handling preemption/interrupt issues. Arch code can still
- * override these instructions since the arch per cpu code may be more
- * efficient and may actually get race freeness for free (that is the
- * case for x86 for example).
+ * Generic percpu operations for context where we do not want to do any checks
+ * for preemptiosn.
+ *
+ * If there is no other protection through preempt disable and/or disabling
+ * interupts then one of these RMW operations can show unexpected behavior
+ * because the execution thread was rescheduled on another processor or an
+ * interrupt occurred and the same percpu variable was modified from the
+ * interrupt context.
*
- * If there is no other protection through preempt disable and/or
- * disabling interupts then one of these RMW operations can show unexpected
- * behavior because the execution thread was rescheduled on another processor
- * or an interrupt occurred and the same percpu variable was modified from
- * the interrupt context.
+ * Use with care!
*/
-#ifndef __this_cpu_read
-# ifndef __this_cpu_read_1
-# define __this_cpu_read_1(pcp) (*__this_cpu_ptr(&(pcp)))
+#ifndef raw_cpu_read
+# ifndef raw_cpu_read_1
+# define raw_cpu_read_1(pcp) (*raw_cpu_ptr(&(pcp)))
# endif
-# ifndef __this_cpu_read_2
-# define __this_cpu_read_2(pcp) (*__this_cpu_ptr(&(pcp)))
+# ifndef raw_cpu_read_2
+# define raw_cpu_read_2(pcp) (*raw_cpu_ptr(&(pcp)))
# endif
-# ifndef __this_cpu_read_4
-# define __this_cpu_read_4(pcp) (*__this_cpu_ptr(&(pcp)))
+# ifndef raw_cpu_read_4
+# define raw_cpu_read_4(pcp) (*raw_cpu_ptr(&(pcp)))
# endif
-# ifndef __this_cpu_read_8
-# define __this_cpu_read_8(pcp) (*__this_cpu_ptr(&(pcp)))
+# ifndef raw_cpu_read_8
+# define raw_cpu_read_8(pcp) (*raw_cpu_ptr(&(pcp)))
# endif
-# define __this_cpu_read(pcp) __pcpu_size_call_return(__this_cpu_read_, (pcp))
+# define raw_cpu_read(pcp) __pcpu_size_call_return(raw_cpu_read_, (pcp))
#endif
-#define __this_cpu_generic_to_op(pcp, val, op) \
+#define raw_cpu_generic_to_op(pcp, val, op) \
do { \
- *__this_cpu_ptr(&(pcp)) op val; \
+ *raw_cpu_ptr(&(pcp)) op val; \
} while (0)
-#ifndef __this_cpu_write
-# ifndef __this_cpu_write_1
-# define __this_cpu_write_1(pcp, val) __this_cpu_generic_to_op((pcp), (val), =)
+#ifndef raw_cpu_write
+# ifndef raw_cpu_write_1
+# define raw_cpu_write_1(pcp, val) raw_cpu_generic_to_op((pcp), (val), =)
# endif
-# ifndef __this_cpu_write_2
-# define __this_cpu_write_2(pcp, val) __this_cpu_generic_to_op((pcp), (val), =)
+# ifndef raw_cpu_write_2
+# define raw_cpu_write_2(pcp, val) raw_cpu_generic_to_op((pcp), (val), =)
# endif
-# ifndef __this_cpu_write_4
-# define __this_cpu_write_4(pcp, val) __this_cpu_generic_to_op((pcp), (val), =)
+# ifndef raw_cpu_write_4
+# define raw_cpu_write_4(pcp, val) raw_cpu_generic_to_op((pcp), (val), =)
# endif
-# ifndef __this_cpu_write_8
-# define __this_cpu_write_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), =)
+# ifndef raw_cpu_write_8
+# define raw_cpu_write_8(pcp, val) raw_cpu_generic_to_op((pcp), (val), =)
# endif
-# define __this_cpu_write(pcp, val) __pcpu_size_call(__this_cpu_write_, (pcp), (val))
+# define raw_cpu_write(pcp, val) __pcpu_size_call(raw_cpu_write_, (pcp), (val))
#endif
-#ifndef __this_cpu_add
-# ifndef __this_cpu_add_1
-# define __this_cpu_add_1(pcp, val) __this_cpu_generic_to_op((pcp), (val), +=)
+#ifndef raw_cpu_add
+# ifndef raw_cpu_add_1
+# define raw_cpu_add_1(pcp, val) raw_cpu_generic_to_op((pcp), (val), +=)
# endif
-# ifndef __this_cpu_add_2
-# define __this_cpu_add_2(pcp, val) __this_cpu_generic_to_op((pcp), (val), +=)
+# ifndef raw_cpu_add_2
+# define raw_cpu_add_2(pcp, val) raw_cpu_generic_to_op((pcp), (val), +=)
# endif
-# ifndef __this_cpu_add_4
-# define __this_cpu_add_4(pcp, val) __this_cpu_generic_to_op((pcp), (val), +=)
+# ifndef raw_cpu_add_4
+# define raw_cpu_add_4(pcp, val) raw_cpu_generic_to_op((pcp), (val), +=)
# endif
-# ifndef __this_cpu_add_8
-# define __this_cpu_add_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), +=)
+# ifndef raw_cpu_add_8
+# define raw_cpu_add_8(pcp, val) raw_cpu_generic_to_op((pcp), (val), +=)
# endif
-# define __this_cpu_add(pcp, val) __pcpu_size_call(__this_cpu_add_, (pcp), (val))
+# define raw_cpu_add(pcp, val) __pcpu_size_call(raw_cpu_add_, (pcp), (val))
#endif
-#ifndef __this_cpu_sub
-# define __this_cpu_sub(pcp, val) __this_cpu_add((pcp), -(val))
+#ifndef raw_cpu_sub
+# define raw_cpu_sub(pcp, val) raw_cpu_add((pcp), -(val))
#endif
-#ifndef __this_cpu_inc
-# define __this_cpu_inc(pcp) __this_cpu_add((pcp), 1)
+#ifndef raw_cpu_inc
+# define raw_cpu_inc(pcp) raw_cpu_add((pcp), 1)
#endif
-#ifndef __this_cpu_dec
-# define __this_cpu_dec(pcp) __this_cpu_sub((pcp), 1)
+#ifndef raw_cpu_dec
+# define raw_cpu_dec(pcp) raw_cpu_sub((pcp), 1)
#endif
-#ifndef __this_cpu_and
-# ifndef __this_cpu_and_1
-# define __this_cpu_and_1(pcp, val) __this_cpu_generic_to_op((pcp), (val), &=)
+#ifndef raw_cpu_and
+# ifndef raw_cpu_and_1
+# define raw_cpu_and_1(pcp, val) raw_cpu_generic_to_op((pcp), (val), &=)
# endif
-# ifndef __this_cpu_and_2
-# define __this_cpu_and_2(pcp, val) __this_cpu_generic_to_op((pcp), (val), &=)
+# ifndef raw_cpu_and_2
+# define raw_cpu_and_2(pcp, val) raw_cpu_generic_to_op((pcp), (val), &=)
# endif
-# ifndef __this_cpu_and_4
-# define __this_cpu_and_4(pcp, val) __this_cpu_generic_to_op((pcp), (val), &=)
+# ifndef raw_cpu_and_4
+# define raw_cpu_and_4(pcp, val) raw_cpu_generic_to_op((pcp), (val), &=)
# endif
-# ifndef __this_cpu_and_8
-# define __this_cpu_and_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), &=)
+# ifndef raw_cpu_and_8
+# define raw_cpu_and_8(pcp, val) raw_cpu_generic_to_op((pcp), (val), &=)
# endif
-# define __this_cpu_and(pcp, val) __pcpu_size_call(__this_cpu_and_, (pcp), (val))
+# define raw_cpu_and(pcp, val) __pcpu_size_call(raw_cpu_and_, (pcp), (val))
#endif
-#ifndef __this_cpu_or
-# ifndef __this_cpu_or_1
-# define __this_cpu_or_1(pcp, val) __this_cpu_generic_to_op((pcp), (val), |=)
+#ifndef raw_cpu_or
+# ifndef raw_cpu_or_1
+# define raw_cpu_or_1(pcp, val) raw_cpu_generic_to_op((pcp), (val), |=)
# endif
-# ifndef __this_cpu_or_2
-# define __this_cpu_or_2(pcp, val) __this_cpu_generic_to_op((pcp), (val), |=)
+# ifndef raw_cpu_or_2
+# define raw_cpu_or_2(pcp, val) raw_cpu_generic_to_op((pcp), (val), |=)
# endif
-# ifndef __this_cpu_or_4
-# define __this_cpu_or_4(pcp, val) __this_cpu_generic_to_op((pcp), (val), |=)
+# ifndef raw_cpu_or_4
+# define raw_cpu_or_4(pcp, val) raw_cpu_generic_to_op((pcp), (val), |=)
# endif
-# ifndef __this_cpu_or_8
-# define __this_cpu_or_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), |=)
+# ifndef raw_cpu_or_8
+# define raw_cpu_or_8(pcp, val) raw_cpu_generic_to_op((pcp), (val), |=)
# endif
-# define __this_cpu_or(pcp, val) __pcpu_size_call(__this_cpu_or_, (pcp), (val))
+# define raw_cpu_or(pcp, val) __pcpu_size_call(raw_cpu_or_, (pcp), (val))
#endif
-#ifndef __this_cpu_xor
-# ifndef __this_cpu_xor_1
-# define __this_cpu_xor_1(pcp, val) __this_cpu_generic_to_op((pcp), (val), ^=)
+#ifndef raw_cpu_xor
+# ifndef raw_cpu_xor_1
+# define raw_cpu_xor_1(pcp, val) raw_cpu_generic_to_op((pcp), (val), ^=)
# endif
-# ifndef __this_cpu_xor_2
-# define __this_cpu_xor_2(pcp, val) __this_cpu_generic_to_op((pcp), (val), ^=)
+# ifndef raw_cpu_xor_2
+# define raw_cpu_xor_2(pcp, val) raw_cpu_generic_to_op((pcp), (val), ^=)
# endif
-# ifndef __this_cpu_xor_4
-# define __this_cpu_xor_4(pcp, val) __this_cpu_generic_to_op((pcp), (val), ^=)
+# ifndef raw_cpu_xor_4
+# define raw_cpu_xor_4(pcp, val) raw_cpu_generic_to_op((pcp), (val), ^=)
# endif
-# ifndef __this_cpu_xor_8
-# define __this_cpu_xor_8(pcp, val) __this_cpu_generic_to_op((pcp), (val), ^=)
+# ifndef raw_cpu_xor_8
+# define raw_cpu_xor_8(pcp, val) raw_cpu_generic_to_op((pcp), (val), ^=)
# endif
-# define __this_cpu_xor(pcp, val) __pcpu_size_call(__this_cpu_xor_, (pcp), (val))
+# define raw_cpu_xor(pcp, val) __pcpu_size_call(raw_cpu_xor_, (pcp), (val))
#endif
-#define __this_cpu_generic_add_return(pcp, val) \
+#define raw_cpu_generic_add_return(pcp, val) \
({ \
- __this_cpu_add(pcp, val); \
- __this_cpu_read(pcp); \
+ raw_cpu_add(pcp, val); \
+ raw_cpu_read(pcp); \
})
-#ifndef __this_cpu_add_return
-# ifndef __this_cpu_add_return_1
-# define __this_cpu_add_return_1(pcp, val) __this_cpu_generic_add_return(pcp, val)
+#ifndef raw_cpu_add_return
+# ifndef raw_cpu_add_return_1
+# define raw_cpu_add_return_1(pcp, val) raw_cpu_generic_add_return(pcp, val)
# endif
-# ifndef __this_cpu_add_return_2
-# define __this_cpu_add_return_2(pcp, val) __this_cpu_generic_add_return(pcp, val)
+# ifndef raw_cpu_add_return_2
+# define raw_cpu_add_return_2(pcp, val) raw_cpu_generic_add_return(pcp, val)
# endif
-# ifndef __this_cpu_add_return_4
-# define __this_cpu_add_return_4(pcp, val) __this_cpu_generic_add_return(pcp, val)
+# ifndef raw_cpu_add_return_4
+# define raw_cpu_add_return_4(pcp, val) raw_cpu_generic_add_return(pcp, val)
# endif
-# ifndef __this_cpu_add_return_8
-# define __this_cpu_add_return_8(pcp, val) __this_cpu_generic_add_return(pcp, val)
+# ifndef raw_cpu_add_return_8
+# define raw_cpu_add_return_8(pcp, val) raw_cpu_generic_add_return(pcp, val)
# endif
-# define __this_cpu_add_return(pcp, val) \
- __pcpu_size_call_return2(__this_cpu_add_return_, pcp, val)
+# define raw_cpu_add_return(pcp, val) \
+ __pcpu_size_call_return2(raw_cpu_add_return_, pcp, val)
#endif
-#define __this_cpu_sub_return(pcp, val) __this_cpu_add_return(pcp, -(val))
-#define __this_cpu_inc_return(pcp) __this_cpu_add_return(pcp, 1)
-#define __this_cpu_dec_return(pcp) __this_cpu_add_return(pcp, -1)
+#define raw_cpu_sub_return(pcp, val) raw_cpu_add_return(pcp, -(val))
+#define raw_cpu_inc_return(pcp) raw_cpu_add_return(pcp, 1)
+#define raw_cpu_dec_return(pcp) raw_cpu_add_return(pcp, -1)
-#define __this_cpu_generic_xchg(pcp, nval) \
+#define raw_cpu_generic_xchg(pcp, nval) \
({ typeof(pcp) ret__; \
- ret__ = __this_cpu_read(pcp); \
- __this_cpu_write(pcp, nval); \
+ ret__ = raw_cpu_read(pcp); \
+ raw_cpu_write(pcp, nval); \
ret__; \
})
-#ifndef __this_cpu_xchg
-# ifndef __this_cpu_xchg_1
-# define __this_cpu_xchg_1(pcp, nval) __this_cpu_generic_xchg(pcp, nval)
+#ifndef raw_cpu_xchg
+# ifndef raw_cpu_xchg_1
+# define raw_cpu_xchg_1(pcp, nval) raw_cpu_generic_xchg(pcp, nval)
# endif
-# ifndef __this_cpu_xchg_2
-# define __this_cpu_xchg_2(pcp, nval) __this_cpu_generic_xchg(pcp, nval)
+# ifndef raw_cpu_xchg_2
+# define raw_cpu_xchg_2(pcp, nval) raw_cpu_generic_xchg(pcp, nval)
# endif
-# ifndef __this_cpu_xchg_4
-# define __this_cpu_xchg_4(pcp, nval) __this_cpu_generic_xchg(pcp, nval)
+# ifndef raw_cpu_xchg_4
+# define raw_cpu_xchg_4(pcp, nval) raw_cpu_generic_xchg(pcp, nval)
# endif
-# ifndef __this_cpu_xchg_8
-# define __this_cpu_xchg_8(pcp, nval) __this_cpu_generic_xchg(pcp, nval)
+# ifndef raw_cpu_xchg_8
+# define raw_cpu_xchg_8(pcp, nval) raw_cpu_generic_xchg(pcp, nval)
# endif
-# define __this_cpu_xchg(pcp, nval) \
- __pcpu_size_call_return2(__this_cpu_xchg_, (pcp), nval)
+# define raw_cpu_xchg(pcp, nval) \
+ __pcpu_size_call_return2(raw_cpu_xchg_, (pcp), nval)
#endif
-#define __this_cpu_generic_cmpxchg(pcp, oval, nval) \
+#define raw_cpu_generic_cmpxchg(pcp, oval, nval) \
({ \
typeof(pcp) ret__; \
- ret__ = __this_cpu_read(pcp); \
+ ret__ = raw_cpu_read(pcp); \
if (ret__ == (oval)) \
- __this_cpu_write(pcp, nval); \
+ raw_cpu_write(pcp, nval); \
ret__; \
})
-#ifndef __this_cpu_cmpxchg
-# ifndef __this_cpu_cmpxchg_1
-# define __this_cpu_cmpxchg_1(pcp, oval, nval) __this_cpu_generic_cmpxchg(pcp, oval, nval)
+#ifndef raw_cpu_cmpxchg
+# ifndef raw_cpu_cmpxchg_1
+# define raw_cpu_cmpxchg_1(pcp, oval, nval) raw_cpu_generic_cmpxchg(pcp, oval, nval)
# endif
-# ifndef __this_cpu_cmpxchg_2
-# define __this_cpu_cmpxchg_2(pcp, oval, nval) __this_cpu_generic_cmpxchg(pcp, oval, nval)
+# ifndef raw_cpu_cmpxchg_2
+# define raw_cpu_cmpxchg_2(pcp, oval, nval) raw_cpu_generic_cmpxchg(pcp, oval, nval)
# endif
-# ifndef __this_cpu_cmpxchg_4
-# define __this_cpu_cmpxchg_4(pcp, oval, nval) __this_cpu_generic_cmpxchg(pcp, oval, nval)
+# ifndef raw_cpu_cmpxchg_4
+# define raw_cpu_cmpxchg_4(pcp, oval, nval) raw_cpu_generic_cmpxchg(pcp, oval, nval)
# endif
-# ifndef __this_cpu_cmpxchg_8
-# define __this_cpu_cmpxchg_8(pcp, oval, nval) __this_cpu_generic_cmpxchg(pcp, oval, nval)
+# ifndef raw_cpu_cmpxchg_8
+# define raw_cpu_cmpxchg_8(pcp, oval, nval) raw_cpu_generic_cmpxchg(pcp, oval, nval)
# endif
-# define __this_cpu_cmpxchg(pcp, oval, nval) \
- __pcpu_size_call_return2(__this_cpu_cmpxchg_, pcp, oval, nval)
+# define raw_cpu_cmpxchg(pcp, oval, nval) \
+ __pcpu_size_call_return2(raw_cpu_cmpxchg_, pcp, oval, nval)
#endif
-#define __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+#define raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
({ \
int __ret = 0; \
- if (__this_cpu_read(pcp1) == (oval1) && \
- __this_cpu_read(pcp2) == (oval2)) { \
- __this_cpu_write(pcp1, (nval1)); \
- __this_cpu_write(pcp2, (nval2)); \
+ if (raw_cpu_read(pcp1) == (oval1) && \
+ raw_cpu_read(pcp2) == (oval2)) { \
+ raw_cpu_write(pcp1, (nval1)); \
+ raw_cpu_write(pcp2, (nval2)); \
__ret = 1; \
} \
(__ret); \
})
-#ifndef __this_cpu_cmpxchg_double
-# ifndef __this_cpu_cmpxchg_double_1
-# define __this_cpu_cmpxchg_double_1(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
+#ifndef raw_cpu_cmpxchg_double
+# ifndef raw_cpu_cmpxchg_double_1
+# define raw_cpu_cmpxchg_double_1(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
# endif
-# ifndef __this_cpu_cmpxchg_double_2
-# define __this_cpu_cmpxchg_double_2(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
+# ifndef raw_cpu_cmpxchg_double_2
+# define raw_cpu_cmpxchg_double_2(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
# endif
-# ifndef __this_cpu_cmpxchg_double_4
-# define __this_cpu_cmpxchg_double_4(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
+# ifndef raw_cpu_cmpxchg_double_4
+# define raw_cpu_cmpxchg_double_4(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
# endif
-# ifndef __this_cpu_cmpxchg_double_8
-# define __this_cpu_cmpxchg_double_8(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
+# ifndef raw_cpu_cmpxchg_double_8
+# define raw_cpu_cmpxchg_double_8(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ raw_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)
# endif
-# define __this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
- __pcpu_double_call_return_bool(__this_cpu_cmpxchg_double_, (pcp1), (pcp2), (oval1), (oval2), (nval1), (nval2))
+# define raw_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) \
+ __pcpu_double_call_return_bool(raw_cpu_cmpxchg_double_, (pcp1), (pcp2), (oval1), (oval2), (nval1), (nval2))
+#endif
+
+#ifdef CONFIG_DEBUG_PREEMPT
+extern void __this_cpu_preempt_check(const char *msg);
+#else
+static inline void __this_cpu_preempt_check(const char *msg) { }
#endif
+/*
+ * Generic percpu operations for context that are safe from preemption/interrupts.
+ * These functions verify that preemption has been disabled.
+ */
+#define __this_cpu_read(pcp) \
+ (__this_cpu_preempt_check("read"), raw_cpu_read(pcp))
+
+#define __this_cpu_write(pcp, val) \
+do { \
+ __this_cpu_preempt_check("write"); \
+ raw_cpu_write((pcp), (val)); \
+} while (0)
+
+#define __this_cpu_add(pcp, val) \
+do { \
+ __this_cpu_preempt_check("add"); \
+ raw_cpu_add((pcp), (val)); \
+} while (0)
+
+#define __this_cpu_sub(pcp, val) \
+do { \
+ __this_cpu_preempt_check("sub"); \
+ raw_cpu_sub((pcp), (val)); \
+} while (0)
+
+#define __this_cpu_inc(pcp) \
+do { \
+ __this_cpu_preempt_check("inc"); \
+ raw_cpu_inc(pcp); \
+} while (0)
+
+#define __this_cpu_dec(pcp) \
+do { \
+ __this_cpu_preempt_check("dec"); \
+ raw_cpu_dec(pcp); \
+} while (0)
+
+#define __this_cpu_and(pcp, val) \
+do { \
+ __this_cpu_preempt_check("and"); \
+ raw_cpu_and((pcp), (val)); \
+} while (0)
+
+#define __this_cpu_or(pcp, val) \
+do { \
+ __this_cpu_preempt_check("or"); \
+ raw_cpu_or((pcp), (val)); \
+} while (0)
+
+#define __this_cpu_xor(pcp, val) \
+do { \
+ __this_cpu_preempt_check("xor"); \
+ raw_cpu_xor((pcp), (val)); \
+} while (0)
+
+#define __this_cpu_add_return(pcp, val) \
+ (__this_cpu_preempt_check("add_return"), raw_cpu_add_return((pcp), (val)))
+
+#define __this_cpu_sub_return(pcp, val) \
+ (__this_cpu_preempt_check("sub_return"), raw_cpu_sub_return((pcp), (val)))
+
+#define __this_cpu_inc_return(pcp) \
+ (__this_cpu_preempt_check("inc_return"), raw_cpu_inc_return(pcp))
+
+#define __this_cpu_dec_return(pcp) \
+ (__this_cpu_preempt_check("dec_return"), raw_cpu_dec_return(pcp))
+
+#define __this_cpu_xchg(pcp, val) \
+ (__this_cpu_preempt_check("xchg"), raw_cpu_xchg((pcp), (val)))
+
+#define __this_cpu_cmpxchg(pcp, oval, nval) \
+ (__this_cpu_preempt_check("cmpxchg"), raw_cpu_cmpxchg((pcp), (oval), (nval)))
+
+#define __this_cpu_cmpxchg_double(pcp, oval1, oval2, nval1, nval2) \
+ (__this_cpu_preempt_check("cmpxchg_double"), \
+ raw_cpu_cmpxchg_double((pcp), (oval1), (oval2), (nval1), (nval2)))
+
#endif /* __LINUX_PERCPU_H */
diff --git a/lib/smp_processor_id.c b/lib/smp_processor_id.c
index 04abe53..f474347 100644
--- a/lib/smp_processor_id.c
+++ b/lib/smp_processor_id.c
@@ -7,7 +7,7 @@
#include <linux/kallsyms.h>
#include <linux/sched.h>
-notrace unsigned int debug_smp_processor_id(void)
+notrace static unsigned int debug_preempt_check(char *msg)
{
int this_cpu = raw_smp_processor_id();
@@ -38,9 +38,8 @@ notrace unsigned int debug_smp_processor_id(void)
if (!printk_ratelimit())
goto out_enable;
- printk(KERN_ERR "BUG: using smp_processor_id() in preemptible [%08x] "
- "code: %s/%d\n",
- preempt_count() - 1, current->comm, current->pid);
+ printk(KERN_ERR "BUG: using %s in preemptible [%08x] code: %s/%d\n",
+ msg, preempt_count() - 1, current->comm, current->pid);
print_symbol("caller is %s\n", (long)__builtin_return_address(0));
dump_stack();
@@ -50,5 +49,17 @@ notrace unsigned int debug_smp_processor_id(void)
return this_cpu;
}
+notrace unsigned int debug_smp_processor_id(void)
+{
+ return debug_preempt_check("smp_processor_id()");
+}
EXPORT_SYMBOL(debug_smp_processor_id);
+notrace void __this_cpu_preempt_check(const char *msg)
+{
+ char op[40];
+
+ snprintf(op, sizeof(op), "__this_cpu_%s()", msg);
+ debug_preempt_check(op);
+}
+EXPORT_SYMBOL(__this_cpu_preempt_check);
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-16 18:38 ` Peter Zijlstra
@ 2013-10-17 19:22 ` Christoph Lameter
2013-10-17 21:13 ` Peter Zijlstra
0 siblings, 1 reply; 33+ messages in thread
From: Christoph Lameter @ 2013-10-17 19:22 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Wed, 16 Oct 2013, Peter Zijlstra wrote:
> On Wed, Oct 16, 2013 at 06:25:37PM +0200, Peter Zijlstra wrote:
> > To rename __this_cpu to raw_cpu; it would then need a little manual
> > fixup and generic __this_cpu -> raW_cpu map, which can add the
> > preemption check.
>
> Something like so perhaps... only lightly compile tested
> (x86_64-defconfig).
First portion looks good to me on first glance.
> diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
> index 8729723..95e3532 100644
> --- a/arch/x86/include/asm/preempt.h
> +++ b/arch/x86/include/asm/preempt.h
> @@ -13,12 +13,12 @@ DECLARE_PER_CPU(int, __preempt_count);
> */
> static __always_inline int preempt_count(void)
> {
> - return __this_cpu_read_4(__preempt_count) & ~PREEMPT_NEED_RESCHED;
> + return raw_cpu_read_4(__preempt_count) & ~PREEMPT_NEED_RESCHED;
> }
>
> static __always_inline void preempt_count_set(int pc)
> {
> - __this_cpu_write_4(__preempt_count, pc);
> + raw_cpu_write_4(__preempt_count, pc);
> }
Huh? What happened here? Why do we use the __this_cpu_read_4 here?
This should be just raw_cpu_write()
And more following
> diff --git a/include/asm-generic/percpu.h b/include/asm-generic/percpu.h
> index d17784e..b5aca54 100644
> --- a/include/asm-generic/percpu.h
> +++ b/include/asm-generic/percpu.h
> @@ -56,17 +56,18 @@ extern unsigned long __per_cpu_offset[NR_CPUS];
> #define per_cpu(var, cpu) \
> (*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
>
> -#ifndef __this_cpu_ptr
> -#define __this_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset)
> +#ifndef raw_cpu_ptr
> +#define raw_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset)
> #endif
Got another patch here that gets rid of __this_cpu_ptr and uses
raw_cpu_ptr everywhere instead. Would be good to make this all symmetric.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops
2013-10-17 19:22 ` Christoph Lameter
@ 2013-10-17 21:13 ` Peter Zijlstra
0 siblings, 0 replies; 33+ messages in thread
From: Peter Zijlstra @ 2013-10-17 21:13 UTC (permalink / raw)
To: Christoph Lameter
Cc: Tejun Heo, akpm, rostedt, linux-kernel, Ingo Molnar,
Thomas Gleixner
On Thu, Oct 17, 2013 at 07:22:12PM +0000, Christoph Lameter wrote:
> On Wed, 16 Oct 2013, Peter Zijlstra wrote:
> > diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
> > index 8729723..95e3532 100644
> > --- a/arch/x86/include/asm/preempt.h
> > +++ b/arch/x86/include/asm/preempt.h
> > @@ -13,12 +13,12 @@ DECLARE_PER_CPU(int, __preempt_count);
> > */
> > static __always_inline int preempt_count(void)
> > {
> > - return __this_cpu_read_4(__preempt_count) & ~PREEMPT_NEED_RESCHED;
> > + return raw_cpu_read_4(__preempt_count) & ~PREEMPT_NEED_RESCHED;
> > }
> >
> > static __always_inline void preempt_count_set(int pc)
> > {
> > - __this_cpu_write_4(__preempt_count, pc);
> > + raw_cpu_write_4(__preempt_count, pc);
> > }
>
> Huh? What happened here? Why do we use the __this_cpu_read_4 here?
> This should be just raw_cpu_write()
Header inclusion hell; I could only easily get away with including
asm/percpu.h, so I had to use the _4 stuff.
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2013-10-17 21:13 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-15 17:47 [PATCH 0/6] percpu: Implement Preemption checks for __this_cpu operations V4b Christoph Lameter
2013-10-15 17:47 ` [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update Christoph Lameter
2013-10-15 18:36 ` Eric Dumazet
2013-10-16 6:09 ` Ingo Molnar
2013-10-16 8:35 ` Peter Zijlstra
2013-10-16 9:14 ` Eric Dumazet
2013-10-16 9:26 ` Ingo Molnar
2013-10-16 14:27 ` Christoph Lameter
2013-10-16 14:37 ` Eric Dumazet
2013-10-15 17:47 ` [PATCH 2/6] percpu: Add raw_cpu_ops Christoph Lameter
2013-10-15 17:47 ` [PATCH 3/6] mm: Use raw_cpu ops for determining current NUMA node Christoph Lameter
2013-10-16 8:38 ` Peter Zijlstra
2013-10-16 14:22 ` Christoph Lameter
2013-10-15 17:47 ` [PATCH 4/6] Use raw_cpu_write for initialization of per cpu refcount Christoph Lameter
2013-10-16 8:43 ` Peter Zijlstra
2013-10-15 17:47 ` [PATCH 5/6] net: __this_cpu_inc in route.c Christoph Lameter
2013-10-16 8:46 ` Peter Zijlstra
2013-10-16 9:22 ` Eric Dumazet
2013-10-16 10:25 ` Peter Zijlstra
2013-10-16 15:07 ` Christoph Lameter
2013-10-15 17:47 ` [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops Christoph Lameter
2013-10-16 8:49 ` Peter Zijlstra
2013-10-16 15:09 ` Christoph Lameter
2013-10-16 15:36 ` Peter Zijlstra
2013-10-16 15:55 ` Christoph Lameter
2013-10-16 16:25 ` Peter Zijlstra
2013-10-16 16:52 ` Steven Rostedt
2013-10-16 17:11 ` Peter Zijlstra
2013-10-16 17:39 ` Steven Rostedt
2013-10-16 18:38 ` Peter Zijlstra
2013-10-17 19:22 ` Christoph Lameter
2013-10-17 21:13 ` Peter Zijlstra
[not found] <20131011175518.634285474@linux.com>
2013-10-11 17:54 ` Christoph Lameter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox