Neil Conway <neilc@samurai.com> writes:
> In other words, you might be able to somewhat reduce the size of the
> patch by, say, not renaming some exported functions in freelist.c, but I
> don't see that that will have a significant effect upon the complexity
> of the patch or the risk that it might cause regressions.
Well, the thing that's bothering me is that you are undoing a number of
changes that we'll probably just have to redo later; with consequently
*two* chances to introduce bugs. Case in point is moving the
responsibility for this:
+ /*
+ * Clear the buffer's tag. This doesn't matter for the hash table,
+ * since the buffer is already removed from it, but it ensures that
+ * sequential searches through the buffer table won't think the buffer
+ * is still valid for its old page.
+ */
+ CLEAR_BUFFERTAG(buf->tag);
out of freelist.c and putting it back in bufmgr.c. Jan actually
introduced a bug in his original ARC patch by deleting this code from
BufTableDelete and forgetting to put equivalent defenses into freelist.c.
I've got little confidence that this patch doesn't create some similar
problem, and none that we won't make the same mistake over again when
we re-revert the division of labor.
In short I'd rather try to minimize the amount of API churn that bufmgr.c
sees in this patch, because we'll probably just be changing it back later.
If the problem is that the current freelist.c API exposes too much about
the ARC implementation, the answer to that is not to go back to exposing
the LRU-list implementation; it's to generalize the API.
regards, tom lane
