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

[Julien Grall]

Hi Jan,

On 22/09/17 10:26, Jan Beulich wrote:

On 21.09.17 at 14:40,  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, ,

   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,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[Jan Beulich]

>>> On 21.09.17 at 14:40,  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, ,
>   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


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[Wei Liu]

On Thu, Sep 21, 2017 at 01:40:30PM +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall 
> Acked-by: Andrew Cooper 

Reviewed-by: Wei Liu 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[Julien Grall]

Signed-off-by: Julien Grall 
Acked-by: Andrew Cooper 

---

Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

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

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, ,
  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;
-- 
2.11.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel