A question regarding LWLock in ProcSleep

On Thu, Dec 17, 2015 at 1:51 PM, Kenan Yao <kyao@pivotal.io> wrote:

Hi there,

In function ProcSleep, after the process has been waken up, either with
lock granted or deadlock detected, it would re-acquire the lock table's
partition LWLock.

The code episode is here:

/*
* Re-acquire the lock table's partition lock. We have to do this to hold
* off cancel/die interrupts before we can mess with lockAwaited (else we
* might have a missed or duplicated locallock update).
*/
LWLockAcquire(partitionLock, LW_EXCLUSIVE);

/*
* We no longer want LockErrorCleanup to do anything.
*/
lockAwaited = NULL;

/*
* If we got the lock, be sure to remember it in the locallock table.
*/
if (MyProc->waitStatus == STATUS_OK)
GrantAwaitedLock();

/*
* We don't have to do anything else, because the awaker did all the
* necessary update of the lock table and MyProc.
*/
return MyProc->waitStatus;

​
Questions are:

(1) The comment says that "we might have a missed or duplicated locallock
update", in what cases would we hit this if without holding the LWLock?

(2) The comment says "we have to do this to hold off cancel/die
interrupts", then:

- why using LWLockAcquire instead of HOLD_INTERRUPTS directly?
- From the handler of SIGINT and SIGTERM, seems nothing serious would
be processed here, since no CHECK_FOR_INTERRUPS is called before releasing
this LWLock. Why we should hold off cancel/die interrupts here?

(3) Before releasing this LWLock, the only share memory access is
MyProc->waitStatus; since the process has been granted the lock or removed
from the lock's waiting list because of deadlock, is it possible some other
processes would access this field? if not, then why we need LWLock here?
what does this lock protect?

I think the other thing which needs protection of LWLock is
access to proclock which is done in the caller
(LockAcquireExtended).

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Hi Amit,

Thanks for the reply!

Yes, in LockAcquireExtended, after exiting WaitOnLock, there would be a
read access to proclock, however, since the lock has either been granted or
rejected by deadlock check, I can think of no possible concurrent write
access to the proclock, so is it really necessary to acquire the LWLock?

Thanks,
Kenan

On Fri, Dec 18, 2015 at 10:25 PM, Amit Kapila <amit.kapila16@gmail.com>
wrote:

On Thu, Dec 17, 2015 at 1:51 PM, Kenan Yao <kyao@pivotal.io> wrote:

Hi there,

In function ProcSleep, after the process has been waken up, either with
lock granted or deadlock detected, it would re-acquire the lock table's
partition LWLock.

The code episode is here:

/*
* Re-acquire the lock table's partition lock. We have to do this to hold
* off cancel/die interrupts before we can mess with lockAwaited (else we
* might have a missed or duplicated locallock update).
*/
LWLockAcquire(partitionLock, LW_EXCLUSIVE);

/*
* We no longer want LockErrorCleanup to do anything.
*/
lockAwaited = NULL;

/*
* If we got the lock, be sure to remember it in the locallock table.
*/
if (MyProc->waitStatus == STATUS_OK)
GrantAwaitedLock();

/*
* We don't have to do anything else, because the awaker did all the
* necessary update of the lock table and MyProc.
*/
return MyProc->waitStatus;

​
Questions are:

(1) The comment says that "we might have a missed or duplicated
locallock update", in what cases would we hit this if without holding the
LWLock?

(2) The comment says "we have to do this to hold off cancel/die
interrupts", then:

- why using LWLockAcquire instead of HOLD_INTERRUPTS directly?
- From the handler of SIGINT and SIGTERM, seems nothing serious
would be processed here, since no CHECK_FOR_INTERRUPS is called before
releasing this LWLock. Why we should hold off cancel/die interrupts here?

(3) Before releasing this LWLock, the only share memory access is
MyProc->waitStatus; since the process has been granted the lock or removed
from the lock's waiting list because of deadlock, is it possible some other
processes would access this field? if not, then why we need LWLock here?
what does this lock protect?

I think the other thing which needs protection of LWLock is
access to proclock which is done in the caller
(LockAcquireExtended).

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com