Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Jan 13, 2011 at 10:43 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> This appears to remove the BM_JUST_DIRTIED logic. �Please explain why
>> that's not completely broken. �Even if it isn't completely broken,
>> it would seem better to do something like that as a separate patch.
> Well, the only point of BM_JUST_DIRTIED is to detect whether BM_DIRTY
> has been set while a buffer write is in progress. With this patch,
> only BM_HINT_BITS can be set while the buffer write is in progress;
> BM_DIRTY cannot. Perhaps one could make the argument that this would
> be a good cleanup anyway: in the unpatched code, BM_DIRTY can only be
> set while a buffer I/O is in progress if it is set due to a hint-bit
> update, and then we don't really care if the update gets lost.
> Although that seems a bit confusing...
[ thinks some more... ] If memory serves, the BM_JUST_DIRTIED mechanism
dates from a time when checkpoints would write dirty buffers without
taking any lock on them; if somebody changed the page meanwhile, the
buffer was just considered to remain dirty. We later decided that was
a bad idea and set up the current arrangement whereby only hint-bit
changes are allowed while a write is in progress. So you're right that
it would be dead code if we don't consider that a hint-bit change is
really dirtying the page. I'm not for removing it altogether though,
because it seems like something we could possibly want again in the
future (for instance, we might decide to go back to write-without-lock
to reduce lock contention). It's not like we are short of buffer flag
bits. Moreover this whole business of not treating hint-bit setting as
a page-dirtying operation is completely experimental/unproven IMO, so it
would be better to keep the patch footprint as small as possible. I'd
suggest leaving BM_JUST_DIRTIED as-is and just adding BM_HINT_BITS_DIRTY
as a new flag.
regards, tom lane
