Received-SPF: pass (google.com: domain of linux-kernel+bounces-95911-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 7 Mar 2024 18:09:01 +0100
From: Borislav Petkov <bp@alien8.de>
To: Sohil Mehta <sohil.mehta@intel.com>
Cc: Tony Luck <tony.luck@intel.com>, "Naik, Avadhut" <avadnaik@amd.com>,
	Yazen Ghannam <yazen.ghannam@amd.com>, x86@kernel.org,
	linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] x86/mce: Dynamically size space for machine check
 records
Message-ID: <20240307170901.GBZen0re6AvpscLaTM@fat_crate.local>
References: <20240307000256.34352-1-tony.luck@intel.com>
 <20240307121634.GAZemwIgbKKJGaUVFg@fat_crate.local>
 <3c2afc2b-85d7-4c2b-8a32-1a6b0f225328@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <3c2afc2b-85d7-4c2b-8a32-1a6b0f225328@intel.com>

On Thu, Mar 07, 2024 at 08:59:53AM -0800, Sohil Mehta wrote:
> I was about the suggest the same thing and maybe slightly more. By
> initializing ret when really needed, I find the code a bit easier to
> follow. No strong preference here.

Except that "really needed" is done this way:

> diff --git a/arch/x86/kernel/cpu/mce/genpool.c
> b/arch/x86/kernel/cpu/mce/genpool.c
> index cadf28662a70..83a01d20bbd9 100644
> --- a/arch/x86/kernel/cpu/mce/genpool.c
> +++ b/arch/x86/kernel/cpu/mce/genpool.c
> @@ -118,22 +118,21 @@ int mce_gen_pool_add(struct mce *mce)
> 
>  static int mce_gen_pool_create(void)
>  {
> -	int mce_numrecords, mce_poolsz, order;
> +	int mce_numrecords, mce_poolsz, order, ret;
>  	struct gen_pool *tmpp;
> -	int ret = -ENOMEM;
>  	void *mce_pool;
> 

	ret = -ENOMEM;
>  	order = order_base_2(sizeof(struct mce_evt_llist));
>  	tmpp = gen_pool_create(order, -1);
>  	if (!tmpp)
> -		return ret;
> +		return -ENOMEM;
> 

	ret = -ENOMEM;
>  	mce_numrecords = max(MCE_MIN_ENTRIES, num_possible_cpus() * MCE_PER_CPU);
>  	mce_poolsz = mce_numrecords * (1 << order);
>  	mce_pool = kmalloc(mce_poolsz, GFP_KERNEL);
>  	if (!mce_pool) {
>  		gen_pool_destroy(tmpp);
> -		return ret;
> +		return -ENOMEM;

before each block, so that it is clear what this particular block is
going to return on error.

But those assignments get redundant so the current way is fine, I'd say.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette