FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Debian > Debian Kernel

 
 
LinkBack Thread Tools
 
Old 12-17-2011, 07:09 PM
Uwe Kleine-König
 
Default m68k/irq: don't use pr_crit in an header

Using pr_crit in an header results in funny messages. Consider

#define pr_fmt(fmt) "mydriver: " fmt
#include <linux/hardirq.h>

which makes the message from ack_bad_irq

mydriver: unexpected IRQ trap...

so better use plain printk with KERN_CRIT directly.

This fixes a build problem on m68k with aufs3 en passant because the
latter builds with

ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"40%s:%d:%s[%d]:40"fmt,__func__,__LINE__,current->comm,current->pid'

without providing AUFS_NAME early enough for ack_bad_irq (which is the
problem of aufs).

Cc: Thorsten Glaser <tg@debian.org>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/m68k/include/asm/hardirq.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/m68k/include/asm/hardirq.h b/arch/m68k/include/asm/hardirq.h
index db30ed2..1f652e0 100644
--- a/arch/m68k/include/asm/hardirq.h
+++ b/arch/m68k/include/asm/hardirq.h
@@ -20,7 +20,7 @@

static inline void ack_bad_irq(unsigned int irq)
{
- pr_crit("unexpected IRQ trap at vector %02x
", irq);
+ printk(KERN_CRIT "unexpected IRQ trap at vector %02x
", irq);
}

/* entry.S is sensitive to the offsets of these fields */
--
1.7.7.3


--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 1324152592-1718-1-git-send-email-u.kleine-koenig@pengutronix.de">http://lists.debian.org/1324152592-1718-1-git-send-email-u.kleine-koenig@pengutronix.de
 
Old 12-17-2011, 08:19 PM
Thorsten Glaser
 
Default m68k/irq: don't use pr_crit in an header

Uwe Kleine-König dixit:

>so better use plain printk with KERN_CRIT directly.

Wasn’t it considered good style to switch f̲r̲o̲m̲ that t̲o̲ pr_crit?

In that case, my other patch from Message-ID
<Pine.BSM.4.64L.1112171426140.856@herc.mirbsd.or g> can still
be used. Feel free to assume a Signed-off on that by me, if
this is the desired direction of change.

(Of course, aufs changes notwithstanding.)

bye,
//mirabilos
--
Support mksh as /bin/sh and RoQA dash NOW!
‣ src:bash (240 (258) bugs: 0 RC, 167 (181) I&N, 73 (77) M&W, 0 F&P)
‣ src:dash (72 (82) bugs: 3 RC, 27 (30) I&N, 42 (49) M&W, 0 F&P)
‣ src:mksh (1 bug: 0 RC, 0 I&N, 1 M&W, 0 F&P)
http://qa.debian.org/data/bts/graphs/d/dash.png is pretty red, innit?


--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: Pine.BSM.4.64L.1112172117060.856@herc.mirbsd.org"> http://lists.debian.org/Pine.BSM.4.64L.1112172117060.856@herc.mirbsd.org
 
Old 12-18-2011, 09:32 AM
Geert Uytterhoeven
 
Default m68k/irq: don't use pr_crit in an header

2011/12/17 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> Using pr_crit in an header results in funny messages. Consider
>
> * * * *#define pr_fmt(fmt) "mydriver: " fmt
> * * * *#include <linux/hardirq.h>
>
> which makes the message from ack_bad_irq
>
> * * * *mydriver: unexpected IRQ trap...
>
> so better use plain printk with KERN_CRIT directly.

Yep, that's expected behavior, as defining pr_fmt() modifies all kernel messages
generated from that module.

> This fixes a build problem on m68k with aufs3 en passant because the
> latter builds with
>
> * * * *ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"40%s:%d:%s[%d]:40"fmt,__func__,__LINE__,current->comm,current->pid'
>
> without providing AUFS_NAME early enough for ack_bad_irq (which is the
> problem of aufs).

Isn't this a problem with (out of tree) aufs?
Why does it put a define that relies on an (apparently sometimes still
undefined)
variable on the build command line?
Any header may contain calls to pr_*().

> Cc: Thorsten Glaser <tg@debian.org>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> *arch/m68k/include/asm/hardirq.h | * *2 +-
> *1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/m68k/include/asm/hardirq.h b/arch/m68k/include/asm/hardirq.h
> index db30ed2..1f652e0 100644
> --- a/arch/m68k/include/asm/hardirq.h
> +++ b/arch/m68k/include/asm/hardirq.h
> @@ -20,7 +20,7 @@
>
> *static inline void ack_bad_irq(unsigned int irq)
> *{
> - * * * pr_crit("unexpected IRQ trap at vector %02x
", irq);
> + * * * printk(KERN_CRIT "unexpected IRQ trap at vector %02x
", irq);

Nack. Nowadays pr_crit(...) is recommended over "printk(KERN_CRIT ...)".

Besides, there are (albeit not that many yet) other callers of pr_*() in
header files. Do you plan to revert them to printk(), too?

Please fix aufs instead. Thanks!

Gr{oetje,eeting}s,

* * * * * * * * * * * * Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
* * * * * * * * * * * * * ** ** -- Linus Torvalds


--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: CAMuHMdXyf_ofM+y6yKrzftyKTOyrSKzW7fUVAJyY6XTV_FVhN A@mail.gmail.com">http://lists.debian.org/CAMuHMdXyf_ofM+y6yKrzftyKTOyrSKzW7fUVAJyY6XTV_FVhN A@mail.gmail.com
 
Old 12-18-2011, 09:42 AM
Uwe Kleine-König
 
Default m68k/irq: don't use pr_crit in an header

Hi Geert,

(thanks for adding Joe to Cc:, I noticed that when I wanted to add him
myself :-)

On Sun, Dec 18, 2011 at 11:32:21AM +0100, Geert Uytterhoeven wrote:
> 2011/12/17 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > Using pr_crit in an header results in funny messages. Consider
> >
> > * * * *#define pr_fmt(fmt) "mydriver: " fmt
> > * * * *#include <linux/hardirq.h>
> >
> > which makes the message from ack_bad_irq
> >
> > * * * *mydriver: unexpected IRQ trap...
> >
> > so better use plain printk with KERN_CRIT directly.
>
> Yep, that's expected behavior, as defining pr_fmt() modifies all kernel messages
> generated from that module.
I'm aware it is expected, I only wondered if it is also desirable to
have messages in headers modified depending on the module the header is
included in.

> > This fixes a build problem on m68k with aufs3 en passant because the
> > latter builds with
> >
> > * * * *ccflags-y += -D'pr_fmt(fmt)=AUFS_NAME"40%s:%d:%s[%d]:40"fmt,__func__,__LINE__,current->comm,current->pid'
> >
> > without providing AUFS_NAME early enough for ack_bad_irq (which is the
> > problem of aufs).
>
> Isn't this a problem with (out of tree) aufs?
> Why does it put a define that relies on an (apparently sometimes still
> undefined)
> variable on the build command line?
This is definitily a bug in aufs that needs fixing independant of the
issue of using or not using pr_... in headers.

> Any header may contain calls to pr_*().
>
> > Cc: Thorsten Glaser <tg@debian.org>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > *arch/m68k/include/asm/hardirq.h | * *2 +-
> > *1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/m68k/include/asm/hardirq.h b/arch/m68k/include/asm/hardirq.h
> > index db30ed2..1f652e0 100644
> > --- a/arch/m68k/include/asm/hardirq.h
> > +++ b/arch/m68k/include/asm/hardirq.h
> > @@ -20,7 +20,7 @@
> >
> > *static inline void ack_bad_irq(unsigned int irq)
> > *{
> > - * * * pr_crit("unexpected IRQ trap at vector %02x
", irq);
> > + * * * printk(KERN_CRIT "unexpected IRQ trap at vector %02x
", irq);
>
> Nack. Nowadays pr_crit(...) is recommended over "printk(KERN_CRIT ...)".
I know that, I just wonder if the proponents of this recommendation are
aware of the issue when using pr_* in headers. Joe?

> Besides, there are (albeit not that many yet) other callers of pr_*() in
> header files. Do you plan to revert them to printk(), too?
That depends on the outcome of this discussion.

> Please fix aufs instead. Thanks!
I already provided a patch for that, too. (Currently only on the Debian
kernel ML.)

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |


--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20111218104248.GS24496@pengutronix.de">http://lists.debian.org/20111218104248.GS24496@pengutronix.de
 
Old 12-18-2011, 04:06 PM
Joe Perches
 
Default m68k/irq: don't use pr_crit in an header

On Sun, 2011-12-18 at 11:42 +0100, Uwe Kleine-König wrote:
> On Sun, Dec 18, 2011 at 11:32:21AM +0100, Geert Uytterhoeven wrote:
> > 2011/12/17 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > > Using pr_crit in an header results in funny messages. Consider
> > > #define pr_fmt(fmt) "mydriver: " fmt
> > > #include <linux/hardirq.h>
> > > which makes the message from ack_bad_irq
> > > mydriver: unexpected IRQ trap...
> > > so better use plain printk with KERN_CRIT directly.

Why or when is that inappropriate?

> I only wondered if it is also desirable to
> have messages in headers modified depending on the module the header is
> included in.
[]
> > Nack. Nowadays pr_crit(...) is recommended over "printk(KERN_CRIT ...)".
> I know that, I just wonder if the proponents of this recommendation are
> aware of the issue when using pr_* in headers. Joe?

I believe it to be a feature rather than a defect.

For instance: commit 256ee435b9a9ee9cca69602fe8046b27ca99fbee

netdevice: Convert printk to pr_info in netif_tx_stop_queue

This allows any caller to be prefaced by any specific
pr_fmt to better identify which device driver is using
this function inappropriately.

cheers, Joe


--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 1324228007.14500.4.camel@joe2Laptop">http://lists.debian.org/1324228007.14500.4.camel@joe2Laptop
 

Thread Tools




All times are GMT. The time now is 12:42 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright ©2007 - 2008, www.linux-archive.org