On 27.02.2025 15:59, Alejandro Vallejo wrote: > On Wed Feb 26, 2025 at 2:02 PM GMT, Jan Beulich wrote: >> On 24.02.2025 14:27, Alejandro Vallejo wrote: >>> @@ -504,17 +502,16 @@ unsigned long domain_adjust_tot_pages(struct domain >>> *d, long pages) >>> goto out; >>> >>> spin_lock(&heap_lock); >>> - /* adjust domain outstanding pages; may not go negative */ >>> - dom_before = d->outstanding_pages; >>> - dom_after = dom_before - pages; >>> - BUG_ON(dom_before < 0); >>> - dom_claimed = dom_after < 0 ? 0 : dom_after; >>> - d->outstanding_pages = dom_claimed; >>> - /* flag accounting bug if system outstanding_claims would go negative >>> */ >>> - sys_before = outstanding_claims; >>> - sys_after = sys_before - (dom_before - dom_claimed); >>> - BUG_ON(sys_after < 0); >>> - outstanding_claims = sys_after; >>> + BUG_ON(outstanding_claims < d->outstanding_pages); >>> + if ( pages > 0 && d->outstanding_pages < pages ) >> >> The lhs isn't needed, is it? d->outstanding_pages is an unsigned quantity, >> after all. Else dropping the earlier of the two BUG_ON() wouldn't be quite >> right. > > d->outstanding pages is unsigned, but pages isn't. > > It was originally like that, but I then got concerned about 32bit machines, > where you'd be comparing a signed and an unsigned integer of the same > not-very-large width. That seems like dangerous terrains if the unsigned > number > grows large enough.
Oh, indeed - I didn't take ILP32 arches into account. Jan
