Re: [PATCH 3/6] xen: arm: only put_page for p2m operations which require it.

[Julien Grall <julien.grall@linaro.org>]

Hi Ian,

On 06/10/2014 10:57 AM, Ian Campbell wrote:
> In particular do not do it for CACHEFLUSH.
> 
> INSERT, RELINQUISH and REMOVE should all put the page (if the current pte is
> valid). ALLOCATE doesn't need to since it asserts the current PTE must be
> invalid.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/p2m.c |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 9960e17..830a9f9 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -299,6 +299,23 @@ enum p2m_operation {
>      CACHEFLUSH,
>  };
>  
> +static void p2m_put_page(const lpae_t pte)
> +{
> +    /* TODO: Handle other p2m types
> +     *
> +     * It's safe to do the put_page here because page_alloc will
> +     * flush the TLBs if the page is reallocated before the end of
> +     * this loop.
> +     */
> +    if ( p2m_is_foreign(pte.p2m.type) )
> +    {
> +        unsigned long mfn = pte.p2m.base;
> +
> +        ASSERT(mfn_valid(mfn));
> +        put_page(mfn_to_page(mfn));
> +    }
> +}
> +

You forgot to drop the put_page in apply_p2m_changes. So, now we have 2
put_page rather than one.

Regards,

-- 
Julien Grall