Re: [Xen-devel] [PATCH 2/2] xen/mce: fix static variable 'severity_cpu'

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.04.17 at 06:55, <haozhong.zhang@xxxxxxxxx> wrote:
> 1. Distinguish 'severity_cpu' used in mcheck_cmn_handler() and
>    mce_softirq(), which should be different variables. Otherwise, they
>    may interfere with each other if MC# comes during mce_softirq().
> 2. Always (re-)initialize 'severity_cpu' to clear historical information.

I agree with this for the mcheck_cmn_handler() case, but ...

> @@ -1704,11 +1703,16 @@ static int mce_delayed_action(mctelem_cookie_t mctc)
>  /* Softirq Handler for this MCE# processing */
>  static void mce_softirq(void)
>  {
> +    static atomic_t severity_cpu;
>      int cpu = smp_processor_id();
>      unsigned int workcpu;
>  
>      mce_printk(MCE_VERBOSE, "CPU%d enter softirq\n", cpu);
>  
> +    mce_barrier_enter(&mce_softirq_init_bar);
> +    atomic_set(&severity_cpu, -1);
> +    mce_barrier_exit(&mce_softirq_init_bar);

... I don't think this is needed, as right after the following comment
it'll be set unconditionally.

> @@ -1774,6 +1778,7 @@ void mce_handler_init(void)
>      mce_barrier_init(&mce_severity_bar);
>      mce_barrier_init(&mce_trap_bar);
>      mce_barrier_init(&mce_handler_init_bar);
> +    mce_barrier_init(&mce_softirq_init_bar);

Just like the variables you move, all these mce_*_bar ones are
really private to their respective functions. I've taken a not to
put together a patch to move the pre-existing ones, but please
don't introduce any new ones with file scope.

Jan


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