Re: [PATCH v7 2/4] KVM: arm64: PMU: Protect the list of PMUs with RCU

Akihiko Odaki
2026-04-20T06:22:30Z

On 2026/04/19 23:34, Marc Zyngier wrote:
> On Sat, 18 Apr 2026 09:14:24 +0100,
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>>
>> Convert the list of PMUs to a RCU-protected list that has primitives to
>> avoid read-side contention.
>>
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> ---
>> arch/arm64/kvm/pmu-emul.c | 14 ++++++--------
>> 1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
>> index 59ec96e09321..ef5140bbfe28 100644
>> --- a/arch/arm64/kvm/pmu-emul.c
>> +++ b/arch/arm64/kvm/pmu-emul.c
>> @@ -7,9 +7,9 @@
>> #include <linux/cpu.h>
>> #include <linux/kvm.h>
>> #include <linux/kvm_host.h>
>> -#include <linux/list.h>
>> #include <linux/perf_event.h>
>> #include <linux/perf/arm_pmu.h>
>> +#include <linux/rculist.h>
>> #include <linux/uaccess.h>
>> #include <asm/kvm_emulate.h>
>> #include <kvm/arm_pmu.h>
>> @@ -26,7 +26,6 @@ static bool kvm_pmu_counter_is_enabled(struct kvm_pmc *pmc);
>>
>> bool kvm_supports_guest_pmuv3(void)
>> {
>> - guard(mutex)(&arm_pmus_lock);
>> return !list_empty(&arm_pmus);
>
> Please read include/linux/rculist.h and the discussion about the
> interaction of list_empty() with RCU-protected lists. How about using
> list_first_or_null_rcu() for peace of mind?

list_first_or_null_rcu() is useful to replace a sequence of list_empty()
and list_first_entry() that is protected by a lock, but this function
instead requires the invariant that nobody deletes an element from the
list, and list_first_or_null_rcu() does not allow removing the requirement.

The header file says:
> Where are list_empty_rcu() and list_first_entry_rcu()?
>
> They do not exist because they would lead to subtle race conditions:
>
> if (!list_empty_rcu(mylist)) {
> struct foo *bar = list_first_entry_rcu(mylist, struct foo,
> list_member);
> do_something(bar);
> }
>
> The list might be non-empty when list_empty_rcu() checks it, but it
> might have become empty by the time that list_first_entry_rcu()
> rereads the ->next pointer, which would result in a SEGV.
>
> When not using RCU, it is OK for list_first_entry() to re-read that
> pointer because both functions should be protected by some lock that
> blocks writers.
>
> When using RCU, list_empty() uses READ_ONCE() to fetch the
> RCU-protected ->next pointer and then compares it to the address of
> the list head. However, it neither dereferences this pointer nor
> provides this pointer to its caller. Thus, READ_ONCE() suffices
> (that is, rcu_dereference() is not needed), which means that
> list_empty() can be used anywhere you would want to use
> list_empty_rcu(). Just don't expect anything useful to happen if you
> do a subsequent lockless call to list_first_entry_rcu()!!!
>
> See list_first_or_null_rcu for an alternative.

However, kvm_supports_guest_pmuv3() locked a mutex when calling
list_empty() and unlocked it immediately after that, instead of
re-reading list_first_entry(). This construct inherently had a race
condition with code that deletes an element; when the caller of
kvm_supports_guest_pmuv3() decides to enable guest PMUv3, the host PMU
may have been gone. But it was still safe because no one deletes an element.

The same logic also applies when using RCU. As the comment says, we can
use list_empty() instead of the hypothetical list_empty_rcu() macro
because we don't expect it to magically enable something like
list_first_entry_rcu(). This function instead keep relying on the fact
that no one deletes an element of the list.

Regards,
Akihiko Odaki