On Mon, Oct 31, 2022 at 5:19 PM Andres Freund <and...@anarazel.de> wrote:
> Hi, > > On 2022-10-31 17:17:03 -0700, Zhihong Yu wrote: > > On Mon, Oct 31, 2022 at 4:51 PM Andres Freund <and...@anarazel.de> > wrote: > > > > > Hi, > > > > > > On 2022-10-31 16:21:06 +0530, Bharath Rupireddy wrote: > > > > BTW, I've seen a sporadic crash (SEGV) with the patch in bg writer > > > > with the same set up [1], I'm not sure if it's really because of the > > > > patch. I'm unable to reproduce it now and unfortunately I didn't > > > > capture further details when it occurred. > > > > > > That's likely because the prototype patch I submitted in this thread > missed > > > updating LWLockUpdateVar(). > > > > > > Updated patch attached. > > > > > > Greetings, > > > > > > Andres Freund > > > > > > > Hi, > > Minor comment: > > > > + uint8 lwWaiting; /* see LWLockWaitState */ > > > > Why not declare `lwWaiting` of type LWLockWaitState ? > > Unfortunately C99 (*) doesn't allow to specify the width of an enum > field. With most compilers we'd end up using 4 bytes. > > Greetings, > > Andres Freund > > (*) C++ has allowed specifying this for quite a few years now and I think > C23 > will support it too, but that doesn't help us at this point. > Hi, Thanks for the response. If possible, it would be better to put your explanation in the code comment (so that other people know the reasoning).
