| From 85abd778a1428d0e2c2981bfb695d70c60105537 Mon Sep 17 00:00:00 2001 |
| From: Milton Miller <miltonm@bga.com> |
| Date: Thu, 12 May 2011 04:13:54 -0500 |
| Subject: [PATCH] seqlock: Don't smp_rmb in seqlock reader spin loop |
| |
| commit 5db1256a5131d3b133946fa02ac9770a784e6eb2 upstream. |
| |
| Move the smp_rmb after cpu_relax loop in read_seqlock and add |
| ACCESS_ONCE to make sure the test and return are consistent. |
| |
| A multi-threaded core in the lab didn't like the update |
| from 2.6.35 to 2.6.36, to the point it would hang during |
| boot when multiple threads were active. Bisection showed |
| af5ab277ded04bd9bc6b048c5a2f0e7d70ef0867 (clockevents: |
| Remove the per cpu tick skew) as the culprit and it is |
| supported with stack traces showing xtime_lock waits including |
| tick_do_update_jiffies64 and/or update_vsyscall. |
| |
| Experimentation showed the combination of cpu_relax and smp_rmb |
| was significantly slowing the progress of other threads sharing |
| the core, and this patch is effective in avoiding the hang. |
| |
| A theory is the rmb is affecting the whole core while the |
| cpu_relax is causing a resource rebalance flush, together they |
| cause an interfernce cadance that is unbroken when the seqlock |
| reader has interrupts disabled. |
| |
| At first I was confused why the refactor in |
| 3c22cd5709e8143444a6d08682a87f4c57902df3 (kernel: optimise |
| seqlock) didn't affect this patch application, but after some |
| study that affected seqcount not seqlock. The new seqcount was |
| not factored back into the seqlock. I defer that the future. |
| |
| While the removal of the timer interrupt offset created |
| contention for the xtime lock while a cpu does the |
| additonal work to update the system clock, the seqlock |
| implementation with the tight rmb spin loop goes back much |
| further, and is just waiting for the right trigger. |
| |
| Cc: <stable@vger.kernel.org> |
| Signed-off-by: Milton Miller <miltonm@bga.com> |
| Cc: <linuxppc-dev@lists.ozlabs.org> |
| Cc: Linus Torvalds <torvalds@linux-foundation.org> |
| Cc: Andi Kleen <andi@firstfloor.org> |
| Cc: Nick Piggin <npiggin@kernel.dk> |
| Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> |
| Cc: Anton Blanchard <anton@samba.org> |
| Cc: Paul McKenney <paulmck@linux.vnet.ibm.com> |
| Acked-by: Eric Dumazet <eric.dumazet@gmail.com> |
| Link: http://lkml.kernel.org/r/%3Cseqlock-rmb%40mdm.bga.com%3E |
| Signed-off-by: Thomas Gleixner <tglx@linutronix.de> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| --- |
| include/linux/seqlock.h | 4 ++-- |
| 1 file changed, 2 insertions(+), 2 deletions(-) |
| |
| diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h |
| index 632205c..4c3257d 100644 |
| --- a/include/linux/seqlock.h |
| +++ b/include/linux/seqlock.h |
| @@ -88,12 +88,12 @@ static __always_inline unsigned read_seqbegin(const seqlock_t *sl) |
| unsigned ret; |
| |
| repeat: |
| - ret = sl->sequence; |
| - smp_rmb(); |
| + ret = ACCESS_ONCE(sl->sequence); |
| if (unlikely(ret & 1)) { |
| cpu_relax(); |
| goto repeat; |
| } |
| + smp_rmb(); |
| |
| return ret; |
| } |
| -- |
| 1.7.9.3 |
| |