Re: [Xen-devel] [RFC PATCH] PVH: cleanup of p2m upon p2m destroy

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.12.13 at 02:47, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> When a controlling domain is destroyed, any p2m_is_foreign pages must
> release the refcnt gotten when the page was added to the p2m.
> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>

First of all, you leave open what the relationship of this is with
your 7-patch v7 series. Why is this not part of it? Obviously any
necessary cleanup would logically have to come no later than
where the first creation of pages of this type happens...

> +/* This function to do any cleanup while walking the entire p2m */
> +int p2m_cleanup(struct domain *d)
> +{
> +    int rc = 0;
> +    unsigned long gfn, count = 0;
> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +
> +    if ( !is_pvh_domain(d) )
> +        return 0;
> +
> +    for ( gfn = p2m->next_foreign_gfn_to_check;
> +          gfn <= p2m->max_mapped_pfn; gfn++, count++ )
> +    {
> +        p2m_type_t p2mt;
> +        mfn_t mfn = get_gfn_query(d, gfn, &p2mt);
> +
> +        if ( unlikely(p2m_is_foreign(p2mt)) )
> +            put_page(mfn_to_page(mfn));
> +        put_gfn(d, gfn);
> +        
> +        /* Preempt every 10k pages, arbritary */
> +        if ( count == 10000 && hypercall_preempt_check() )
> +        {
> +            p2m->next_foreign_gfn_to_check = gfn + 1;
> +            rc = -EAGAIN;
> +            break;
> +        }

So it looks like you copied one of the two obvious bugs from
relinquish_shared_pages() _and_ deferred the preemption point
by quite a bit - 10,000 pages is quite a lot, the 512 used there
seems much more reasonable.

As to the copied bug: Should hypercall_preempt_check() return
false, you'd never again try to preempt.

Jan

> +    }
> +    return rc;
> +}



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel