On 11.03.2025 19:35, Alejandro Vallejo wrote: > 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. > > Why would it have to be arch-specific though? As far as the hypervisor is > concerned, it doesn't seem to be.
Together with Roger's earlier clarification on his original remark, I fear I don't understand the question: I asked that it not be arch-specific. And Roger clarified that he also didn't mean it to be. Jan
