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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.09.15 at 18:45, <George.Dunlap@xxxxxxxxxxxxx> wrote:
> 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...

Okay, meaning we'd need to revert.

>> @@ -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?

You're right - those checks not depending on individual page
attributes in that condition have misled me to assume all are of
that kind. We indeed do need an inner loop dealing with per-page 
properties. I suppose you won't mind cleaning this up a little in the
course of the anyway necessary transformation.

Jan


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