On 2026/04/20 16:01, Marc Zyngier wrote:
On Mon, 20 Apr 2026 07:21:45 +0100,
Akihiko Odaki <[email protected]> wrote:
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.
And that's exactly the sort of thing I am trying to plan for. *Should*
we introduce a way to remove PMUs from the list, this predicate
becomes unsafe.
Perhaps so. In regards to this series, I'd rather like to keep it out of
scope as the requirement is not new.
So I want at least a comment explaining this to the unsuspecting
reader, as this is rather subtle.
I agree. I had to put some effort to understand the previous
mutex-protected implementation and to design the new RCU-protected one.
I'll add one with the next version.
Regards,
Akihiko Odaki
