On 11.03.2025 16:42, Roger Pau Monné wrote: > On Tue, Mar 11, 2025 at 02:53:04PM +0000, Alejandro Vallejo wrote: >> 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. > > I see. Another option would be to refuse the unpause a domain if it > still has pending claims. However I don't know how that will work out > with all possible toolstacks. > >> However, I agree the hypervisor should do it on its own. I didn't find a >> suitable place for it. > > You could do it in arch_domain_creation_finished().
Except that better wouldn't be arch-specific. Jan
