Re: [Xen-devel] [PATCH 05 of 14] Don't trigger unnecessary shadow scans on p2m entry update

["Jan Beulich" <JBeulich@xxxxxxxx>]

 >>> On 23.11.11 at 22:11, Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx> wrote:
> xen/arch/x86/mm/shadow/common.c |  4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> 
> When updating a p2m entry, the hypervisor scans all shadow pte's to find
> mappings of that gfn and tear them down. This is avoided if the page count
> reveals that there are no additional mappings. The current test ignores the
> PGC_allocated flag and its effect on the page count.
> 
> Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
> Signed-off-by: Adin Scannell <adin@xxxxxxxxxxx>
> 
> diff -r 93066bdc1e1c -r e8f0709af2b7 xen/arch/x86/mm/shadow/common.c
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -2501,7 +2501,8 @@ int sh_remove_all_mappings(struct vcpu *
>          ;
>  
>      perfc_incr(shadow_mappings);
> -    if ( (page->count_info & PGC_count_mask) == 0 )
> +    expected_count = (page->count_info & PGC_allocated) ? 1 : 0;
> +    if ( (page->count_info & PGC_count_mask) == expected_count )

Is that really valid outside the locked region?

>          return 0;
>  
>      /* Although this is an externally visible function, we do not know
> @@ -2517,7 +2518,6 @@ int sh_remove_all_mappings(struct vcpu *
>      hash_foreach(v, callback_mask, callbacks, gmfn);
>  
>      /* If that didn't catch the mapping, something is very wrong */
> -    expected_count = (page->count_info & PGC_allocated) ? 1 : 0;

This certainly isn't right - iiuc the count would normally have changed
during the hash_foreach() above this.

Jan

>      if ( (page->count_info & PGC_count_mask) != expected_count )
>      {
>          /* Don't complain if we're in HVM and there are some extra 
> mappings: 




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