Re: [PATCH v2 08/10] x86/mce: Remove the redundant zeroing assignments

[Sohil Mehta]

On 10/16/2024 5:30 AM, Qiuxu Zhuo wrote:
> As the entire mce structure is initialized to zero using memset(0)
> within mce_gather_info(), remove the redundant zeroing assignments to
> mce->misc and mce->addr.
> 

...

> 
> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
> index e718b9bbe8e5..844a6f8d6f39 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -706,8 +706,6 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
>  		if (!mce_banks[i].ctl || !test_bit(i, *b))
>  			continue;
>  
> -		m.misc = 0;
> -		m.addr = 0;
>  		m.bank = i;
>  

This makes sense since mce_gather_info() happens in the same function.

>  		barrier();
> @@ -1284,8 +1282,6 @@ __mc_scan_banks(struct mce *m, struct pt_regs *regs, struct mce *final,
>  		if (!mce_banks[i].ctl)
>  			continue;
>  
> -		m->misc = 0;
> -		m->addr = 0;
>  		m->bank = i;
>  

However, in this case, I am not fully convinced if the misc and addr
would already be 0 when we reach here.

There are potentially a lot of things that happen in do_machine_check()
between mce_gather_info() and __mc_scan_banks(). Especially,
mce_no_way_out() which could theoretically call mce_read_aux() in some
cases.

Maybe it doesn't matter, misc and addr would be overwritten anyway. But
I feel some more details in the commit message would be useful. It
doesn't seem as simple as the brief description makes it sound (at least
to me).


>  		m->status = mce_rdmsrl(mca_msr_reg(i, MCA_STATUS));