Hello Robert, On 2024/2/2 5:05, Robert Haas wrote: > On Thu, Feb 1, 2024 at 2:16 AM Jingxian Li <aqkt...@qq.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
v2-0002-LockAcquireExtended-improvement.patch
Description: Binary data
