[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for-next 7/9] xen: Convert is_xen_heap_mfn to use typesafe MFN



>>> On 18.02.19 at 12:35, <julien.grall@xxxxxxx> wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4300,7 +4300,8 @@ int xenmem_add_to_physmap_one(
>  {
>      struct page_info *page = NULL;
>      unsigned long gfn = 0; /* gcc ... */
> -    unsigned long prev_mfn, old_gpfn;
> +    mfn_t prev_mfn;
> +    unsigned long old_gpfn;

Please can you put this together with "gfn"?

> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -2121,9 +2121,9 @@ void init_xenheap_pages(paddr_t ps, paddr_t pe)
>       * Yuk! Ensure there is a one-page buffer between Xen and Dom zones, to
>       * prevent merging of power-of-two blocks across the zone boundary.
>       */
> -    if ( ps && !is_xen_heap_mfn(paddr_to_pfn(ps)-1) )
> +    if ( ps && !is_xen_heap_mfn(_mfn(paddr_to_pfn(ps)-1)) )

Please add the missing blanks around - at the same time.

>          ps += PAGE_SIZE;
> -    if ( !is_xen_heap_mfn(paddr_to_pfn(pe)) )
> +    if ( !is_xen_heap_mfn(maddr_to_mfn(pe)) )

Why maddr_to_mfn() here but still paddr_to_pfn() above? Oh,
we don't have any mfn_sub(), I see.

> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -278,7 +278,7 @@ struct page_info
>  
>  #define is_xen_heap_page(page) ((page)->count_info & PGC_xen_heap)
>  #define is_xen_heap_mfn(mfn) \
> -    (__mfn_valid(mfn) && is_xen_heap_page(mfn_to_page(_mfn(mfn))))
> +    (__mfn_valid(mfn_x(mfn)) && is_xen_heap_page(mfn_to_page(mfn)))

Please don't open code mfn_valid().

With these minor issues taken care of
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.