Re: [RFC PATCH v2 3/4] hp: Implement Hazard Pointers

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2024-10-05 18:07, Peter Zijlstra wrote:
> On Sat, Oct 05, 2024 at 06:04:44PM +0200, Peter Zijlstra wrote:
>> On Fri, Oct 04, 2024 at 02:27:33PM -0400, Mathieu Desnoyers wrote:
> 
>>> +void hp_scan(struct hp_slot __percpu *percpu_slots, void *addr,
>>> +	     void (*retire_cb)(int cpu, struct hp_slot *slot, void *addr))
>>> +{
>>> +	int cpu;
>>> +
>>> +	/*
>>> +	 * Store A precedes hp_scan(): it unpublishes addr (sets it to
>>> +	 * NULL or to a different value), and thus hides it from hazard
>>> +	 * pointer readers.
>>> +	 */
> 
> This should probably assert we're in a preemptible context. Otherwise
> people will start using this in non-preemptible context and then we get
> to unfuck things later.

Something like this ?

+       /* Should only be called from preemptible context. */
+       WARN_ON_ONCE(in_atomic());

> 
>>> +
>>> +	if (!addr)
>>> +		return;
>>> +	/* Memory ordering: Store A before Load B. */
>>> +	smp_mb();
>>> +	/* Scan all CPUs slots. */
>>> +	for_each_possible_cpu(cpu) {
>>> +		struct hp_slot *slot = per_cpu_ptr(percpu_slots, cpu);
>>> +
>>> +		if (retire_cb && smp_load_acquire(&slot->addr) == addr)	/* Load B */
>>> +			retire_cb(cpu, slot, addr);
>>
>> Is retirce_cb allowed to cmpxchg the thing?

Renaming retire_cb to "on_match_cb". Whatever the callback does needs to
be done with knowledge of the slot user (e.g. IPI).


>>
>>> +		/* Busy-wait if node is found. */
>>> +		while ((smp_load_acquire(&slot->addr)) == addr)	/* Load B */
>>> +			cpu_relax();
>>
>> This really should be using smp_cond_load_acquire()

Done,

Thanks,

Mathieu

>>
>>> +	}
>>> +}

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com