Re: [Xen-devel] [PATCH v2 2/5] tools: add routine to get CMT L3 event mask

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Jan 07, 2015 at 04:37:40PM +0000, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [Xen-devel] [PATCH v2 2/5] tools: add routine to 
> get CMT L3 event mask"):
> > On 07/01/15 11:12, Chao Peng wrote:
> > > +int xc_psr_cmt_get_l3_event_mask(xc_interface *xch, uint32_t *event_mask)
> > > +{
> > > +    static int val = 0;
> > 
> > This should be uint32_t rather than int.
> > 
> > I am somewhat concerned about multithreaded use of libxc, but this is
> > not the first issue in libxc, and probably shouldn't be held against
> > this patch.
> 
> On the contrary, this is quite bad.  libxc should be properly
> reentrant and I think it generally is.  If you are aware of other
> global variables of this kind please do ...
> 

xc_misc.c has two similar variables.

> ... I have just seen that the existing version of xc_psr.c has this
> problem already.
> 
> IMO this is a serious bug.  Why was it made static before ?
> 
> >  As the result of the hypercall is going to be the same, the
> > worse that a race could achieve is a wasted hypercall.
> 
> This kind of analysis is unfounded in the presence of modern compilers
> with aggressive optimisations.  At the very least, if you're going to
> do some caching like this, it needs a lock around it.
> 
> Ian.

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