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

[Don Slutz <dslutz@xxxxxxxxxxx>]

On 01/09/14 04:59, Ian Campbell wrote:
> On Wed, 2014-01-08 at 16:38 -0800, Mukesh Rathor wrote:
> > 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.
> So you want to be able to toggle the value in between two immediately
> adjacent debug print calls? While debugging the debugging infrastructure
> itself? (using itself?).
>
> I'm surprised that even works, but if you say so then OK.
>
> Ian.
Based on Mukesh's statement, attached is the rebased version of this patch 
(labeled v3).  I included Mukesh's ack.

    -Don Slutz

Attachment: 0003-dbg_rw_guest_mem-Conditionally-enable-debug-log-outp.patch
Description: Text Data

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