Message ID | 20170921124035.2410-12-julien.grall@arm.com |
---|---|
State | Superseded |
Headers | show |
Series | xen/x86: Clean-up the PoD code | expand |
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>
>>> 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
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 --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;