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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-01-08 at 18:04 +0100, Tim Deegan wrote:
> 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.

Is there any need to be asynchronously frobbing this value in the middle
of a function within this file and expecting it to be reliable? I'd have
thought that changing the value and having it take affect on the next
debug event/hypercall/whatever would be what was wanted.

Ian.


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