Message ID | 56781BFA02000078000C1ECD@prv-mh.provo.novell.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 21/12/15 14:34, Jan Beulich wrote: > --- a/xen/arch/x86/io_apic.c > +++ b/xen/arch/x86/io_apic.c > @@ -2310,13 +2310,14 @@ int ioapic_guest_read(unsigned long phys > return 0; > } > > -#define WARN_BOGUS_WRITE(f, a...) \ > - dprintk(XENLOG_INFO, "\n%s: " \ > - "apic=%d, pin=%d, irq=%d\n" \ > - "%s: new_entry=%08x\n" \ > - "%s: " f, __FUNCTION__, apic, pin, irq, \ > - __FUNCTION__, *(u32 *)&rte, \ > - __FUNCTION__ , ##a ) > +#define WARN_BOGUS_WRITE(f, a...) \ > + dprintk(XENLOG_INFO, "\n" \ > + XENLOG_INFO "%s: apic=%d, pin=%d, irq=%d\n" \ > + XENLOG_INFO "%s: new_entry=%08x\n" \ > + XENLOG_INFO "%s: " f "\n", \ > + __func__, apic, pin, irq, \ > + __func__, *(u32 *)&rte, \ > + __func__, ##a ) > > int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val) > { > @@ -2388,7 +2389,7 @@ int ioapic_guest_write(unsigned long phy > rte.vector = desc->arch.vector; > if ( *(u32*)&rte != ret ) > WARN_BOGUS_WRITE("old_entry=%08x pirq=%d\n%s: " > - "Attempt to modify IO-APIC pin for in-use IRQ!\n", > + "Attempt to modify IO-APIC pin for in-use IRQ!", > ret, pirq, __FUNCTION__); Given that this is the sole user of WARN_BOGUS_WRITE(), I would recommend folding it all together in a simple dprintk(), and remove some of the redundant information, and fixing the resulting message to take up fewer lines. ~Andrew
>>> On 21.12.15 at 15:41, <andrew.cooper3@citrix.com> wrote: > On 21/12/15 14:34, Jan Beulich wrote: >> --- a/xen/arch/x86/io_apic.c >> +++ b/xen/arch/x86/io_apic.c >> @@ -2310,13 +2310,14 @@ int ioapic_guest_read(unsigned long phys >> return 0; >> } >> >> -#define WARN_BOGUS_WRITE(f, a...) \ >> - dprintk(XENLOG_INFO, "\n%s: " \ >> - "apic=%d, pin=%d, irq=%d\n" \ >> - "%s: new_entry=%08x\n" \ >> - "%s: " f, __FUNCTION__, apic, pin, irq, \ >> - __FUNCTION__, *(u32 *)&rte, \ >> - __FUNCTION__ , ##a ) >> +#define WARN_BOGUS_WRITE(f, a...) \ >> + dprintk(XENLOG_INFO, "\n" \ >> + XENLOG_INFO "%s: apic=%d, pin=%d, irq=%d\n" \ >> + XENLOG_INFO "%s: new_entry=%08x\n" \ >> + XENLOG_INFO "%s: " f "\n", \ >> + __func__, apic, pin, irq, \ >> + __func__, *(u32 *)&rte, \ >> + __func__, ##a ) >> >> int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val) >> { >> @@ -2388,7 +2389,7 @@ int ioapic_guest_write(unsigned long phy >> rte.vector = desc->arch.vector; >> if ( *(u32*)&rte != ret ) >> WARN_BOGUS_WRITE("old_entry=%08x pirq=%d\n%s: " >> - "Attempt to modify IO-APIC pin for in-use IRQ!\n", >> + "Attempt to modify IO-APIC pin for in-use IRQ!", >> ret, pirq, __FUNCTION__); > > Given that this is the sole user of WARN_BOGUS_WRITE(), I would > recommend folding it all together in a simple dprintk(), and remove some > of the redundant information, and fixing the resulting message to take > up fewer lines. If you feel strongly about this I could certainly do so, but I did consider that option and decided against it to retain the option of using the macro (again?) in a second place. Jan
On 21/12/15 14:48, Jan Beulich wrote: >>>> On 21.12.15 at 15:41, <andrew.cooper3@citrix.com> wrote: >> On 21/12/15 14:34, Jan Beulich wrote: >>> --- a/xen/arch/x86/io_apic.c >>> +++ b/xen/arch/x86/io_apic.c >>> @@ -2310,13 +2310,14 @@ int ioapic_guest_read(unsigned long phys >>> return 0; >>> } >>> >>> -#define WARN_BOGUS_WRITE(f, a...) \ >>> - dprintk(XENLOG_INFO, "\n%s: " \ >>> - "apic=%d, pin=%d, irq=%d\n" \ >>> - "%s: new_entry=%08x\n" \ >>> - "%s: " f, __FUNCTION__, apic, pin, irq, \ >>> - __FUNCTION__, *(u32 *)&rte, \ >>> - __FUNCTION__ , ##a ) >>> +#define WARN_BOGUS_WRITE(f, a...) \ >>> + dprintk(XENLOG_INFO, "\n" \ >>> + XENLOG_INFO "%s: apic=%d, pin=%d, irq=%d\n" \ >>> + XENLOG_INFO "%s: new_entry=%08x\n" \ >>> + XENLOG_INFO "%s: " f "\n", \ >>> + __func__, apic, pin, irq, \ >>> + __func__, *(u32 *)&rte, \ >>> + __func__, ##a ) >>> >>> int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val) >>> { >>> @@ -2388,7 +2389,7 @@ int ioapic_guest_write(unsigned long phy >>> rte.vector = desc->arch.vector; >>> if ( *(u32*)&rte != ret ) >>> WARN_BOGUS_WRITE("old_entry=%08x pirq=%d\n%s: " >>> - "Attempt to modify IO-APIC pin for in-use IRQ!\n", >>> + "Attempt to modify IO-APIC pin for in-use IRQ!", >>> ret, pirq, __FUNCTION__); >> Given that this is the sole user of WARN_BOGUS_WRITE(), I would >> recommend folding it all together in a simple dprintk(), and remove some >> of the redundant information, and fixing the resulting message to take >> up fewer lines. > If you feel strongly about this I could certainly do so, but I did > consider that option and decided against it to retain the option > of using the macro (again?) in a second place. I don't forsee it being useful to use again. Either way, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> ~Andrew
--- a/xen/arch/x86/acpi/power.c +++ b/xen/arch/x86/acpi/power.c @@ -268,7 +268,7 @@ int acpi_enter_sleep(struct xenpf_enter_ else if ( sleep->val_b && ((sleep->val_a ^ sleep->val_b) & ACPI_BITMASK_SLEEP_ENABLE) ) { - gdprintk(XENLOG_ERR, "Mismatched pm1a/pm1b setting."); + gdprintk(XENLOG_ERR, "Mismatched pm1a/pm1b setting\n"); return -EINVAL; } --- a/xen/arch/x86/io_apic.c +++ b/xen/arch/x86/io_apic.c @@ -2310,13 +2310,14 @@ int ioapic_guest_read(unsigned long phys return 0; } -#define WARN_BOGUS_WRITE(f, a...) \ - dprintk(XENLOG_INFO, "\n%s: " \ - "apic=%d, pin=%d, irq=%d\n" \ - "%s: new_entry=%08x\n" \ - "%s: " f, __FUNCTION__, apic, pin, irq, \ - __FUNCTION__, *(u32 *)&rte, \ - __FUNCTION__ , ##a ) +#define WARN_BOGUS_WRITE(f, a...) \ + dprintk(XENLOG_INFO, "\n" \ + XENLOG_INFO "%s: apic=%d, pin=%d, irq=%d\n" \ + XENLOG_INFO "%s: new_entry=%08x\n" \ + XENLOG_INFO "%s: " f "\n", \ + __func__, apic, pin, irq, \ + __func__, *(u32 *)&rte, \ + __func__, ##a ) int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val) { @@ -2388,7 +2389,7 @@ int ioapic_guest_write(unsigned long phy rte.vector = desc->arch.vector; if ( *(u32*)&rte != ret ) WARN_BOGUS_WRITE("old_entry=%08x pirq=%d\n%s: " - "Attempt to modify IO-APIC pin for in-use IRQ!\n", + "Attempt to modify IO-APIC pin for in-use IRQ!", ret, pirq, __FUNCTION__); return 0; } --- a/xen/arch/x86/x86_64/mm.c +++ b/xen/arch/x86/x86_64/mm.c @@ -1366,8 +1366,9 @@ int memory_add(unsigned long spfn, unsig if ( !valid_numa_range(spfn << PAGE_SHIFT, epfn << PAGE_SHIFT, node) ) { - dprintk(XENLOG_WARNING, "spfn %lx ~ epfn %lx pxm %x node %x" - "is not numa valid", spfn, epfn, pxm, node); + printk(XENLOG_WARNING + "pfn range %lx..%lx PXM %x node %x is not NUMA-valid\n", + spfn, epfn, pxm, node); return -EINVAL; }