Hi, On 2022-11-15 23:14:42 +0000, Simon Riggs wrote: > On Tue, 15 Nov 2022 at 23:06, Tom Lane <t...@sss.pgh.pa.us> wrote: > > > > BTW, while nosing around this code I came across this statement > > (procarray.c, about line 4550 in HEAD): > > > > * ... To add XIDs to the array, we just insert > > * them into slots to the right of the head pointer and then advance the > > head > > * pointer. This wouldn't require any lock at all, except that on machines > > * with weak memory ordering we need to be careful that other processors > > * see the array element changes before they see the head pointer change. > > * We handle this by using a spinlock to protect reads and writes of the > > * head/tail pointers. (We could dispense with the spinlock if we were to > > * create suitable memory access barrier primitives and use those instead.) > > * The spinlock must be taken to read or write the head/tail pointers unless > > * the caller holds ProcArrayLock exclusively. > > > > Nowadays we've *got* those primitives. Can we get rid of > > known_assigned_xids_lck, and if so would it make a meaningful > > difference in this scenario?
Forgot to reply to this part: I'm confused by the explanation of the semantics of the spinlock: "The spinlock must be taken to read or write the head/tail pointers unless the caller holds ProcArrayLock exclusively." makes it sound like it'd be valid to modify the KnownAssignedXids without holding ProcArrayLock exclusively. Doesn't that contradict only needing the spinlock because of memory ordering? And when would it be valid to do any modifications of KnownAssignedXids without holding ProcArrayLock exclusively? Concurrent readers of KnownAssignedXids will operate on a snapshot of head/tail (the spinlock is only ever held to query them). Afaict any such modification would be racy, because subsequent modifications of KnownAssignedXids could overwrite contents of KnownAssignedXids that another backend is in the process of reading, based on the stale snapshot of head/tail. To me it sounds like known_assigned_xids_lck is pointless and the talk about memory barriers a red herring, since all modifications have to happen with ProcArrayLock held exlusively and all reads with ProcArrayLock held in share mode. It can't be legal to modify head/tail or the contents of the array outside of that. And lwlocks provide sufficient barrier semantics. > I think you could do that *as well*, since it does act as an overhead > but that is not related to the main issues I think it might be a bigger effect than one might immediately think. Because the spinlock will typically be on the same cacheline as head/tail, and because every spinlock acquisition requires the cacheline to be modified (and thus owned mexlusively) by the current core, uses of head/tail will very commonly be cache misses even in workloads without a lot of KAX activity. Greetings, Andres Freund