Re: [PATCH v2] x86/mce: Dynamically size space for machine check records

[Borislav Petkov]

On Wed, Mar 06, 2024 at 04:02:56PM -0800, Tony Luck wrote:
> -	ret = gen_pool_add(tmpp, (unsigned long)gen_pool_buf, MCE_POOLSZ, -1);
> +	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);
> +		goto out;
> +	}
> +	ret = gen_pool_add(tmpp, (unsigned long)mce_pool, mce_poolsz, -1);
>  	if (ret) {
>  		gen_pool_destroy(tmpp);
> +		kfree(mce_pool);
>  		goto out;

Might as well get rid of the out label too since you're not doing the
error handling pattern of jumping to err* labels and then unwinding. See
diff below.

Otherwise, patch looks ok to me, if we can test it quickly with all
possible scenarios and Linus does a -rc8, I probably can take it even
now...

Thx.

diff --git a/arch/x86/kernel/cpu/mce/genpool.c b/arch/x86/kernel/cpu/mce/genpool.c
index 42ce3dc97ca8..cadf28662a70 100644
--- a/arch/x86/kernel/cpu/mce/genpool.c
+++ b/arch/x86/kernel/cpu/mce/genpool.c
@@ -126,25 +126,24 @@ static int mce_gen_pool_create(void)
 	order = order_base_2(sizeof(struct mce_evt_llist));
 	tmpp = gen_pool_create(order, -1);
 	if (!tmpp)
-		goto out;
+		return ret;
 
 	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);
-		goto out;
+		return ret;
 	}
 	ret = gen_pool_add(tmpp, (unsigned long)mce_pool, mce_poolsz, -1);
 	if (ret) {
 		gen_pool_destroy(tmpp);
 		kfree(mce_pool);
-		goto out;
+		return ret;
 	}
 
 	mce_evt_pool = tmpp;
 
-out:
 	return ret;
 }
 

-- 
Regards/Gruss,
    Boris.

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