On Fri, Apr 8, 2016 at 7:39 PM, Andres Freund <and...@anarazel.de> wrote:
> On 2016-04-07 16:50:44 +0300, Alexander Korotkov wrote: > > On Thu, Apr 7, 2016 at 4:41 PM, Andres Freund <and...@anarazel.de> > wrote: > > > > > On 2016-03-31 20:21:02 +0300, Alexander Korotkov wrote: > > > > ! BEGIN_BUFSTATE_CAS_LOOP(bufHdr); > > > > > > > > ! Assert(BUF_STATE_GET_REFCOUNT(state) > 0); > > > > ! wasDirty = (state & BM_DIRTY) ? true : false; > > > > ! state |= BM_DIRTY | BM_JUST_DIRTIED; > > > > ! if (state == oldstate) > > > > ! break; > > > > > > I'm doubtful that this early exit is entirely safe. None of the > > > preceding operations imply a memory barrier. The buffer could > previously > > > have been marked dirty, but cleaned since. It's pretty critical that we > > > re-set the dirty bit (there's no danger of loosing it with a barrier, > > > because we hold an exclusive content lock). > > > > > > > Oh, I get it. > > > > > > > Practically the risk seems fairly low, because acquiring the exclusive > > > content lock will have implied a barrier. But it seems unlikely to have > > > a measurable performance effect to me, so I'd rather not add the early > > > exit. > > > > > > > Ok, let's just remove it. > > Here's my updated version of the patch. I've updated this on an > intercontinental flight, after a otherwise hectic week (moving from SF > to Berlin); so I'm planning to look over this once more before pushing (. > Ok. I've decided that the cas-loop macros are too obfuscating for my > taste. To avoid duplicating the wait part I've introduced > WaitBufHdrUnlocked(). > That's OK for me. Cas-loop macros looks cute, but too magic. > As you can see in > > http://archives.postgresql.org/message-id/CA%2BTgmoaeRbN%3DZ4oWENLvgGLeHEvGZ_S_Z3KGrdScyKiSvNt3oA%40mail.gmail.com > I'm planning to apply this sometime this weekend, after running some > tests and going over the patch again. > > Any chance you could have a look over this? > I took a look at this. Changes you made look good for me. I also run test on 4x18 Intel server. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
