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

Naik, Avadhut
2024-02-12T19:40:26Z

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
>
>
>
>