linux-kernel - Re: [PATCH v3 06/10] x86/mce: Break up __mcheck_cpu_apply

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241030141635.GB1288714@yaz-khff2.amd.com>
Date: Wed, 30 Oct 2024 10:16:35 -0400
From: Yazen Ghannam <yazen.ghannam@....com>
To: "Zhuo, Qiuxu" <qiuxu.zhuo@...el.com>
Cc: "bp@...en8.de" <bp@...en8.de>, "Luck, Tony" <tony.luck@...el.com>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
	"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 06/10] x86/mce: Break up __mcheck_cpu_apply_quirks()

On Wed, Oct 30, 2024 at 01:39:43AM +0000, Zhuo, Qiuxu wrote:

[...]

Thanks Qiuxu.

> 
> > > +static void apply_quirks_intel(struct cpuinfo_x86 *c) {
> > > +	struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array);
> > > +	struct mca_config *cfg = &mca_cfg;
> > 
> > Is there a benefit to this pointer? We use mca_cfg.FIELD in most other places.
> 
> This could make the diff smaller for easier review, and I also believe that fewer direct
> uses of global variables in functions are better. Additionally, there are multiple uses of
> 'mca_cfg' in the function, the local variable 'cfg' is shorter and more convenient to use.
>

I don't think it would make the diff smaller here since the code is
already being moved.

Though you could say this is a separate logical change compared to just
moving the code as-is.

Also, I don't think the "shorter, more convenient" idea holds. It's not
that much shorter. And there are already cases of using the global
variables "mca_cfg" and "mce_flags".

Why is "...fewer direct uses of global variables in functions..." better?

> [ Certainly, if the global variable 'mca_cfg' is only used once in the function, directly
>   using it might be more convenient. ]
>

There is one such case in your patch.

> Just from my perspective, no strong preference. 😊
> 

Same here. I just figured this suggestion would be another possible
cleanup. :)

Thanks,
Yazen