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.
> + {
> + /* `pages` exceeds the domain's outstanding count. Zero it out. */
> + outstanding_claims -= d->outstanding_pages;
> + d->outstanding_pages = 0;
> + } else {
Nit: Braces on their own lines please.
In any event - yes, this reads quite a bit easier after the adjustment.
With the adjustments (happy to make while committing, so long as you agree)
Reviewed-by: Jan Beulich <[email protected]>
Jan
> + outstanding_claims -= pages;
> + d->outstanding_pages -= pages;
> + }
> spin_unlock(&heap_lock);
>
> out:
