Re: [PATCH v3] Make the pr_*() family of macros in kernel.h complete
On Fri, 14 Sep 2007 06:27:04 -0700 "Medve Emilian-EMMEDVE1" <[EMAIL PROTECTED]> wrote: > I realize this e-mail might be nuisance and time waster for you but I'm > in need of advice. I apologize in advance for any commonsense cultural > conventions I'm breaking. > > I sent the below patch to four e-mail lists and it lead to orthogonal > conversations about how the entire kernel logging system/mechanisms need > to be re-written and thus such incremental improvements as these get out > of focus... > > In this case I started needing pr_err() and discovered that is defined > already four times but not with global visibility as some other pr_*() > from kernel.h (a subset of the entire family). I chose not to define it > yet the fifth time but clean up the existing definitions and complete > the family. For some reason it didn't go through even though I had some > positive feedback. Now it seems I'm encouraged to really define the > pr_err() for the fifth time... Not quite sure what to do... I normally troll the lkml list for patches like this and will sweep them up. But I'm presently 1400 messages in arrears so there is some latency. I'll go grab this patch. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] Make the pr_*() family of macros in kernel.h complete
On Fri, 14 Sep 2007 22:11:59 +0530 Satyam Sharma wrote: > > > -Original Message- > > > From: Medve Emilian-EMMEDVE1 > > > Sent: Wednesday, September 12, 2007 11:40 AM > > > To: linux-kernel@vger.kernel.org; [EMAIL PROTECTED]; > > > [EMAIL PROTECTED]; [EMAIL PROTECTED] > > > Cc: Medve Emilian-EMMEDVE1 > > ... no maintainer explicitly copied on the original mail who'd push this in. > Little thing, but LKML is too noisy, so you need to Cc: folks explicitly ... > > > > Subject: [PATCH v3] Make the pr_*() family of macros in > > > kernel.h complete > > > > > > Other/Some pr_*() macros are already defined in kernel.h, but > > > pr_err() was defined > > > multiple times in several other places > > > > > > Signed-off-by: Emil Medve <[EMAIL PROTECTED]> > > I think it's a useful cleanup, patch looks good to me ... > > Reviewed-by: Satyam Sharma <[EMAIL PROTECTED]> I like it too. Acked-by: Randy Dunlap <[EMAIL PROTECTED]> (since I still haven't seen reviewed-by in SubmittingPatches) > [ Original diff is at: http://lkml.org/lkml/diff/2007/9/12/182/1 ] --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] Make the pr_*() family of macros in kernel.h complete
> > -Original Message- > > From: Medve Emilian-EMMEDVE1 > > Sent: Wednesday, September 12, 2007 11:40 AM > > To: linux-kernel@vger.kernel.org; [EMAIL PROTECTED]; > > [EMAIL PROTECTED]; [EMAIL PROTECTED] > > Cc: Medve Emilian-EMMEDVE1 ... no maintainer explicitly copied on the original mail who'd push this in. Little thing, but LKML is too noisy, so you need to Cc: folks explicitly ... > > Subject: [PATCH v3] Make the pr_*() family of macros in > > kernel.h complete > > > > Other/Some pr_*() macros are already defined in kernel.h, but > > pr_err() was defined > > multiple times in several other places > > > > Signed-off-by: Emil Medve <[EMAIL PROTECTED]> I think it's a useful cleanup, patch looks good to me ... Reviewed-by: Satyam Sharma <[EMAIL PROTECTED]> [ Original diff is at: http://lkml.org/lkml/diff/2007/9/12/182/1 ] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3] Make the pr_*() family of macros in kernel.h complete
Hello Andrew,
I realize this e-mail might be nuisance and time waster for you but I'm
in need of advice. I apologize in advance for any commonsense cultural
conventions I'm breaking.
I sent the below patch to four e-mail lists and it lead to orthogonal
conversations about how the entire kernel logging system/mechanisms need
to be re-written and thus such incremental improvements as these get out
of focus...
In this case I started needing pr_err() and discovered that is defined
already four times but not with global visibility as some other pr_*()
from kernel.h (a subset of the entire family). I chose not to define it
yet the fifth time but clean up the existing definitions and complete
the family. For some reason it didn't go through even though I had some
positive feedback. Now it seems I'm encouraged to really define the
pr_err() for the fifth time... Not quite sure what to do...
Cheers,
Emil.
> -Original Message-
> From: Medve Emilian-EMMEDVE1
> Sent: Wednesday, September 12, 2007 11:40 AM
> To: linux-kernel@vger.kernel.org; [EMAIL PROTECTED];
> [EMAIL PROTECTED]; [EMAIL PROTECTED]
> Cc: Medve Emilian-EMMEDVE1
> Subject: [PATCH v3] Make the pr_*() family of macros in
> kernel.h complete
>
> Other/Some pr_*() macros are already defined in kernel.h, but
> pr_err() was defined
> multiple times in several other places
>
> Signed-off-by: Emil Medve <[EMAIL PROTECTED]>
> ---
>
> I'm writing a driver and I've been using the pr_*() macros
> from kernel.h and I
> was surprised not to find there pr_err() but defined multiple
> times (in four
> different files). I didn't want to define it yet one more
> time so I did this
> cleanup
>
> As per community request/suggestion, I added the rest of the
> missing pr_*()
> macros to complete the family. The names of the macros are
> based on the KERN_*
> loglevel names and are macthing the naming convention of the
> dev_*() print
> macros from device.h. The macros are defined in the ascending
> order of the
> loglevel
>
> This patch is against Linus' tree
> (577107e8e4cf9f6f4f5ef8350ac9a8faa6c3796d)
>
> linux-2.6> scripts/checkpatch.pl
> 0001-Make-the-pr_-family-of-macros-in-kernel.h-complet.patch
> Your patch has no obvious style problems and is ready for submission.
>
> drivers/i2c/chips/menelaus.c | 10 --
> drivers/net/spider_net.h |3 ---
> drivers/video/omap/lcd_h3.c |6 ++
> drivers/video/omap/lcd_inn1610.c |6 ++
> include/linux/kernel.h | 22 +-
> 5 files changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/i2c/chips/menelaus.c
> b/drivers/i2c/chips/menelaus.c
> index d9c92c5..66436ba 100644
> --- a/drivers/i2c/chips/menelaus.c
> +++ b/drivers/i2c/chips/menelaus.c
> @@ -49,8 +49,6 @@
>
> #define DRIVER_NAME "menelaus"
>
> -#define pr_err(fmt, arg...) printk(KERN_ERR DRIVER_NAME ":
> ", ## arg);
> -
> #define MENELAUS_I2C_ADDRESS 0x72
>
> #define MENELAUS_REV 0x01
> @@ -155,7 +153,7 @@ static int menelaus_write_reg(int reg, u8 value)
> int val =
> i2c_smbus_write_byte_data(the_menelaus->client, reg, value);
>
> if (val < 0) {
> - pr_err("write error");
> + pr_err(DRIVER_NAME ": write error");
> return val;
> }
>
> @@ -167,7 +165,7 @@ static int menelaus_read_reg(int reg)
> int val = i2c_smbus_read_byte_data(the_menelaus->client, reg);
>
> if (val < 0)
> - pr_err("read error");
> + pr_err(DRIVER_NAME ": read error");
>
> return val;
> }
> @@ -1177,7 +1175,7 @@ static int menelaus_probe(struct
> i2c_client *client)
> /* If a true probe check the device */
> rev = menelaus_read_reg(MENELAUS_REV);
> if (rev < 0) {
> - pr_err("device not found");
> + pr_err(DRIVER_NAME ": device not found");
> err = -ENODEV;
> goto fail1;
> }
> @@ -1258,7 +1256,7 @@ static int __init menelaus_init(void)
>
> res = i2c_add_driver(&menelaus_i2c_driver);
> if (res < 0) {
> - pr_err("driver registration failed\n");
> + pr_err(DRIVER_NAME ": driver registration failed\n");
> return res;
> }
>
> diff --git a/drivers/net/spider_net.h b/drivers/net/spider_net.h
> index dbbdb8c..c67b11d 100644
> --- a/drivers/net/spider_net.h
> +++ b/drivers/net/spider_net.h
> @@ -493,7 +493,4 @@ struct spider_net_card {
> struct spider_net_descr darray[0];
> };
>
> -#define pr_err(fmt,arg...) \
> - printk(KERN_ERR fmt ,##arg)
> -
> #endif
> diff --git a/drivers/video/omap/lcd_h3.c b/drivers/video/omap/lcd_h3.c
> index 51807b4..c604d93 100644
> --- a/drivers/video/omap/lcd_h3.c
> +++ b/drivers/video/omap/lcd_h3.c
> @@ -28,8 +28,6 @@
>
> #define MODULE_NAME "omapfb-lcd_h3"
>
> -#define pr_err(fmt, args...) printk(KERN_ERR MODULE_NAME ":
> " fmt, ## args)
> -
> static i
RE: [PATCH v3] Make the pr_*() family of macros in kernel.h complete
On Wed, 2007-09-12 at 11:44 -0700, Medve Emilian-EMMEDVE1 wrote: > First, this patch doesn't have the trailing "\n" problem that one had. I expect all the kernel logging functions to be overhauled eventually. I'd prefer a mechanism that somehow supports identifying complete messages. I think the new pr_ functions are not particularly useful without a mechanism to avoid or identify multiple processors or threads interleaving partial in-progress multiple statement messages. I've got a very large patch series that converts _all_ the current single line messages that use KERN_ to pr_ and identifies, prefixes and postfixes the rest of the multiple source line messages. At some point, sooner or later, the logging functions will be improved. Apparently, more likely later. cheers, Joe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3] Make the pr_*() family of macros in kernel.h complete
> -Original Message- > From: Jan Engelhardt [mailto:[EMAIL PROTECTED] > Sent: Wednesday, September 12, 2007 1:04 PM > To: Medve Emilian-EMMEDVE1 > Cc: linux-kernel@vger.kernel.org; [EMAIL PROTECTED]; > [EMAIL PROTECTED]; [EMAIL PROTECTED] > Subject: Re: [PATCH v3] Make the pr_*() family of macros in > kernel.h complete > > > On Sep 12 2007 11:39, Emil Medve wrote: > > > >Other/Some pr_*() macros are already defined in kernel.h, > but pr_err() was defined > >multiple times in several other places > > Note http://lkml.org/lkml/2007/8/4/30 . Hi Jan, I didn't see that thread before I submitted the patch... I don't want to start the conversation from the top 'cause this is not a patch that fixes some existing showstopper... however, I have two comments to make. First, this patch doesn't have the trailing "\n" problem that one had. Second, it seems that multiple people at different moments in time have the same idea to complete and, more important, to use the pr_*() family of macros because I guess it makes it easier to use them. In addition, it would make it less likely for people to forget using the loglevel in their kernel messages. Is trying to add them a dead path? Thing is that the existing ones suggest to, more or less, new people to the kernel that they should use them and then discover the incompleteness/inconsistency... Cheers, Emil. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] Make the pr_*() family of macros in kernel.h complete
On Sep 12 2007 11:39, Emil Medve wrote: > >Other/Some pr_*() macros are already defined in kernel.h, but pr_err() was >defined >multiple times in several other places Note http://lkml.org/lkml/2007/8/4/30 . - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
