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

[Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>]

On Wed, 8 Jan 2014 17:44:22 +0000
Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:

> 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

Yes. I can stop the machine via kdb or other debugger, change the value
during debug, and upon resuming it will start printing stuff. Often
this is needed when going thru several iterations of call before problem
is seen. Making it volatile makes sure the compiler loads it every
instance of its use. This is not in main path, only debugger path, so
the overhead should not be an issue.

thanks,
mukesh

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