Re: [PATCH] xenoprof: drop necessary macro

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 22/02/16 14:33, Doug Goldstein wrote:
> This macro doesn't really provide a benefit. When support is added the
> implementer can implement this how it needs to be and not conform to the
> macro.
>
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> ---
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>
> 1) I'm not partial to this patch. Feel free to NACK it.
> 2) The first white space chunk is intentional just as a clean up.

Whitespace fixup like this is fine if you make passing reference to it
in the commit message "e.g. While editing this area, drop trailing
whitespace".

> ---
>  xen/common/xenoprof.c          | 7 +++++--
>  xen/include/asm-x86/xenoprof.h | 8 --------
>  2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/xen/common/xenoprof.c b/xen/common/xenoprof.c
> index 7a3fc86..53f6d4a 100644
> --- a/xen/common/xenoprof.c
> +++ b/xen/common/xenoprof.c
> @@ -177,11 +177,14 @@ xenoprof_shared_gmfn_with_guest(
>      struct domain *d, unsigned long maddr, unsigned long gmaddr, int npages)
>  {
>      int i;
> -    
> +
>      for ( i = 0; i < npages; i++, maddr += PAGE_SIZE, gmaddr += PAGE_SIZE )
>      {
>          BUG_ON(page_get_owner(maddr_to_page(maddr)) != d);
> -        xenoprof_shared_gmfn(d, gmaddr, maddr);
> +        gdprintk(XENLOG_WARNING,
> +                 "xenoprof/x86 with autotranslated mode enabled"
> +                 "isn't supported yet\n");

Contrary to the usual 80 character rule, we prefer not splitting strings
like this, for grepability.

However, I would recommend shortening it to something like "xenoprof
unsupported with autotranslated guests".

Also as a bugfix, please put it in an "if ( i == 0 )" conditional, so we
don't get npages identical warning messages at once.

~Andrew

> +
>      }
>  }
>  
> diff --git a/xen/include/asm-x86/xenoprof.h b/xen/include/asm-x86/xenoprof.h
> index dca4223..3a1b001 100644
> --- a/xen/include/asm-x86/xenoprof.h
> +++ b/xen/include/asm-x86/xenoprof.h
> @@ -62,14 +62,6 @@ static inline int xenoprof_backtrace_supported(void)
>  void xenoprof_backtrace(struct vcpu *, const struct cpu_user_regs *,
>                          unsigned long depth, int mode);
>  
> -#define xenoprof_shared_gmfn(d, gmaddr, maddr)                      \
> -    do {                                                            \
> -        (void)(maddr);                                              \
> -        gdprintk(XENLOG_WARNING,                                    \
> -                 "xenoprof/x86 with autotranslated mode enabled"    \
> -                 "isn't supported yet\n");                          \
> -    } while (0)
> -
>  int passive_domain_do_rdmsr(unsigned int msr, uint64_t *msr_content);
>  int passive_domain_do_wrmsr(unsigned int msr, uint64_t msr_content);
>  void passive_domain_destroy(struct vcpu *v);