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

From: Sohil Mehta
Date: Fri Oct 18 2024 - 16:26:00 EST


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));