Re: [PATCH] x86/mcheck: allow varying bank counts per CPU

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.09.2025 19:02, Jason Andryuk wrote:
> 
> 
> On 2025-09-05 12:52, Soham Dandapat wrote:
>> In mca_cap_init function,the mcabanks_alloc allocates and
>> initializes an mca_banks structure for managing MCA banks,
>> setting up a bank map and storing the specified or default number
>> of banks.
>>
>> After this we will call mcabanks_set(i, mca_allbanks);
>> The mcabanks_set function sets a specific bit in the bank_map of
>> an mca_banks structure, provided the structure, its bank_map, and
>> the bit index are valid.
>>
>> At the end, we will call
>> mcabanks_free(xchg(&mca_allbanks, all));
>> This function is thread safe and does below:
>>     1. Atomically exchanges the value of "mca_allbanks" with "all"
>>     2. Returns the old value that was previously in "mca_allbanks"
>> So, when we will call mcabanks_free , that will free the memory.
>>
>> The problem is that mcabanks_set(i, mca_allbanks) function is updating
>> mca_allbanks which will be freed via mcabanks_free later. This means
>> new mca_allbanks instance("all") will never get chance to update
>> it's bank_map.
>>
>> Due to this when we will collect log from mcheck_mca_logout function ,
>> the condition "if ( !mcabanks_test(i, bankmask) )" will always fails
>> and MCA logs will not be collected for any bank.
>>
>> The fix is to solve this problem.
>>
>> Fixes: 560cf418c845 ("x86/mcheck: allow varying bank counts per CPU")
>> Signed-off-by: Soham Dandapat <soham.dandapat@xxxxxxx>
> 
> Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> 
> Maybe the patch subject should be "x86/mcheck: Fix mca bank 
> initialization" to differentiate from the Fixes commit?

That's still more generic than wanted. How about "x86/mcheck: fix
mca_allbanks updating"? With a more concise title (which can be
adjusted while committing, so long as there's agreement):
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan