On Tue Mar 11, 2025 at 3:45 PM GMT, Jan Beulich wrote: > 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 <[email protected]> > >>> > >>> Acked-by: Roger Pau Monné <[email protected]> > >> > >> 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
Why would it have to be arch-specific though? As far as the hypervisor is concerned, it doesn't seem to be. Cheers, Alejandro
