* [PATCH] x86: drop pointless uses of __func__ / __FUNCTION__
@ 2016-08-24 8:02 Jan Beulich
2016-08-24 20:34 ` Konrad Rzeszutek Wilk
2016-08-31 18:43 ` Andrew Cooper
0 siblings, 2 replies; 6+ messages in thread
From: Jan Beulich @ 2016-08-24 8:02 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 4518 bytes --]
Non-debugging message text should be (and is in the cases here)
distinguishable without also logging function names. Debugging message
text, otoh, already includes file name and line number, so also
logging function names is redundant. One relatively pointless debugging
message gets removed altogether. In another case a mising log level
specifier gets added at once.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2307,14 +2307,12 @@ int ioapic_guest_read(unsigned long phys
return 0;
}
-#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 )
+#define WARN_BOGUS_WRITE(f, a...) \
+ dprintk(XENLOG_INFO, "\n" \
+ XENLOG_INFO "IO-APIC: apic=%d, pin=%d, irq=%d\n" \
+ XENLOG_INFO "IO-APIC: new_entry=%08x\n" \
+ XENLOG_INFO "IO-APIC: " f "\n", \
+ apic, pin, irq, *(u32 *)&rte, ##a )
int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val)
{
@@ -2385,9 +2383,9 @@ int ioapic_guest_write(unsigned long phy
spin_unlock_irqrestore(&ioapic_lock, flags);
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!",
- ret, pirq, __FUNCTION__);
+ WARN_BOGUS_WRITE("old_entry=%08x pirq=%d\n" XENLOG_INFO
+ "IO-APIC: Attempt to modify IO-APIC pin for in-use IRQ!",
+ ret, pirq);
return 0;
}
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -404,10 +404,8 @@ static vmask_t *irq_get_used_vector_mask
ret = &global_used_vector_map;
if ( desc->arch.used_vectors )
- {
- printk(XENLOG_INFO "%s: Strange, unassigned irq %d already has used_vectors!\n",
- __func__, irq);
- }
+ printk(XENLOG_INFO "Unassigned IRQ %d already has used_vectors\n",
+ irq);
else
{
int vector;
@@ -415,8 +413,8 @@ static vmask_t *irq_get_used_vector_mask
vector = irq_to_vector(irq);
if ( vector > 0 )
{
- printk(XENLOG_INFO "%s: Strange, irq %d already assigned vector %d!\n",
- __func__, irq, vector);
+ printk(XENLOG_INFO "IRQ %d already assigned vector %d\n",
+ irq, vector);
ASSERT(!test_bit(vector, ret));
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -164,10 +164,7 @@ void msi_compose_msg(unsigned vector, co
memset(msg, 0, sizeof(*msg));
if ( !cpumask_intersects(cpu_mask, &cpu_online_map) )
- {
- dprintk(XENLOG_ERR,"%s, compose msi message error!!\n", __func__);
return;
- }
if ( vector )
{
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1467,8 +1467,7 @@ static int __init verify_tsc_reliability
tsc_check_reliability();
if ( tsc_max_warp )
{
- printk("%s: TSC warp detected, disabling TSC_RELIABLE\n",
- __func__);
+ printk("TSC warp detected, disabling TSC_RELIABLE\n");
setup_clear_cpu_cap(X86_FEATURE_TSC_RELIABLE);
}
}
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -574,8 +574,8 @@ void xstate_init(struct cpuinfo_x86 *c)
* We know FP/SSE and YMM about eax, and nothing about edx at present.
*/
xsave_cntxt_size = _xstate_ctxt_size(feature_mask);
- printk("%s: using cntxt_size: %#x and states: %#"PRIx64"\n",
- __func__, xsave_cntxt_size, xfeature_mask);
+ printk("xstate: size: %#x and states: %#"PRIx64"\n",
+ xsave_cntxt_size, xfeature_mask);
asm ( "fxsave %0" : "=m" (ctxt) );
if ( ctxt.mxcsr_mask )
[-- Attachment #2: x86-drop-__func__.patch --]
[-- Type: text/plain, Size: 4569 bytes --]
x86: drop pointless uses of __func__ / __FUNCTION__
Non-debugging message text should be (and is in the cases here)
distinguishable without also logging function names. Debugging message
text, otoh, already includes file name and line number, so also
logging function names is redundant. One relatively pointless debugging
message gets removed altogether. In another case a mising log level
specifier gets added at once.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2307,14 +2307,12 @@ int ioapic_guest_read(unsigned long phys
return 0;
}
-#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 )
+#define WARN_BOGUS_WRITE(f, a...) \
+ dprintk(XENLOG_INFO, "\n" \
+ XENLOG_INFO "IO-APIC: apic=%d, pin=%d, irq=%d\n" \
+ XENLOG_INFO "IO-APIC: new_entry=%08x\n" \
+ XENLOG_INFO "IO-APIC: " f "\n", \
+ apic, pin, irq, *(u32 *)&rte, ##a )
int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val)
{
@@ -2385,9 +2383,9 @@ int ioapic_guest_write(unsigned long phy
spin_unlock_irqrestore(&ioapic_lock, flags);
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!",
- ret, pirq, __FUNCTION__);
+ WARN_BOGUS_WRITE("old_entry=%08x pirq=%d\n" XENLOG_INFO
+ "IO-APIC: Attempt to modify IO-APIC pin for in-use IRQ!",
+ ret, pirq);
return 0;
}
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -404,10 +404,8 @@ static vmask_t *irq_get_used_vector_mask
ret = &global_used_vector_map;
if ( desc->arch.used_vectors )
- {
- printk(XENLOG_INFO "%s: Strange, unassigned irq %d already has used_vectors!\n",
- __func__, irq);
- }
+ printk(XENLOG_INFO "Unassigned IRQ %d already has used_vectors\n",
+ irq);
else
{
int vector;
@@ -415,8 +413,8 @@ static vmask_t *irq_get_used_vector_mask
vector = irq_to_vector(irq);
if ( vector > 0 )
{
- printk(XENLOG_INFO "%s: Strange, irq %d already assigned vector %d!\n",
- __func__, irq, vector);
+ printk(XENLOG_INFO "IRQ %d already assigned vector %d\n",
+ irq, vector);
ASSERT(!test_bit(vector, ret));
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -164,10 +164,7 @@ void msi_compose_msg(unsigned vector, co
memset(msg, 0, sizeof(*msg));
if ( !cpumask_intersects(cpu_mask, &cpu_online_map) )
- {
- dprintk(XENLOG_ERR,"%s, compose msi message error!!\n", __func__);
return;
- }
if ( vector )
{
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1467,8 +1467,7 @@ static int __init verify_tsc_reliability
tsc_check_reliability();
if ( tsc_max_warp )
{
- printk("%s: TSC warp detected, disabling TSC_RELIABLE\n",
- __func__);
+ printk("TSC warp detected, disabling TSC_RELIABLE\n");
setup_clear_cpu_cap(X86_FEATURE_TSC_RELIABLE);
}
}
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -574,8 +574,8 @@ void xstate_init(struct cpuinfo_x86 *c)
* We know FP/SSE and YMM about eax, and nothing about edx at present.
*/
xsave_cntxt_size = _xstate_ctxt_size(feature_mask);
- printk("%s: using cntxt_size: %#x and states: %#"PRIx64"\n",
- __func__, xsave_cntxt_size, xfeature_mask);
+ printk("xstate: size: %#x and states: %#"PRIx64"\n",
+ xsave_cntxt_size, xfeature_mask);
asm ( "fxsave %0" : "=m" (ctxt) );
if ( ctxt.mxcsr_mask )
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: drop pointless uses of __func__ / __FUNCTION__
2016-08-24 8:02 [PATCH] x86: drop pointless uses of __func__ / __FUNCTION__ Jan Beulich
@ 2016-08-24 20:34 ` Konrad Rzeszutek Wilk
2016-08-25 6:48 ` Jan Beulich
2016-08-31 18:43 ` Andrew Cooper
1 sibling, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-08-24 20:34 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Andrew Cooper
On Wed, Aug 24, 2016 at 02:02:51AM -0600, Jan Beulich wrote:
> Non-debugging message text should be (and is in the cases here)
> distinguishable without also logging function names. Debugging message
> text, otoh, already includes file name and line number, so also
> logging function names is redundant. One relatively pointless debugging
> message gets removed altogether. In another case a mising log level
s/mising/missing/
> specifier gets added at once.
Which one? I was thinking it would be either one of these:
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1467,8 +1467,7 @@ static int __init verify_tsc_reliability
> tsc_check_reliability();
> if ( tsc_max_warp )
> {
> - printk("%s: TSC warp detected, disabling TSC_RELIABLE\n",
> - __func__);
> + printk("TSC warp detected, disabling TSC_RELIABLE\n");
> setup_clear_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> }
> }
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -574,8 +574,8 @@ void xstate_init(struct cpuinfo_x86 *c)
> * We know FP/SSE and YMM about eax, and nothing about edx at present.
> */
> xsave_cntxt_size = _xstate_ctxt_size(feature_mask);
> - printk("%s: using cntxt_size: %#x and states: %#"PRIx64"\n",
> - __func__, xsave_cntxt_size, xfeature_mask);
> + printk("xstate: size: %#x and states: %#"PRIx64"\n",
> + xsave_cntxt_size, xfeature_mask);
>
> asm ( "fxsave %0" : "=m" (ctxt) );
> if ( ctxt.mxcsr_mask )
>
>
but both just remove the __func__ usage?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: drop pointless uses of __func__ / __FUNCTION__
2016-08-24 20:34 ` Konrad Rzeszutek Wilk
@ 2016-08-25 6:48 ` Jan Beulich
0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2016-08-25 6:48 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: Andrew Cooper, xen-devel
>>> On 24.08.16 at 22:34, <konrad.wilk@oracle.com> wrote:
> On Wed, Aug 24, 2016 at 02:02:51AM -0600, Jan Beulich wrote:
>> Non-debugging message text should be (and is in the cases here)
>> distinguishable without also logging function names. Debugging message
>> text, otoh, already includes file name and line number, so also
>> logging function names is redundant. One relatively pointless debugging
>> message gets removed altogether. In another case a mising log level
>
> s/mising/missing/
>> specifier gets added at once.
>
> Which one? I was thinking it would be either one of these:
>
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -1467,8 +1467,7 @@ static int __init verify_tsc_reliability
>> tsc_check_reliability();
>> if ( tsc_max_warp )
>> {
>> - printk("%s: TSC warp detected, disabling TSC_RELIABLE\n",
>> - __func__);
>> + printk("TSC warp detected, disabling TSC_RELIABLE\n");
>> setup_clear_cpu_cap(X86_FEATURE_TSC_RELIABLE);
>> }
>> }
>> --- a/xen/arch/x86/xstate.c
>> +++ b/xen/arch/x86/xstate.c
>> @@ -574,8 +574,8 @@ void xstate_init(struct cpuinfo_x86 *c)
>> * We know FP/SSE and YMM about eax, and nothing about edx at present.
>> */
>> xsave_cntxt_size = _xstate_ctxt_size(feature_mask);
>> - printk("%s: using cntxt_size: %#x and states: %#"PRIx64"\n",
>> - __func__, xsave_cntxt_size, xfeature_mask);
>> + printk("xstate: size: %#x and states: %#"PRIx64"\n",
>> + xsave_cntxt_size, xfeature_mask);
>>
>> asm ( "fxsave %0" : "=m" (ctxt) );
>> if ( ctxt.mxcsr_mask )
>>
>>
>
> but both just remove the __func__ usage?
No, these are (mostly) fine (they could have one added, but there
not being one is not an immediate problem). The respective hunk is
@@ -2385,9 +2383,9 @@ int ioapic_guest_write(unsigned long phy
spin_unlock_irqrestore(&ioapic_lock, flags);
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!",
- ret, pirq, __FUNCTION__);
+ WARN_BOGUS_WRITE("old_entry=%08x pirq=%d\n" XENLOG_INFO
+ "IO-APIC: Attempt to modify IO-APIC pin for in-use IRQ!",
+ ret, pirq);
return 0;
}
where a sequence of XENLOG_INFO messages (due to there
being multiple line breaks) is followed one without log level.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: drop pointless uses of __func__ / __FUNCTION__
2016-08-24 8:02 [PATCH] x86: drop pointless uses of __func__ / __FUNCTION__ Jan Beulich
2016-08-24 20:34 ` Konrad Rzeszutek Wilk
@ 2016-08-31 18:43 ` Andrew Cooper
2016-09-01 7:17 ` Jan Beulich
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2016-08-31 18:43 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 24/08/16 09:02, Jan Beulich wrote:
> Non-debugging message text should be (and is in the cases here)
> distinguishable without also logging function names. Debugging message
> text, otoh, already includes file name and line number, so also
> logging function names is redundant. One relatively pointless debugging
> message gets removed altogether. In another case a mising log level
> specifier gets added at once.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -2307,14 +2307,12 @@ int ioapic_guest_read(unsigned long phys
> return 0;
> }
>
> -#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 )
> +#define WARN_BOGUS_WRITE(f, a...) \
> + dprintk(XENLOG_INFO, "\n" \
I would be tempted to get rid of this stray newline entirely. It serves
no useful purpose.
Alternatively, the literal "BOGUS WRITE\n" would be an improvement.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: drop pointless uses of __func__ / __FUNCTION__
2016-08-31 18:43 ` Andrew Cooper
@ 2016-09-01 7:17 ` Jan Beulich
2016-09-01 9:52 ` Andrew Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2016-09-01 7:17 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
>>> On 31.08.16 at 20:43, <andrew.cooper3@citrix.com> wrote:
> On 24/08/16 09:02, Jan Beulich wrote:
>> Non-debugging message text should be (and is in the cases here)
>> distinguishable without also logging function names. Debugging message
>> text, otoh, already includes file name and line number, so also
>> logging function names is redundant. One relatively pointless debugging
>> message gets removed altogether. In another case a mising log level
>> specifier gets added at once.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -2307,14 +2307,12 @@ int ioapic_guest_read(unsigned long phys
>> return 0;
>> }
>>
>> -#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 )
>> +#define WARN_BOGUS_WRITE(f, a...) \
>> + dprintk(XENLOG_INFO, "\n" \
>
> I would be tempted to get rid of this stray newline entirely. It serves
> no useful purpose.
In fact I did consider this, but then put it off as unrelated. But now
that you ask for it - done. Should I resend?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: drop pointless uses of __func__ / __FUNCTION__
2016-09-01 7:17 ` Jan Beulich
@ 2016-09-01 9:52 ` Andrew Cooper
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2016-09-01 9:52 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On 01/09/16 08:17, Jan Beulich wrote:
>>>> On 31.08.16 at 20:43, <andrew.cooper3@citrix.com> wrote:
>> On 24/08/16 09:02, Jan Beulich wrote:
>>> Non-debugging message text should be (and is in the cases here)
>>> distinguishable without also logging function names. Debugging message
>>> text, otoh, already includes file name and line number, so also
>>> logging function names is redundant. One relatively pointless debugging
>>> message gets removed altogether. In another case a mising log level
>>> specifier gets added at once.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> --- a/xen/arch/x86/io_apic.c
>>> +++ b/xen/arch/x86/io_apic.c
>>> @@ -2307,14 +2307,12 @@ int ioapic_guest_read(unsigned long phys
>>> return 0;
>>> }
>>>
>>> -#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 )
>>> +#define WARN_BOGUS_WRITE(f, a...) \
>>> + dprintk(XENLOG_INFO, "\n" \
>> I would be tempted to get rid of this stray newline entirely. It serves
>> no useful purpose.
> In fact I did consider this, but then put it off as unrelated. But now
> that you ask for it - done. Should I resend?
Not worth a resend.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-01 9:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-24 8:02 [PATCH] x86: drop pointless uses of __func__ / __FUNCTION__ Jan Beulich
2016-08-24 20:34 ` Konrad Rzeszutek Wilk
2016-08-25 6:48 ` Jan Beulich
2016-08-31 18:43 ` Andrew Cooper
2016-09-01 7:17 ` Jan Beulich
2016-09-01 9:52 ` Andrew Cooper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).