[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2] xen/page_alloc: Simplify domain_adjust_tot_pages
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.vallejo@xxxxxxxxx> > > > > Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> > > 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(). > 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... There's another side missing IMO, XENMEM_claim_pages should return an error (and refuse the set any claims) once d->creation_finished == true. Thanks, Roger.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |