Re: mem_sharing_nominate_page: p2mt should never change before p2m_change_type()

Re: mem_sharing_nominate_page: p2mt should never change before p2m_change_type()

[Andres Lagar-Cavilla]

> The p2mt change check for p2m_change_type() was first introduced when this code path was not protected by p2m_lock().
> Now this code path is protected by p2m_lock. So p2mt should never change before p2m_change_type().

Makes sense. You need to put a proper Signed-off-by line. In any case:
Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>

Andres
> 
> ---
> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> index 0364bb0..b3940cb 100644
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -900,20 +900,8 @@ int mem_sharing_nominate_page(struct domain *d,
>         goto out;
>     }
> 
> -    /* Change the p2m type */
> -    if ( p2m_change_type(d, gfn, p2mt, p2m_ram_shared) != p2mt )
> -    {
> -        /* This is unlikely, as the type must have changed since we've checked
> -         * it a few lines above.
> -         * The mfn needs to revert back to rw type. This should never fail,
> -         * since no-one knew that the mfn was temporarily sharable */
> -        mem_sharing_gfn_destroy(page, d, gfn_info);
> -        xfree(page->sharing);
> -        page->sharing = NULL;
> -        /* NOTE: We haven't yet added this to the audit list. */
> -        BUG_ON(page_make_private(d, page) != 0);
> -        goto out;
> -    }
> +    /* Change the p2m type, should never fail with p2m locked. */
> +    BUG_ON(p2m_change_type(d, gfn, p2mt, p2m_ram_shared) != p2mt);
> 
>     /* Account for this page. */
>     atomic_inc(&nr_shared_mfns);
> 
> 
> 

Re: mem_sharing_nominate_page: p2mt should never change before p2m_change_type()

[Nai Xia]

[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]

Thanks. Please append the following line to the change log:
Signed-off-by: Nai Xia <nai.xia@gmail.com>

BR,
Nai Xia

On 2013年08月06日 23:38, Andres Lagar-Cavilla wrote:
>> The p2mt change check for p2m_change_type() was first introduced when this code path was not protected by p2m_lock().
>> Now this code path is protected by p2m_lock. So p2mt should never change before p2m_change_type().
> 
> Makes sense. You need to put a proper Signed-off-by line. In any case:
> Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
> 
> Andres
>>
>> ---
>> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
>> index 0364bb0..b3940cb 100644
>> --- a/xen/arch/x86/mm/mem_sharing.c
>> +++ b/xen/arch/x86/mm/mem_sharing.c
>> @@ -900,20 +900,8 @@ int mem_sharing_nominate_page(struct domain *d,
>>         goto out;
>>     }
>>
>> -    /* Change the p2m type */
>> -    if ( p2m_change_type(d, gfn, p2mt, p2m_ram_shared) != p2mt )
>> -    {
>> -        /* This is unlikely, as the type must have changed since we've checked
>> -         * it a few lines above.
>> -         * The mfn needs to revert back to rw type. This should never fail,
>> -         * since no-one knew that the mfn was temporarily sharable */
>> -        mem_sharing_gfn_destroy(page, d, gfn_info);
>> -        xfree(page->sharing);
>> -        page->sharing = NULL;
>> -        /* NOTE: We haven't yet added this to the audit list. */
>> -        BUG_ON(page_make_private(d, page) != 0);
>> -        goto out;
>> -    }
>> +    /* Change the p2m type, should never fail with p2m locked. */
>> +    BUG_ON(p2m_change_type(d, gfn, p2mt, p2m_ram_shared) != p2mt);
>>
>>     /* Account for this page. */
>>     atomic_inc(&nr_shared_mfns);
>>
>>
>>
> 

[-- Attachment #2: 0xE6BAA6F9.asc --]
[-- Type: application/pgp-keys, Size: 1674 bytes --]

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: mem_sharing_nominate_page: p2mt should never change before p2m_change_type()

[Tim Deegan]

At 00:25 +0800 on 07 Aug (1375835148), Nai Xia wrote:
> Thanks. Please append the following line to the change log:
> Signed-off-by: Nai Xia <nai.xia@gmail.com>

Done, acked + applied.

Cheers,

Tim.

> BR,
> Nai Xia
> 
> On 2013???08???06??? 23:38, Andres Lagar-Cavilla wrote:
> >> The p2mt change check for p2m_change_type() was first introduced when this code path was not protected by p2m_lock().
> >> Now this code path is protected by p2m_lock. So p2mt should never change before p2m_change_type().
> > 
> > Makes sense. You need to put a proper Signed-off-by line. In any case:
> > Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
> > 
> > Andres
> >>
> >> ---
> >> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> >> index 0364bb0..b3940cb 100644
> >> --- a/xen/arch/x86/mm/mem_sharing.c
> >> +++ b/xen/arch/x86/mm/mem_sharing.c
> >> @@ -900,20 +900,8 @@ int mem_sharing_nominate_page(struct domain *d,
> >>         goto out;
> >>     }
> >>
> >> -    /* Change the p2m type */
> >> -    if ( p2m_change_type(d, gfn, p2mt, p2m_ram_shared) != p2mt )
> >> -    {
> >> -        /* This is unlikely, as the type must have changed since we've checked
> >> -         * it a few lines above.
> >> -         * The mfn needs to revert back to rw type. This should never fail,
> >> -         * since no-one knew that the mfn was temporarily sharable */
> >> -        mem_sharing_gfn_destroy(page, d, gfn_info);
> >> -        xfree(page->sharing);
> >> -        page->sharing = NULL;
> >> -        /* NOTE: We haven't yet added this to the audit list. */
> >> -        BUG_ON(page_make_private(d, page) != 0);
> >> -        goto out;
> >> -    }
> >> +    /* Change the p2m type, should never fail with p2m locked. */
> >> +    BUG_ON(p2m_change_type(d, gfn, p2mt, p2m_ram_shared) != p2mt);
> >>
> >>     /* Account for this page. */
> >>     atomic_inc(&nr_shared_mfns);
> >>
> >>
> >>
> > 

> pub  1024D/E6BAA6F9 2008-06-09 Nai Xia <xianai@nju.edu.cn>
> sub  2048g/8AA69835 2008-06-09

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