Re: [HACKERS] Another bug introduced by fastpath patch

2013-11-28 Thread Tom Lane
Andres Freund writes: > On 2013-11-28 10:31:21 -0500, Tom Lane wrote: >> The only remaining risk is that, if pointer >> fetch/store isn't atomic, we might fetch a half-updated pointer; which >> will be non-null, but not something we can use to reach the list. Since >> we do purport to support suc

Re: [HACKERS] Another bug introduced by fastpath patch

2013-11-28 Thread Andres Freund
On 2013-11-28 10:31:21 -0500, Tom Lane wrote: > The only remaining risk is that, if pointer > fetch/store isn't atomic, we might fetch a half-updated pointer; which > will be non-null, but not something we can use to reach the list. Since > we do purport to support such architectures, we'd better

Re: [HACKERS] Another bug introduced by fastpath patch

2013-11-28 Thread Tom Lane
Robert Haas writes: > In terms of making this more robust, one idea - along the lines you > mentioned in your original post - is to have a separate code path for > the case where we're releasing *all* locks. Yeah, I thought seriously about that, but concluded that it was too debatable for a back-

Re: [HACKERS] Another bug introduced by fastpath patch

2013-11-28 Thread Robert Haas
On Wed, Nov 27, 2013 at 8:21 PM, Tom Lane wrote: > I wrote: >> We could still do this if we were willing to actually reject requests >> for session level locks on fast-path-eligible lock types. At the moment >> that costs us nothing really. If anyone ever did have a use case, we >> could conside

Re: [HACKERS] Another bug introduced by fastpath patch

2013-11-27 Thread Tom Lane
I wrote: > We could still do this if we were willing to actually reject requests > for session level locks on fast-path-eligible lock types. At the moment > that costs us nothing really. If anyone ever did have a use case, we > could consider adding the extra logic to support it. Nope, that *sti

Re: [HACKERS] Another bug introduced by fastpath patch

2013-11-27 Thread Tom Lane
Andres Freund writes: > On 2013-11-27 17:25:44 -0500, Tom Lane wrote: >> Or we >> could add a restriction to EligibleForRelationFastPath that restricts >> the fast-path mechanism to non-session locks, in which case we'd not >> need to make the zeroing contingent on allLocks either. I don't think

Re: [HACKERS] Another bug introduced by fastpath patch

2013-11-27 Thread Andres Freund
On 2013-11-27 17:25:44 -0500, Tom Lane wrote: > Or we > could add a restriction to EligibleForRelationFastPath that restricts > the fast-path mechanism to non-session locks, in which case we'd not > need to make the zeroing contingent on allLocks either. I don't think > we take any fast-path-eligi

[HACKERS] Another bug introduced by fastpath patch

2013-11-27 Thread Tom Lane
In LockReleaseAll, we have this coding: for (partition = 0; partition < NUM_LOCK_PARTITIONS; partition++) { LWLockIdpartitionLock = FirstLockMgrLock + partition; SHM_QUEUE *procLocks = &(MyProc->myProcLocks[partition]); proclock = (PROCLOCK *) SHMQueueNext(pro