Re: [patch 2/2] fix SMP data race in pagetable setup vs walking

[Nick Piggin <npiggin@suse.de>]

On Tue, May 13, 2008 at 05:55:50PM -0700, Linus Torvalds wrote:
> 
> 
> On Wed, 14 May 2008, Nick Piggin wrote:
> > 
> > Uh, I don't follow your logic. The "reference" Linux memory model
> > requires it, so I don't see how you can justify saying it is wrong
> > just because a *specific* architecture doesn't need it.
> 
> You're thinking about it the wrong way.
> 
> NO specific architecture requires it except for alpha, and alpha already 
> has it.
> 
> Nobody else is *ever* likely to want it ever again.

Sure, I understand that. But unless you're hinting about removing
SMP support for those alphas that require it, all generic code
obviously still has to care about it. And not many people would
say they only ever look at x86 code and nothing else. I just think
it is a good exercise to _always_ be thinking about barriers, and
be thinking about them in terms of the Linux memory consistency
standard. Maybe I'm biased because I don't do much arch work...

 
> In other words, it's not a "reference model". It's an "alpha hack". We do 
> not want to copy it in code that doesn't need or want it.
> 
> And that's especially true when it's not needed at all, and adding it just 
> makes a really simple macro much more complex and totally unreadable.
> 
> If it was about adding something to a function that was already a real 
> function, it would be different.
> 
> If you coudl write it as a nice inline function, it would be different.
> 
> But when that alpha hack turns a regular (simple) #define into a thing of 
> horror, the downside is much *much* bigger than any (non-existent) upside.

Oh that's the main thing you're worried about. Then what about the
ACCESS_ONCE that might be required? That would require the same kind
of thing as I did. And actually smp_read_barrier_depends is a pretty
good indicator for (one of the) cases where we need that macro (although
as I said, I prefer gcc to be fixed).

Anyway, what I will do is send a patch which does the core, and the
alpha bits. At least then the actual bugs will be fixed.