Fix a race condition accessing oops_in_progress.  Which may be changed on
multiple CPU simultaneously, but it is changed via non-atomic operation
++/--.  This patch changes the definition of oops_in_process from int to
atomic_t, and accessing method to atomic operations.


ChangeLog

v2:

- Includes fixes from Andrew Moton.

- Re-based on Matthew Wilcox's new atomic_t patch.


Signed-off-by: Huang Ying <ying.huang@intel.com>

---

 arch/blackfin/kernel/traps.c            |   16 ++++++++--------
 arch/cris/arch-v32/kernel/time.c        |    4 ++--
 arch/cris/kernel/traps.c                |    6 +++---
 arch/cris/mm/fault.c                    |    6 +++---
 arch/ia64/kernel/mca.c                  |    6 +++---
 arch/mn10300/mm/fault.c                 |    4 ++--
 arch/parisc/kernel/traps.c              |    4 ++--
 arch/s390/kernel/setup.c                |    6 +++---
 arch/s390/mm/fault.c                    |    4 ++--
 drivers/char/vt.c                       |    2 +-
 drivers/mtd/mtdoops.c                   |    2 +-
 drivers/parisc/led.c                    |    2 +-
 drivers/serial/8250.c                   |    2 +-
 drivers/serial/cpm_uart/cpm_uart_core.c |    2 +-
 drivers/serial/sunhv.c                  |    4 ++--
 drivers/serial/sunsab.c                 |    2 +-
 drivers/serial/sunsu.c                  |    2 +-
 drivers/serial/sunzilog.c               |    2 +-
 drivers/serial/uartlite.c               |    4 ++--
 drivers/video/aty/radeonfb.h            |    2 +-
 include/linux/console.h                 |    3 ++-
 include/linux/debug_locks.h             |    2 +-
 include/linux/kernel.h                  |    3 ++-
 kernel/printk.c                         |    6 +++---
 kernel/sched.c                          |    3 ++-
 lib/bust_spinlocks.c                    |    4 ++--
 lib/debug_locks.c                       |    2 +-
 27 files changed, 54 insertions(+), 51 deletions(-)

--- a/arch/blackfin/kernel/traps.c
+++ b/arch/blackfin/kernel/traps.c
@@ ...
[ message continues ]

hm, there are a couple of places now that do atomic_set(,1) - they 
should be atomic_inc(), correct?

	Ingo
--

[ message continues ]

I just translate "oops_in_progress =3D 1" to
"atomic_set(&oops_in_progress, 1)". I think this is the safest method to
do the translation.

Best Regards,
Huang Ying=20

[ message continues ]

You also need barriers.  I believe rmb() before atomic_read() and wmb() after 
atomic_set() should suffice.

-- Chris
--

[ message continues ]

Hi, Chris,


I don't think that is necessary. I haven't found there is particular
consistent requirement about oops_in_progress.

Best Regards,
Huang Ying

[ message continues ]

atomic_read() and atomic_set() don't inherently cause changes to be visible on 
other CPUs any faster than ++ and -- operators.  Sometimes it happens to work 
out that way as a result of how the compiler and the CPU order operations, but 
there's no semantic guarantee, and it could even take arbitrarily long under 
some circumstances.  If you want to use atomic ops to close the race, you need 
to use barriers.

-- Chris
--

[ message continues ]

As far as I know, barriers don't cause changes to be visible on other
CPUs faster too. It just guarantees corresponding operations after will
not get executed until that before have finished. And, I don't think we
need make changes to be visible on other CPUs faster.

Best Regards,
Huang Ying

[ message continues ]

You're correct that barrier() has no impact on other CPUs.  wmb() and rmb() do. 
  If we don't need to make changes visible any faster, what's the point in using 
atomic_set()?  It's not any less racy.  atomic_inc() and atomic_dec() would be 
less racy, but you're not using those.

-- Chris
--

[ message continues ]

In default bust_spinlocks() implementation in lib/bust_spinlocks.c,
atomic_inc() and atomic_dec_and_test() is used. Which is used by x86
too. In some other architecture, atomic_set() is used to replace
"oops_in_progress =3D <xxx>". So this patch fixes architectures which use
default bust_spinlocks(), other architectures can be fixed by
corresponding architecture developers.

Best Regards,
Huang Ying

[ message continues ]

I think Chris is right.
So, I reccomend to read Documentation/memory-barriers.txt

Almost architecture gurantee atomic_inc cause barrier implicitly.
but not _all_ architecture.



--

[ message continues ]

The rmb() before atomic_read() is even more critical, since that's a 
non-barrier operation on nearly all platforms.

-- Chris
--

[ message continues ]

Yes. atomic_inc() doesn't imply barrier on all architecture. But we
should not add barriers before all atomic_inc(), just ones needed. Can
you figure out which ones in the patch should has barrier added?

Best Regards,
Huang Ying

[ message continues ]

You need barriers *after* writes, and *before* reads.  Adding barriers to the 
oops path should be extremely cheap for performance, unless oopsing is a common 
occurrence, in which case we have bigger problems.

-- Chris
--

[ message continues ]

I just suspect why we need these barriers. Do we have some memory must
to be written after oops_in_progress? Or some memory must to be read
before oops_in_progress?

Best Regards,
Huang Ying
[ message continues ]