Re: [Xen-devel] [PATCH v2 3/5] dbg_rw_guest_mem: Conditionally enable debug log output

[Tim Deegan <tim@xxxxxxx>]

At 16:47 +0000 on 08 Jan (1389196026), Ian Campbell wrote:
> On Wed, 2014-01-08 at 09:28 -0500, Don Slutz wrote:
> > > Using volatile is almost always wrong. Why do you think it is needed
> > > here?
> > 
> > This was from Mukesh Rathor:
> > 
> > http://lists.xen.org/archives/html/xen-devel/2014-01/msg00426.html
> > 
> > I saw no reason to make it volatile, but maybe "kdb" needs this? Happy
> > to change any way you want.
> 
> I'm not the maintainer but if I were I'd say drop the volatile and maybe
> mark it __read_mostly and perhaps __used too (since it's static it might
> otherwise get eliminated).
> 
> > > If anything this variable is exactly the opposite, i..e __read_mostly or
> > > even const (given that I can't see anything which writes it I suppose
> > > this is a compile time setting?)
> > 
> > That has been how I have been testing it so far (changing the source
> > to set values).  Mukesh claims to be able to change it at will.  Not
> > sure how const may effect this.

If the idea is to use kdb itself to frob the value, then it does need
something to stop the compiler caching it.  This might even be one of
the few cases where 'volatile' actually DTRT; it would still be more
in keeping with Xen style to use an explicit read op (like
atomic_read()) where the value is consumed.

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel