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

[Tim Deegan <tim@xxxxxxx>]

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.

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