Re: [PATCH] sched.c : correct comment for this_rq_lock() routine

[Nick Piggin]

Robert P. J. Day wrote:
> On Wed, 1 Nov 2006, Nick Piggin wrote:
>
>
> > Robert P. J. Day wrote:
> >
> >
> > > example, i was just poking around the source for the various
> > > "atomic.h" files and noticed a couple possible cleanups:
> > >
> > > 1) make sure *everyone* uses "volatile" in the typedef struct (which
> > > 	i actually submitted recently)
> >
> > I don't see why. There is nothing in atomic (eg. atomic_read) that
> > says there must be a compiler barrier around the operation.
> >
> > Have you checked that the architecture implementation actually needs
> > the volatile where you've added it?
>
>
> as just one example, you can read in include/asm-alpha/atomic.h:
>
> /*
>  * Counter is volatile to make sure gcc doesn't try to be clever
>  * and move things around on us. We need to use _exactly_ the address
>  * the user gave us, not some alias that contains the same information.
>  */

asm-alpha has volatile.

> now it may be that *some* architectures don't specifically require a
> volatile counter but, AFAIK, it doesn't actually hurt if it isn't
> necessary.  OTOH, if it isn't necessary *at all* for *any*
> architecture, then that storage class should be *removed* in its
> entirety.

Then given that architecture specific code is private to that architecture,
the logical conclusion is that if it isn't required by *an* architectures,
then it should be removed as well.

IOW, you can't assume that because one arch does something one way, that
another has the same restrictions.

> in any event, all this is is another example of what appears to be
> niggling and unnecessary differences between arch-specific header
> files that could easily be turned into a single, standard definition
> that would work for everyone with very little effort (and perhaps some
> day be included from a single generic header file to avoid all that
> duplication in the first place).

So if you want to do that, then I'd prefer that you instead go through all
arch headers and figure out where the volatile is needed, and why, and
document it.

For example FRV, at a glance, doesn't need volatile AFAIKS. Probably neither
do a lot of architectures, given that atomic_read/set/add/etc. need not
imply compiler or memory barriers (I think).

--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 
-
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/