[PATCH] xenoprof: drop necessary macro

[PATCH] xenoprof: drop necessary macro

[Doug Goldstein]

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.
---
 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");
+
     }
 }
 
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);
-- 
2.4.10

Re: [PATCH] xenoprof: drop necessary macro

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 341 bytes --]

On 2/22/16 8:33 AM, 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>

*laugh* That subject line should be 'unnecessary'.


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] xenoprof: drop necessary macro

[Andrew Cooper]

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);