| Message ID | 1456151599-21941-1-git-send-email-cardoe@cardoe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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'.
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);
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);
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(-)