Re: [PATCH 4/5] x86/mm: Provide helpers for unaccepted memory

[Kirill A. Shutemov]

On Tue, Aug 10, 2021 at 11:16:26AM -0700, Dave Hansen wrote:
> On 8/9/21 11:26 PM, Kirill A. Shutemov wrote:
> > +void accept_memory(phys_addr_t start, phys_addr_t end)
> > +{
> > +	if (!boot_params.unaccepted_memory)
> > +		return;
> > +
> > +	spin_lock(&unaccepted_memory_lock);
> > +	__accept_memory(start, end);
> > +	spin_unlock(&unaccepted_memory_lock);
> > +}
> 
> Isn't this taken in the:
> 
> 	del_page_from_free_list()->
> 	clear_page_offline()->
> 	accept_memory()
> 
> call path?
> 
> That's underneath:
> 
> 	spin_lock_irqsave(&zone->lock, flags);
> 
> Which means that accept_memory() can happen from interrupt context.  Is
> it always covered by another spin_lock_irqsave() which means that it can
> use a plain spin_lock()?

I didn't give it enough thought yet, but we always run under zone lock
which has to use spin_lock_irqsave() if it called from interrupt context.

Having said that I think it is good idea to move clear_page_offline() out
zone lock. It should help with allocation latency. Not sure how messy it
gets. Merging/splitting path looks complex and I'm not an expert in the
page allocator.

> If so, it would be nice to call out that logic.  It *looks* like a
> spinlock that we would want to be spin_lock_irqsave().

-- 
 Kirill A. Shutemov