Re: [Xen-devel] [PATCH 1/2] xen/mce: fix static variables 'found_error' and 'mce_fatal_cpus'

[Jan Beulich]

>>> On 06.04.17 at 10:49,  wrote:
> On 04/06/17 01:54 -0600, Jan Beulich wrote:
>> >>> On 06.04.17 at 06:55,  wrote:
>> > 1. Move them into mcheck_cmn_handler() which is their only user.
>> 
>> This part is uncontroversial.
>> 
>> > 2. Always (re-)initialize them to clear historical information.
>> 
>> But without further explanation I'm not convinced this part is correct.
>> That's a good indication that you should split patches.
>>
>> I assume, btw, that you're aware that these patches won't go
>> in very soon (not until after 4.9 has been branched off), unless
>> they fix an actual bug.
>>
> 
> Please drop this patch as it does not fix any bug. I was blind.
> 'found_error' is already cleared after mcheck_cmn_handler() used it.
> Non-empty 'mce_fatal_errors' results in mc_panic(), so there is no
> need to clear it.
> 
> Sorry for the noise.

Well, reducing the scope of the variables is still a worthwhile thing.

Jan


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

Re: [Xen-devel] [PATCH 1/2] xen/mce: fix static variables 'found_error' and 'mce_fatal_cpus'

[Haozhong Zhang]

On 04/06/17 01:54 -0600, Jan Beulich wrote:
> >>> On 06.04.17 at 06:55,  wrote:
> > 1. Move them into mcheck_cmn_handler() which is their only user.
> 
> This part is uncontroversial.
> 
> > 2. Always (re-)initialize them to clear historical information.
> 
> But without further explanation I'm not convinced this part is correct.
> That's a good indication that you should split patches.
>
> I assume, btw, that you're aware that these patches won't go
> in very soon (not until after 4.9 has been branched off), unless
> they fix an actual bug.
>

Please drop this patch as it does not fix any bug. I was blind.
'found_error' is already cleared after mcheck_cmn_handler() used it.
Non-empty 'mce_fatal_errors' results in mc_panic(), so there is no
need to clear it.

Sorry for the noise.

Haozhong

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

Re: [Xen-devel] [PATCH 1/2] xen/mce: fix static variables 'found_error' and 'mce_fatal_cpus'

[Jan Beulich]

>>> On 06.04.17 at 06:55,  wrote:
> 1. Move them into mcheck_cmn_handler() which is their only user.

This part is uncontroversial.

> 2. Always (re-)initialize them to clear historical information.

But without further explanation I'm not convinced this part is correct.
That's a good indication that you should split patches.

I assume, btw, that you're aware that these patches won't go
in very soon (not until after 4.9 has been branched off), unless
they fix an actual bug.

Jan


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

[Xen-devel] [PATCH 1/2] xen/mce: fix static variables 'found_error' and 'mce_fatal_cpus'

[Haozhong Zhang]

1. Move them into mcheck_cmn_handler() which is their only user.
2. Always (re-)initialize them to clear historical information.

Signed-off-by: Haozhong Zhang 
---
 xen/arch/x86/cpu/mcheck/mce.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 11d0e23..7618120 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -177,6 +177,7 @@ void mce_need_clearbank_register(mce_need_clearbank_t 
cbfunc)
 
 static struct mce_softirq_barrier mce_inside_bar, mce_severity_bar;
 static struct mce_softirq_barrier mce_trap_bar;
+static struct mce_softirq_barrier mce_handler_init_bar;
 
 /*
  * mce_logout_lock should only be used in the trap handler,
@@ -187,8 +188,6 @@ static struct mce_softirq_barrier mce_trap_bar;
 static DEFINE_SPINLOCK(mce_logout_lock);
 
 static atomic_t severity_cpu = ATOMIC_INIT(-1);
-static atomic_t found_error = ATOMIC_INIT(0);
-static cpumask_t mce_fatal_cpus;
 
 const struct mca_error_handler *__read_mostly mce_dhandlers;
 const struct mca_error_handler *__read_mostly mce_uhandlers;
@@ -453,12 +452,19 @@ static int mce_urgent_action(const struct cpu_user_regs 
*regs,
 /* Shared #MC handler. */
 void mcheck_cmn_handler(const struct cpu_user_regs *regs)
 {
+static atomic_t found_error;
+static cpumask_t mce_fatal_cpus;
 struct mca_banks *bankmask = mca_allbanks;
 struct mca_banks *clear_bank = __get_cpu_var(mce_clear_banks);
 uint64_t gstatus;
 mctelem_cookie_t mctc = NULL;
 struct mca_summary bs;
 
+mce_barrier_enter(_handler_init_bar);
+atomic_set(_error, 0);
+cpumask_clear(_fatal_cpus);
+mce_barrier_exit(_handler_init_bar);
+
 mce_spin_lock(_logout_lock);
 
 if (clear_bank != NULL) {
@@ -1767,6 +1773,7 @@ void mce_handler_init(void)
 mce_barrier_init(_inside_bar);
 mce_barrier_init(_severity_bar);
 mce_barrier_init(_trap_bar);
+mce_barrier_init(_handler_init_bar);
 spin_lock_init(_logout_lock);
 open_softirq(MACHINE_CHECK_SOFTIRQ, mce_softirq);
 }
-- 
2.10.1


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