Re: [PATH -mm -v2] Fix a race condtion of oops_in_progress
From: Chris Snook
Date: Mon Nov 10 2008 - 20:12:20 EST
Huang Ying wrote:
On Mon, 2008-11-10 at 15:35 +0800, KOSAKI Motohiro wrote:
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.
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.
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 = <xxx>". So this patch fixes architectures which use
default bust_spinlocks(), other architectures can be fixed by
corresponding architecture developers.
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.
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?
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/