Re: [PATCH v3 12/32] KVM: arm64: Introduce a Hyp buddy page allocator

[Quentin Perret]

On Thursday 04 Mar 2021 at 15:30:36 (+0000), Will Deacon wrote:
> On Tue, Mar 02, 2021 at 02:59:42PM +0000, Quentin Perret wrote:
> > +void *hyp_alloc_pages(struct hyp_pool *pool, unsigned int order)
> > +{
> > +	unsigned int i = order;
> > +	struct hyp_page *p;
> > +
> > +	hyp_spin_lock(&pool->lock);
> > +
> > +	/* Look for a high-enough-order page */
> > +	while (i < pool->max_order && list_empty(&pool->free_area[i]))
> > +		i++;
> > +	if (i >= pool->max_order) {
> > +		hyp_spin_unlock(&pool->lock);
> > +		return NULL;
> > +	}
> > +
> > +	/* Extract it from the tree at the right order */
> > +	p = list_first_entry(&pool->free_area[i], struct hyp_page, node);
> > +	p = __hyp_extract_page(pool, p, order);
> > +
> > +	hyp_spin_unlock(&pool->lock);
> > +	hyp_page_ref_inc(p);
> 
> I find this a little scary, as we momentarily drop the lock. It think
> it's ok because the reference count on the page must be 0 at this point,

Yep, @p shouldn't be visible to the caller yet so this should be fine.

> but actually then I think it would be clearer to have a
> hyp_page_ref_init() function which could take the lock, check that the
> refcount is indeed 0 and then set it to 1.

Works for me. Maybe I'll use another name for the API to stay consistent
with the kernel gfp code (hyp_page_ref_inc() and friends are inspired
from their kernel counterpart). And I guess I can hyp_panic() if the
refcount is not 0 at this point to match the VM_BUG_ON_PAGE() in
set_page_refcounted().

Thanks!
Quentin