diff mbox series

[Xen-devel,v2,11/16] xen/x86: p2m-pod: Clean-up p2m_pod_zero_check

Message ID 20170921124035.2410-12-julien.grall@arm.com
State Superseded
Headers show
Series xen/x86: Clean-up the PoD code | expand

Commit Message

Julien Grall Sept. 21, 2017, 12:40 p.m. UTC 
Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

---

Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

    Changes in v2:
        - Add Andrew's acked-by
---
 xen/arch/x86/mm/p2m-pod.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Wei Liu Sept. 21, 2017, 3:35 p.m. UTC | #1 
On Thu, Sep 21, 2017 at 01:40:30PM +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Jan Beulich Sept. 22, 2017, 9:26 a.m. UTC | #2 
>>> On 21.09.17 at 14:40, <julien.grall@arm.com> wrote:
> --- a/xen/arch/x86/mm/p2m-pod.c
> +++ b/xen/arch/x86/mm/p2m-pod.c
> @@ -861,17 +861,19 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long *gfns, int count)
>      for ( i = 0; i < count; i++ )
>      {
>          p2m_access_t a;
> +        struct page_info *pg;
>  
>          mfns[i] = p2m->get_entry(p2m, _gfn(gfns[i]), types + i, &a,
>                                   0, NULL, NULL);
> +        pg = mfn_to_page(mfns[i]);
> +
>          /*
>           * If this is ram, and not a pagetable or from the xen heap, and
>           * probably not mapped elsewhere, map it; otherwise, skip.
>           */
> -        if ( p2m_is_ram(types[i])
> -             && ( (mfn_to_page(mfns[i])->count_info & PGC_allocated) != 0 )
> -             && ( (mfn_to_page(mfns[i])->count_info & (PGC_page_table|PGC_xen_heap)) == 0 )
> -             && ( (mfn_to_page(mfns[i])->count_info & PGC_count_mask) <= max_ref ) )
> +        if ( p2m_is_ram(types[i]) && (pg->count_info & PGC_allocated) &&

If you omit the != 0 here (which I appreciate) ...

> +             ((pg->count_info & (PGC_page_table | PGC_xen_heap)) == 0) &&

... you should also use ! instead of == 0 here.

> +             ((pg->count_info & (PGC_count_mask)) <= max_ref) )

Stray innermost parentheses left?

Jan
Julien Grall Oct. 2, 2017, 12:43 p.m. UTC | #3 
Hi Jan,

On 22/09/17 10:26, Jan Beulich wrote:
>>>> On 21.09.17 at 14:40, <julien.grall@arm.com> wrote:
>> --- a/xen/arch/x86/mm/p2m-pod.c
>> +++ b/xen/arch/x86/mm/p2m-pod.c
>> @@ -861,17 +861,19 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long *gfns, int count)
>>       for ( i = 0; i < count; i++ )
>>       {
>>           p2m_access_t a;
>> +        struct page_info *pg;
>>   
>>           mfns[i] = p2m->get_entry(p2m, _gfn(gfns[i]), types + i, &a,
>>                                    0, NULL, NULL);
>> +        pg = mfn_to_page(mfns[i]);
>> +
>>           /*
>>            * If this is ram, and not a pagetable or from the xen heap, and
>>            * probably not mapped elsewhere, map it; otherwise, skip.
>>            */
>> -        if ( p2m_is_ram(types[i])
>> -             && ( (mfn_to_page(mfns[i])->count_info & PGC_allocated) != 0 )
>> -             && ( (mfn_to_page(mfns[i])->count_info & (PGC_page_table|PGC_xen_heap)) == 0 )
>> -             && ( (mfn_to_page(mfns[i])->count_info & PGC_count_mask) <= max_ref ) )
>> +        if ( p2m_is_ram(types[i]) && (pg->count_info & PGC_allocated) &&
> 
> If you omit the != 0 here (which I appreciate) ...
> 
>> +             ((pg->count_info & (PGC_page_table | PGC_xen_heap)) == 0) &&
> 
> ... you should also use ! instead of == 0 here.

Good point. I will do that.

> 
>> +             ((pg->count_info & (PGC_count_mask)) <= max_ref) )
> 
> Stray innermost parentheses left?

I will drop the parentheses around PGC_count_mask.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 176d06cb42..611a087855 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -861,17 +861,19 @@  p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long *gfns, int count)
     for ( i = 0; i < count; i++ )
     {
         p2m_access_t a;
+        struct page_info *pg;
 
         mfns[i] = p2m->get_entry(p2m, _gfn(gfns[i]), types + i, &a,
                                  0, NULL, NULL);
+        pg = mfn_to_page(mfns[i]);
+
         /*
          * If this is ram, and not a pagetable or from the xen heap, and
          * probably not mapped elsewhere, map it; otherwise, skip.
          */
-        if ( p2m_is_ram(types[i])
-             && ( (mfn_to_page(mfns[i])->count_info & PGC_allocated) != 0 )
-             && ( (mfn_to_page(mfns[i])->count_info & (PGC_page_table|PGC_xen_heap)) == 0 )
-             && ( (mfn_to_page(mfns[i])->count_info & PGC_count_mask) <= max_ref ) )
+        if ( p2m_is_ram(types[i]) && (pg->count_info & PGC_allocated) &&
+             ((pg->count_info & (PGC_page_table | PGC_xen_heap)) == 0) &&
+             ((pg->count_info & (PGC_count_mask)) <= max_ref) )
             map[i] = map_domain_page(mfns[i]);
         else
             map[i] = NULL;