Re: [Xen-devel] [PATCH v2] x86/PoD: shorten certain operations on higher order ranges

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Mon, Sep 28, 2015 at 3:30 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> Now that p2m->get_entry() always returns a valid order, utilize this
> to accelerate some of the operations in PoD code. (There are two uses
> of p2m->get_entry() left which don't easily lend themselves to this
> optimization.)
>
> Also adjust a few types as needed and remove stale comments from
> p2m_pod_cache_add() (to avoid duplicating them yet another time).
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

So I was just looking at my own suggestion, and I want to withdraw my
"reviewed-by" temporarily to ask a question...

> @@ -649,7 +656,8 @@ p2m_pod_zero_check_superpage(struct p2m_
>      p2m_type_t type, type0 = 0;
>      unsigned long * map = NULL;
>      int ret=0, reset = 0;
> -    int i, j;
> +    unsigned long i, n;
> +    unsigned int j;
>      int max_ref = 1;
>      struct domain *d = p2m->domain;
>
> @@ -668,10 +676,13 @@ p2m_pod_zero_check_superpage(struct p2m_
>
>      /* Look up the mfns, checking to make sure they're the same mfn
>       * and aligned, and mapping them. */
> -    for ( i=0; i<SUPERPAGE_PAGES; i++ )
> +    for ( i = 0; i < SUPERPAGE_PAGES; i += n )
>      {
>          p2m_access_t a;
> -        mfn = p2m->get_entry(p2m, gfn + i, &type, &a, 0, NULL, NULL);
> +        unsigned int cur_order;
> +
> +        mfn = p2m->get_entry(p2m, gfn + i, &type, &a, 0, &cur_order, NULL);
> +        n = 1UL << min(cur_order, SUPERPAGE_ORDER + 0U);
>
>          if ( i == 0 )
>          {

The check at the bottom of this loop not only checks to see that all
the mfns are contiguous; they also check that each of the indivual
(4k) mfns has certain properties; namely:

+ None of the mfns are used as pagetables, or allocated via xenheap
+ None of the mfns are likely to be mapped elsewhere (refcount 2 or
less for shadow, 1 for hap)

This change makes it so that if the p2m entry is 2M or larger, only
the first mfn actually gets checked.  But I don't think we can assume
that just because the first page of a superpage is not used as a
pagetable, or mapped elsewhere, that none of the pages are.  (Please
correct me if I'm wrong here.)

If that's the case, then we would need to run this loop all the way
through, even if cur_order is SUPERPAGE_ORDER.

I suppose we could have two loops, one which checks for superpage
integrity, and another which checks for the necessary properties for
each individual mfn in the superpage.

Thoughts?

 -George

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