Messages in this thread Patch in this message | | | Date | Mon, 8 Apr 2013 07:50:19 -0700 | Subject | Re: [PATCH] [PATCH] Gaurantee spinlocks implicit barrier for !PREEMPT_COUNT | From | Linus Torvalds <> |
| |
On Mon, Apr 8, 2013 at 7:31 AM, Steven Rostedt <srostedt@redhat.com> wrote: > > It requires gcc reordering the code to where a preempt can happen inside > preempt_disable. And also put in a position where the preempt_disable > code it gets added matters. > > Then if gcc does this, we need a page fault to occur with a get_user() > operation, which in practice seldom happens as most get user operations > are done on freshly modified memory. > > And then, it would require the page fault to cause a schedule. This is > the most likely of the things needed to occur, but itself is not a > problem. > > Then, the schedule would have to cause the data that is being protect by > the preempt_disable() to be corrupted. Either by scheduling in another > process that monkeys with the data. Or if it protects per-cpu data, > scheduling to another CPU (for the SMP case only). > > If any of the above does not occur, then you wont see a bug.
Right. The gcc reordering is also hard to actually notice if it does happen, so even testing the fix for this looks nontrivial.
Something like the appended (whitespace-damaged and TOTALLY UNTESTED) might be sufficient, but..
Comments?
Linus
--- diff --git a/include/linux/preempt.h b/include/linux/preempt.h index 5a710b9c578e..465df1c13386 100644 --- a/include/linux/preempt.h +++ b/include/linux/preempt.h @@ -93,14 +93,17 @@ do { \
#else /* !CONFIG_PREEMPT_COUNT */
-#define preempt_disable() do { } while (0) -#define sched_preempt_enable_no_resched() do { } while (0) -#define preempt_enable_no_resched() do { } while (0) -#define preempt_enable() do { } while (0) - -#define preempt_disable_notrace() do { } while (0) -#define preempt_enable_no_resched_notrace() do { } while (0) -#define preempt_enable_notrace() do { } while (0) +/* This is only a barrier to other asms. Notably get_user/put_user */ +#define asm_barrier() asm volatile("") + +#define preempt_disable() asm_barrier() +#define sched_preempt_enable_no_resched() asm_barrier() +#define preempt_enable_no_resched() asm_barrier() +#define preempt_enable() asm_barrier() + +#define preempt_disable_notrace() asm_barrier() +#define preempt_enable_no_resched_notrace() asm_barrier() +#define preempt_enable_notrace() asm_barrier() #endif /* CONFIG_PREEMPT_COUNT */
| |