Amit Kapila <amit.kapil...@gmail.com> writes: > On Tue, Aug 5, 2014 at 9:21 PM, Robert Haas <robertmh...@gmail.com> wrote: >>> I think you should get rid of BufFreelistLock completely and just >>> decide that freelist_lck will protect everything the freeNext links, plus >>> everything in StrategyControl except for nextVictimBuffer. victimbuf_lck >>> will protect nextVictimBuffer and nothing else.
> Another point is I think it will be better to protect > StrategyControl->completePasses with victimbuf_lck rather than > freelist_lck, as when we are going to update it we will already be > holding the victimbuf_lck and it doesn't make much sense to release > the victimbuf_lck and reacquire freelist_lck to update it. I'm rather concerned by this cavalier assumption that we can protect fields a,b,c with one lock and fields x,y,z in the same struct with some other lock. A minimum requirement for that to work safely at all is that the fields are of atomically fetchable/storable widths; which might be okay here but it's a restriction that bears thinking about (and documenting). But quite aside from safety, the fields are almost certainly going to be in the same cache line which means contention between processes that are trying to fetch or store them concurrently. For a patch whose sole excuse for existence is to improve performance, that should be a very scary concern. (And yes, I realize these issues already affect the freelist. Perhaps that's part of the reason we have performance issues with it.) regards, tom lane -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers