On 2026/04/19 23:34, Marc Zyngier wrote:
On Sat, 18 Apr 2026 09:14:24 +0100,
Akihiko Odaki <[email protected]> wrote:

Convert the list of PMUs to a RCU-protected list that has primitives to
avoid read-side contention.

Signed-off-by: Akihiko Odaki <[email protected]>
---
  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

Reply via email to