Re: [Xen-devel] [RFC PATCH for-next 04/18] x86/mem_sharing: cleanup code in various locations

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.09.2019 17:48, Tamas K Lengyel wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1879,12 +1879,11 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned 
> long gla,
>      if ( npfec.write_access && (p2mt == p2m_ram_shared) )
>      {
>          ASSERT(p2m_is_hostp2m(p2m));
> -        sharing_enomem = 
> -            (mem_sharing_unshare_page(currd, gfn, 0) < 0);
> +        sharing_enomem = mem_sharing_unshare_page(currd, gfn, 0);

I guess the implication here is that the function can only return
-ENOMEM? Not very forward compatible, but well. However, if you
touch this already, shouldn't you at least make "sharing_enomem"
bool?

> @@ -1240,11 +1277,11 @@ int __mem_sharing_unshare_page(struct domain *d,
>      mem_sharing_page_unlock(old_page);
>      put_page_and_type(old_page);
>  
> -private_page_found:    
> +private_page_found:

Please also indent this label by (at least) one blank.

> @@ -57,34 +59,34 @@ struct page_sharing_info
>      };
>  };
>  
> -#define sharing_supported(_d) \
> -    (is_hvm_domain(_d) && paging_mode_hap(_d)) 
> -
>  unsigned int mem_sharing_get_nr_saved_mfns(void);
>  unsigned int mem_sharing_get_nr_shared_mfns(void);
>  
>  #define MEM_SHARING_DESTROY_GFN       (1<<1)
>  /* Only fails with -ENOMEM. Enforce it with a BUG_ON wrapper. */
>  int __mem_sharing_unshare_page(struct domain *d,
> -                             unsigned long gfn, 
> -                             uint16_t flags);
> -static inline int mem_sharing_unshare_page(struct domain *d,
> -                                           unsigned long gfn,
> -                                           uint16_t flags)
> +                               unsigned long gfn,
> +                               uint16_t flags);
> +
> +static inline
> +int mem_sharing_unshare_page(struct domain *d,
> +                             unsigned long gfn,
> +                             uint16_t flags)
>  {
>      int rc = __mem_sharing_unshare_page(d, gfn, flags);
>      BUG_ON( rc && (rc != -ENOMEM) );

Would you mind also dropping the stray blanks here?

Jan

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