Re: [PATCH 3/7] xen/p2m: put reference for superpage

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, May 10, 2024 at 10:37:53PM +0100, Julien Grall wrote:
> Hi Roger,
> 
> On 09/05/2024 13:58, Roger Pau Monné wrote:
> > On Thu, May 09, 2024 at 01:12:00PM +0100, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 09/05/2024 12:28, Roger Pau Monné wrote:
> > > > OTOH for 1GB given the code here the page could be freed in one go,
> > > > without a chance of preempting the operation.
> > > > 
> > > > Maybe you have to shatter superpages into 4K entries and then remove
> > > > them individually, as to allow for preemption to be possible by
> > > > calling put_page() for each 4K chunk?
> > > This would require to allocate some pages from the P2M pool for the 
> > > tables.
> > > As the pool may be exhausted, it could be problematic when relinquishing 
> > > the
> > > resources.
> > 
> > Indeed, it's not ideal.
> > 
> > > It may be possible to find a way to have memory available by removing 
> > > other
> > > mappings first. But it feels a bit hackish and I would rather prefer if we
> > > avoid allocating any memory when relinquishing.
> > 
> > Maybe it could be helpful to provide a function to put a superpage,
> > that internally calls free_domheap_pages() with the appropriate order
> > so that freeing a superpage only takes a single free_domheap_pages()
> > call.
> 
> Today, free_domheap_page() is only called when the last reference is
> removed. I don't thinkt here is any guarantee that all the references will
> dropped at the same time.

I see, yes, we have no guarantee that removing the superpage entry in
the mapping domain will lead to either the whole superpage freed at
once, or not freed.  The source domain may have shattered the
super-page and hence freeing might need to be done at a smaller
granularity.

> >  That could reduce some of the contention around the heap_lock
> > and d->page_alloc_lock locks.
> 
> From previous experience (when Hongyan and I worked on optimizing
> init_heap_pages() for Live-Update), the lock is actually not the biggest
> problem. The issue is adding the pages back to the heap (which may requiring
> merging). So as long as the pages are not freed contiguously, we may not
> gain anything.

Would it help to defer the merging to the idle context, kind of
similar to what we do with scrubbing?

Thanks, Roger.