[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86/mm: Make asserts on types and counts of shared pages more accurate


  • To: "Tim Deegan" <tim@xxxxxxx>
  • From: "Andres Lagar-Cavilla" <andres@xxxxxxxxxxxxxxxx>
  • Date: Thu, 9 Feb 2012 06:50:22 -0800
  • Cc: andres@xxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxx, adin@xxxxxxxxxxxxxx
  • Delivery-date: Thu, 09 Feb 2012 14:50:41 +0000
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=lagarcavilla.org; h=message-id :in-reply-to:references:date:subject:from:to:cc:reply-to :mime-version:content-type:content-transfer-encoding; q=dns; s= lagarcavilla.org; b=kapYGm1UyTUiDMqDLrWe4gKLNr4mMPK648IsWxS6UUcZ P+F4m6jdbIud2KmVi37oLMGGe1hoNhmW+Mf7RfVYR8MyjRkJbGUdQn5SsRwRX+0d qyodbNZ1EM56fqwzIuybNbzyyyIJf2518jY77S/cgYjeZRotbun265sMC9/pn7A=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>

> At 01:01 -0500 on 09 Feb (1328749297), Andres Lagar-Cavilla wrote:
>>  xen/arch/x86/mm/mem_sharing.c |  4 +++-
>>  xen/arch/x86/mm/p2m.c         |  6 ++++--
>>  2 files changed, 7 insertions(+), 3 deletions(-)
>>
>>
>> Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
>
> NACK, I'm afraid, especially the second one.  '<=' comparisons with a
> number that's made up of a count ORed with a type don't make sense.
> If you want to test both type and count, just test them separately.

The type I'm ORing with is a mask with single bit set, not a multi-bit
type mask. And the type is defined as a higher order bit than the count
mask. So, nothing that I'm ORing with could get in the way of the numeric
comparison.

Having said that I understand it's not terribly elegant or easy to
understand.

Andres

>
> Cheers,
>
> Tim.
>
>> diff -r 667191f054c3 -r f2efbfaa8d26 xen/arch/x86/mm/mem_sharing.c
>> --- a/xen/arch/x86/mm/mem_sharing.c
>> +++ b/xen/arch/x86/mm/mem_sharing.c
>> @@ -200,7 +200,9 @@ static struct page_info* mem_sharing_loo
>>              /* Count has to be at least two, because we're called
>>               * with the mfn locked (1) and this is supposed to be
>>               * a shared page (1). */
>> -            ASSERT((page->u.inuse.type_info & PGT_count_mask) >= 2);
>> +            ASSERT((page->u.inuse.type_info &
>> +                        (PGT_shared_page | PGT_count_mask)) >=
>> +                            (PGT_shared_page | 2));
>>              ASSERT(get_gpfn_from_mfn(mfn) == SHARED_M2P_ENTRY);
>>              return page;
>>          }
>> diff -r 667191f054c3 -r f2efbfaa8d26 xen/arch/x86/mm/p2m.c
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -745,8 +745,10 @@ set_shared_p2m_entry(struct domain *d, u
>>       * sharable first */
>>      ASSERT(p2m_is_shared(ot));
>>      ASSERT(mfn_valid(omfn));
>> -    if ( ((mfn_to_page(omfn)->u.inuse.type_info & PGT_type_mask)
>> -                    != PGT_shared_page) )
>> +    /* Set the m2p entry to invalid only if there are no further type
>> +     * refs to this page as shared */
>> +    if ( (mfn_to_page(omfn)->u.inuse.type_info &
>> +            (PGT_shared_page | PGT_count_mask)) <= PGT_shared_page )
>>          set_gpfn_from_mfn(mfn_x(omfn), INVALID_M2P_ENTRY);
>>
>>      P2M_DEBUG("set shared %lx %lx\n", gfn, mfn_x(mfn));
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
>



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.