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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.09.15 at 16:23, <george.dunlap@xxxxxxxxxx> wrote:
> On 30/09/15 13:12, Jan Beulich wrote:
>>>>> 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.
> 
> Yes, I've got a prototype patch I'll probably send out tomorrow.

Oh, I've got one too (actually the original, now reverted one
suitably fixed up). Just didn't get around to test it yet.

Jan


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