Re: A question regarding LWLock in ProcSleep

+Heikki,

Hi Heikki,

Could you please help provide a case to elaborate why we need the LWLock
here? or could you please tell me to whom should I ask this question?

Thanks,
Kenan

On Tue, Dec 22, 2015 at 11:41 AM, Kenan Yao <kyao(at)pivotal(dot)io> wrote:

> 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(dot)kapila16(at)gmail(dot)com>
> wrote:
>
>> On Thu, Dec 17, 2015 at 1:51 PM, Kenan Yao <kyao(at)pivotal(dot)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
>>
>
>