[PATCH 2/2] x86/MCE: Add command line option to extend MCE Records pool

[Naik, Avadhut]

Hi,

On 2/12/2024 12:58, Luck, Tony wrote:
>> I need to look more at the genpool code, but I thought I'd ask too.
> 
> Yazen,
> 
> gen_pool_add_owner() is the code that adds an extra chunk to an existing genpool.
> 
> This bit doesn't look obviously safe against a #MC at the wrong moment in the middle of
> the list_add_rcu()
> 
>         spin_lock(&pool->lock);
>         list_add_rcu(&chunk->next_chunk, &pool->chunks);
>         spin_unlock(&pool->lock);
> 

Even I am somewhat confused by this.

The spinlock is mostly held to prevent other primitives
from modifying chunks within the genpool.

In #MC context however, we are not modifying the existing
chunks, per se.

While in the MC context, records will be added to the genpool
through gen_pool_alloc() which eventually drops down into
gen_pool_alloc_algo_owner().

gen_pool_alloc_algo_owner() iterates over the existing chunks
within the genpool through list_for_each_entry_rcu(), within
an RCU read-side critical section (rcu_read_lock()).

Now, the below description of list_for_each_entry_rcu():

 * list_for_each_entry_rcu  -   iterate over rcu list of given type
 * @pos:    the type * to use as a loop cursor.
 * @head:   the head for your list.
 * @member: the name of the list_head within the struct.
 * @cond:   optional lockdep expression if called from non-RCU protection.
 *
 * This list-traversal primitive may safely run concurrently with
 * the _rcu list-mutation primitives such as list_add_rcu()
 * as long as the traversal is guarded by rcu_read_lock().


Makes me wonder if the genpool can be extended and traversed
concurrently.

OFC, not sure if gen_pool_alloc_algo_owner() being in #MC context
makes a difference here or if I am missing something.

-- 
Thanks,
Avadhut Naik
> -Tony
> 
> 
> 
>