On Thu, May 18, 2023 at 11:18:25AM +0530, Bharath Rupireddy wrote:
> I think what I have so far seems more verbose explaining what a
> barrier does and all that. I honestly think we don't need to be that
> verbose, thanks to README.barrier.
Agreed. This file is a mine of information.
> I simplified those 2 comments as the following:
>
> * NB: pg_atomic_exchange_u64, having full barrier semantics will ensure
> * the variable is updated before releasing the lock.
>
> * NB: pg_atomic_exchange_u64, having full barrier semantics will ensure
> * the variable is updated before waking up waiters.
>
> Please find the attached v7 patch.
Nit. These sentences seem to be worded a bit weirdly to me. How
about:
"pg_atomic_exchange_u64 has full barrier semantics, ensuring that the
variable is updated before (releasing the lock|waking up waiters)."
+ * Be careful that LWLockConflictsWithVar() does not include a memory barrier,
+ * hence the caller of this function may want to rely on an explicit barrier or
+ * a spinlock to avoid memory ordering issues.
Thanks, this addition looks OK to me.
--
Michael