hazptr: Record per-CPU slot owner after claiming the slot
An interrupt that uses a hazard pointer while the interrupted task is
in the acquire slow path can leave its own context recorded as the
owner of the per-CPU slot the task then publishes into. The next
context switch promotes the task's pointer with the interrupt's
context.
hazptr_get_free_percpu_slot() records the owner when it picks an empty
slot, but only Store B in __hazptr_try_acquire() claims the slot. An
interrupt in between picks the same slot, as the lowest free one:
Task on CPU 0 IRQ on CPU 0
------------- ------------
hazptr_acquire(ctx)
fast path fails
__hazptr_acquire(ctx)
picks item k (empty)
item k->ctx.ctx = ctx
hazptr_acquire(irq_ctx)
claims item k
item k->ctx.ctx = irq_ctx
hazptr_release(irq_ctx)
clears slot k
__hazptr_try_acquire()
Store B into slot k
(item k still names irq_ctx)
sleeps holding the pointer
hazptr_note_context_switch()
promotes item k with irq_ctx
The task gets there with k > 0 when it already holds an outer hazard
pointer in slot 0; no other task can, as its slot is promoted when it
is switched out.
The task gets there with k == 0 when the fast path leaves slot 0
empty: either its ptr_eq() check fails because an updater changed the
pointer, or the pointer is NULL and an updater publishes a non-NULL one
before the slow path loads it.
The promotion moves the task's pointer into irq_ctx's backup slot,
which nothing unchains, so hazptr_synchronize() on that pointer never
returns. The task's ctx->slot still names slot k, so its
hazptr_release() clears whatever another reader on that CPU has since
published there.
If the interrupt detaches instead, as hazptr_torture_acquire() does,
the promotion chains irq_ctx's backup slot a second time while it is
still linked. The overflow list then loops on that node, or follows
LIST_POISON1 once hazptr_unchain_backup_slot() removes it.
Record the owner after __hazptr_try_acquire() succeeds, as the fast
path in hazptr_acquire() does. This should be safe because an
interrupt that used the slot has released or detached it by the time
the task resumes, and only hazptr_note_context_switch() reads the
owner, which cannot run before the task enables preemption.
Fixes: 50b568234163 ("hazptr: Implement Hazard Pointers")
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Acked-by: Bradley Morgan <brads@mainlining.org>
Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
diff --git a/kernel/hazptr.c b/kernel/hazptr.c
index 3ca73b5..b15b84e 100644
--- a/kernel/hazptr.c
+++ b/kernel/hazptr.c
@@ -54,19 +54,16 @@ unsigned int flip_list_phase(unsigned int phase)
}
static
-struct hazptr_slot *hazptr_get_free_percpu_slot(struct hazptr_ctx *ctx)
+struct hazptr_slot_item *hazptr_get_free_percpu_slot_item(void)
{
struct hazptr_percpu_slots *percpu_slots = this_cpu_ptr(&hazptr_percpu_slots);
unsigned int idx;
for (idx = 0; idx < NR_HAZPTR_PERCPU_SLOTS; idx++) {
struct hazptr_slot_item *item = &percpu_slots->items[idx];
- struct hazptr_slot *slot = &item->slot;
- if (!slot->addr) {
- item->ctx.ctx = ctx;
- return slot;
- }
+ if (!item->slot.addr)
+ return item;
}
/* All slots are in use. */
return NULL;
@@ -78,6 +75,7 @@ struct hazptr_slot *hazptr_get_free_percpu_slot(struct hazptr_ctx *ctx)
*/
void *__hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p)
{
+ struct hazptr_slot_item *item;
struct hazptr_slot *slot;
void *addr;
@@ -85,10 +83,17 @@ void *__hazptr_acquire(struct hazptr_ctx *ctx, void * const *addr_p)
* In case we are called due to nested use of hazard pointers,
* try a slot protection with per-CPU slots.
*/
- slot = hazptr_get_free_percpu_slot(ctx);
- if (likely(slot)) {
+ item = hazptr_get_free_percpu_slot_item();
+ if (likely(item)) {
+ slot = &item->slot;
addr = __hazptr_try_acquire(ctx, addr_p, slot);
if (addr) {
+ /*
+ * Record the owner only once Store B has claimed the
+ * slot, as hazptr_acquire() does: an interrupt may
+ * have used this slot since it was chosen.
+ */
+ item->ctx.ctx = ctx;
ctx->slot = slot;
return addr;
}