On Tue Mar 11, 2025 at 12:46 PM GMT, Roger Pau Monné wrote: > On Tue, Mar 04, 2025 at 11:10:00AM +0000, Alejandro Vallejo wrote: > > The logic has too many levels of indirection and it's very hard to > > understand it its current form. Split it between the corner case where > > the adjustment is bigger than the current claim and the rest to avoid 5 > > auxiliary variables. > > > > Add a functional change to prevent negative adjustments from > > re-increasing the claim. This has the nice side effect of avoiding > > taking the heap lock here on every free. > > > > While at it, fix incorrect field name in nearby comment. > > > > Signed-off-by: Alejandro Vallejo <alejandro.vall...@cloud.com> > > Acked-by: Roger Pau Monné <roger....@citrix.com>
Thanks. > I think it would be nice to also ensure that once the domain is > finished building (maybe when it's unpaused for the first > time?) d->outstanding_pages is set to 0. IMO the claim system was > designed to avoid races during domain building, and shouldn't be used > once the domain is already running. > > Thanks, Roger. As a matter of implementation that's already the case by toolstack being "nice" and unconditionally clearing claims after populating the physmap. However, I agree the hypervisor should do it on its own. I didn't find a suitable place for it. It may be possible to do it on every unpause with minimal overhead because it doesn't need to take the heap lock unless there are outstanding claims on the domains. Would that sound like an ok idea? I'd rather not add even more state into "struct domain" to count pauses... Cheers, Alejandro
