Re: [RFC PATCH 1/4] hazptr: Add initial implementation of hazard pointers

[Jonas Oberhauser <jonas.oberhauser@huaweicloud.com>]

Am 9/17/2024 um 4:33 PM schrieb Boqun Feng:
> +static inline void *__hazptr_tryprotect(hazptr_t *hzp,
> +					void *const *p,
> +					unsigned long head_offset)
> +{
> +	void *ptr;
> +	struct callback_head *head;
> +
> +	ptr = READ_ONCE(*p);
> +
> +	if (ptr == NULL)
> +		return NULL;
> +
> +	head = (struct callback_head *)(ptr + head_offset);
> +
> +	WRITE_ONCE(*hzp, head);
> +	smp_mb();
> +
> +	ptr = READ_ONCE(*p); // read again
> +
> +	if (ptr + head_offset != head) { // pointer changed
> +		WRITE_ONCE(*hzp, NULL);  // reset hazard pointer
> +		return NULL;
> +	} else
> +		return ptr;
> +}

There is a subtle potential for ABA issues here.

If the compiler replaces 'return ptr;' with 'return head - 
head_offset;', then you do not have an address dependency from the 
second read.

In this case, in ABA, the first read can read from a stale store, then 
the second read reads the same value from a newer store but only 
establishes control-dependency based synchronization with that store; 
any reads from *ptr could be speculatively executed before doing the 
second ptr = READ_ONCE(*p).

Therefore you could read the object state before it is properly 
reinitialized by the second store.

I'm not sure what the most efficient fix is or if you just want to 
gamble that "the compiler will never do that".
I guess either READ_ONCE(ptr) or a compiler barrier before return ptr 
might do it?

Have fun,
    jonas