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

[Chao Peng <chao.p.peng@linux.intel.com>]

On Wed, Jan 07, 2015 at 09:43:44PM +0000, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [Xen-devel] [PATCH v2 2/5] tools: add routine to get CMT L3 event mask"):
> > Other culprits are xc_get_max_nodes(), xc_get_max_cpus(), 4 instances in
> > xc_psr.c and most things in xc_offline_page.c which appears to have
> > static structures for domain context.  The "pluggable loader"
> > infrastructure in xc_dom.c also appears to be thread-unsafe.
> >
> > xc_dom_decompress_unsafe.c also uses static data, but "unsafe" in the
> > name might be a sufficient guard?
> 
> I will look at these tomorrow.
> 
> > No aggressively optimising compiler is going to perform partial writes
> > on a naturally aligned integer, so I stand by my comment when applied to
> > the common case.
> 
> You misunderstand.  An aggressively optimising compiler might be able
> to "prove" (perhaps through whole program analysis - we have link-time
> optimisation nowadays) various falsehoods about the way these
> variables are used.
> 
> The resulting generated machine code might be arbitrarily bad, up to
> and including missing important parts of the whole program.
Does this really happen, as for this example?
> 
> I'm not aware of any compilers which currently take "advantge" of
> thread safety "bugs" (really, just spec-violations) but I think this
> is just a matter of time.
I think this can be the issue.  At least we should be very careful.
Or to build a common mechanism in libxc to cache something safely, using
that to replace all the static places current we have.


While I personally agree to make xc re-enterable. The cache
responsibility then can be move to top caller.

For this example, remove static in libxc and the caching for const
hypercall return value is then moved to xl. It can just 'cache' the value
in local variable. While this is not the most performance way as at least
one hypercall is needed for each xl command invocation.

Chao

> 
> Ian.