Hi, Thank you for your quick reply
I misunderstood the logic of pg_atomic_compare_exchange_u32, so the loop cannot be infinite.
> I wonder if we should make ->nextVictimBuffer a 64bit atomic. At the time the changes went in we didn't (or rather, couldn't) rely on it, but these days we could. I think with a 64bit number we could get rid of ->completePasses and just infer it from ->nextVictimBuffer/NBuffers, removing th need for the spinlock. It's very unlikely that 64bit would ever wrap, and even if, it'd just be a small inaccuracy in the assumed number of passes. OTOH, it's doubtful the overflow handling / the spinlock matters performance wise.
I'm not sure why 64 bit was not used at the time, so I'm concerned about it.
but, except for it, you have a point and I completely agree with you. as you have said, we should use 64 bit whose higher-order 32 bit indicates completePasses, and should remove spinlock.
maybe we don't have to exceptionally worry about the overflow here mainly because, even now, the completePasses can overflow and the possibility of overflow may not be so different so that the 64 bit atomic operation is better.
if overflow would happen, passes_delta variable in the function called by bgwriter would be negative high value and it would lead to the failure of assert. (the code is below
Do you send patch for the replacement with 64 bit? If you don't mind, I would like to send patch. ( or is there some procedure before sending patch?
Thanks
hayato