Re: convert various variables to atomics

On Tue, Sep 08, 2026 at 12:00:47PM -0500, Nathan Bossart wrote:
> * v2-0001: We are changing a variable from signed to unsigned, but the code
> goes out of its way to avoid negative values and signed integer overflow,
> so I don't think there are any real problems here. The only atomic
> arithmetic operation is in SICleanupQueue() where we subtract
> MSGNUMWRAPAROUND, which IIUC should never produce a negative value. That
> being said, I don't think it would be too disruptive to switch all relevant
> variables to uint32 as a prerequisite patch. I don't see any particular
> reason for those variables to be signed, anyway.

v3-0001 is the prerequisite patch. This requires some new clamping logic
in SICleanupQueue() for minsig and lowbound, since the subtractions can
produce negative values. I believe this retains the existing behavior, but
need to double-check.

> * v2-0006: The variables in this one are only ever incremented by 1, and
> they track the number of workers for a given operation, which I can't
> imagine approaches anything even close to overflowing an integer. Not to
> mention that we're using signed integers for all the relevant variables
> today... I don't see any risk here, but I'll try to switch the relevant
> variables to unsigned as a prerequisite and see how it looks. If it's too
> invasive, it's probably not worth worrying about.

Yeah, this looks far too invasive. I left it alone.

> * v2-0007: I think this one already does all the work to avoid any signed
> versus unsigned mismatches. The Assert() in SharedFileSetOnDetach() looks
> bogus, though, so I'll fix that. I guess there could be some risk of
> overflow in the "refcnt + 1" in SharedFileSetAttach(), but we don't handle
> that at all today, so I don't think we need to worry about it. (In theory
> this patch actually reduces the overflow risk by switching to unsigned,
> anyway.)

Upon closer inspection, the Assert() looks fine. I'm not sure why I
thought it was bogus.

--
nathan