On 6/25/25 12:09 PM, Boqun Feng wrote:
On Wed, Jun 25, 2025 at 11:52:04AM -0400, Waiman Long wrote:
[...]
+/*
+ * Acquire a hazptr slot and begin the hazard pointer critical section.
+ *
+ * Must be called with preemption disabled, and preemption must remain disabled
+ * until shazptr_clear().
+ */
+static inline struct shazptr_guard shazptr_acquire(void *ptr)
+{
+ struct shazptr_guard guard = {
+ /* Preemption is disabled. */
+ .slot = this_cpu_ptr(&shazptr_slots),
+ .use_wildcard = false,
+ };
+
+ if (likely(!READ_ONCE(*guard.slot))) {
+ WRITE_ONCE(*guard.slot, ptr);
+ } else {
+ guard.use_wildcard = true;
+ WRITE_ONCE(*guard.slot, SHAZPTR_WILDCARD);
+ }
Is it correct to assume that shazptr cannot be used in a mixed context
environment on the same CPU like a task context and an interrupt context
trying to acquire it simultaneously because the current check isn't atomic
with respect to that?
I think the current implementation actually support mixed context usage,
let see (assuming we start in a task context):
if (likely(!READ_ONCE(*guard.slot))) {
if an interrupt happens here, it's fine because the slot is still empty,
as long as the interrupt will eventually clear the slot.
WRITE_ONCE(*guard.slot, ptr);
if an interrupt happens here, it's fine because the interrupt would
notice that the slot is already occupied, hence the interrupt will use a
wildcard, and because it uses a wild, it won't clear the slot after it
returns. However the task context's shazptr_clear() will eventually
clear the slot because its guard's .use_wildcard is false.
} else {
if an interrupt happens here, it's fine because of the same: interrupt
will use wildcard, and it will not clear the slot, and some
shazptr_clear() in the task context will eventually clear it.
guard.use_wildcard = true;
WRITE_ONCE(*guard.slot, SHAZPTR_WILDCARD);
if an interrupt happens here, it's fine because of the same.
}
It's similar to why rcu_read_lock() can be just a non-atomic inc.
You are right.
+
+ smp_mb(); /* Synchronize with smp_mb() at synchronize_shazptr(). */
+
+ return guard;
+}
+
+static inline void shazptr_clear(struct shazptr_guard guard)
+{
+ /* Only clear the slot when the outermost guard is released */
+ if (likely(!guard.use_wildcard))
+ smp_store_release(guard.slot, NULL); /* Pair with ACQUIRE at
synchronize_shazptr() */
+}
Is it better to name it shazptr_release() to be conformant with our current
locking convention?
Maybe, but I will need to think about slot reusing between
shazptr_acquire() and shazptr_release(), in the general hazptr API,
you can hazptr_alloc() a slot, use it and hazptr_clear() and then
use it again, eventually hazptr_free(). I would like to keep both hazptr
APIs consistent as well. Thanks!
Thanks for the explanation. Maybe we can reuse the general hazptr API
names (alloc/clear/free) even though shazptr_free() will be a no-op for
now. Just that the current acquire/clear naming looks odd to me.
Thanks,
Longman
