Re: [PATCH] LockAcquireExtended improvement

Hello Robert,

On 2024/2/2 5:05, Robert Haas wrote:
> On Thu, Feb 1, 2024 at 2:16 AM Jingxian Li <aqktjcm(at)qq(dot)com> wrote:
>> According to what you said, I resubmitted a patch which splits the ProcSleep
>> logic into two parts, the former is responsible for inserting self to
>> WaitQueue,
>> the latter is responsible for deadlock detection and processing, and the
>> former part is directly called by LockAcquireExtended before nowait fails.
>> In this way the nowait case can also benefit from adjusting the insertion
>> order of WaitQueue.
>
> I don't have time for a full review of this patch right now
> unfortunately, but just looking at it quickly:
>
> - It will be helpful if you write a clear commit message. If it gets
> committed, there is a high chance the committer will rewrite your
> message, but in the meantime it will help understanding.
>
> - The comment for InsertSelfIntoWaitQueue needs improvement. It is
> only one line. And it says "Insert self into queue if dontWait is
> false" but then someone will wonder why the function would ever be
> called with dontWait = true.
>
> - Between the comments and the commit message, the division of
> responsibilities between InsertSelfIntoWaitQueue and ProcSleep needs
> to be clearly explained. Right now I don't think it is.

Based on your comments above, I improve the commit message and comment for
InsertSelfIntoWaitQueue in new patch.

--
Jingxian Li