Re: [PATH -mm] Fix a race condtion of oops_in_progress

[Huang Ying]

On Tue, 2008-08-19 at 02:41 -0700, Andrew Morton wrote:
[...]
> > > It seems that asm/atomic.h is used for both atomic_t declaration and
> > > implementation, how about separate them? That it, add a new file
> > > asm/atomic_def.h, put typedef there, and include asm/atomic_def.h in
> > > kernel.h?
> > 
> > yup, that sounds sensible.
> 
> otoh, it means altering every architectures's atomic.h.

Yes. But the advantage is that we can use atomic_t in almost all header
files. I already have a patch for this.

> Finding a different header file for the oops_in_progress declaration
> might be more practical.
> 
> Or we could just do nothing.  How realistic is this race?

The possibility of race is fairly low in real life. Multiple OOPS on
difference CPU occur simultaneously? But its possibility increases
significantly for kernel panic related regression testing. Recently, I
am working on a kernel MCE regression testing suite, where panic may be
triggered on multiple CPU simultaneously as the result of MCE. I can
observe the race condition at quite high possibility.

> umm, how about making it a function?
> 
> static atomic_t oops_in_progress = ATOMIC_INIT(0);
> 
> int oops_is_in_progress(void)
> {
> 	return atomic_read(&oops_in_progress);
> }
> 
> int oops_in_progress_inc(void)
> {
> 	atomic_inc(&oops_in_progress);
> }
> 
> then just open-code the atomic_inc and atomic_dec in
> lib/bust_spinlocks.c and call oops_in_progress_inc() from
> debug_locks_off().

There is an issue of this solution, the oops_in_process is assigned
directly on some architecture. Although we can add a function such as
oops_in_progress_set(int val), it seems like a hack.

Best Regards,
Huang Ying


--
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/