kmsg macros and script, take x+1.

From: Michael Holzheu <[email protected]>
From: Martin Schwidefsky <[email protected]>

Introduce a new family of printk macros which prefixes each kmsg message with a component name and allows to tag the printk with a message id.

The kmsg component name is defined per source file with the KMSG_COMPONENT macro. The first argument of each kmsg printk is the message id. The message id "0" is special as it will suppress the message id prefix.

If the message id will be printed to the console / syslog at all depends on CONFIG_MSG_IDS. If it is "n" then a kmsg_xxx call is just another printk wrapper. These macros are intended to be used uniformly in the s390 architecture and the s390 device drivers.

Signed-off-by Michael Holzheu <[email protected]>

Signed-off-by Martin Schwidefsky <[email protected]>

---

 arch/s390/Kconfig    |    9 +++
 include/linux/kmsg.h |  124 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 133 insertions(+)

Index: linux-2.6/arch/s390/Kconfig
===================================================================

arch/s390/Kconfig +9 -0

@@ -568,6 +568,15 @@ bool "s390 guest support (EXPERIMENTAL)"
 	select VIRTIO_CONSOLE
 	help
 	  Select this option if you want to run the kernel under s390 linux
+
+config KMSG_IDS
+	bool "Kernel message numbers"
+	default y
+	help
+	  Select this option if you want to include a message number to the
+	  prefix for kernel messages issued by the s390 architecture and
+	  driver code. See "Documentation/s390/kmsg.txt" for more details.
+
 endmenu
 
 source "net/Kconfig"

Index: linux-2.6/include/linux/kmsg.h
===================================================================

include/linux/kmsg.h line 6

Quoted 11 lines

Index: linux-2.6/include/linux/kmsg.h
===================================================================

include/linux/kmsg.h new file +6 -0

@@ -0,0 +1,124 @@
+#ifndef _LINUX_KMSG_H
+#define _LINUX_KMSG_H
+
+#ifndef __KMSG_CHECKER
+#define __KMSG_CHECK(level, id) KERN_##level
+#endif

What if __KMSG_CHECKER is enabled? What does __KMSG_CHECK resolve to then?

And what sets __KMSG_CHECKER?

include/linux/kmsg.h line 12

Quoted 3 lines · Martin Schwidefsky

include/linux/kmsg.h new file +3 -0

+#define kmsg_dev_alert(id, dev, format, arg...) \
+       printk(__KMSG_CHECK(ALERT, id) KMSG_COMPONENT \
+               ": %s: " format, (dev)->bus_id , ## arg)

Care to use dev_name(), bus_id will be removed soon.

Quoted 3 lines · Kay Sievers

On Wed, Jul 30, 2008 at 18:56, Martin Schwidefsky
<[email protected]> wrote:

Quoted 5 lines

include/linux/kmsg.h line 12

Quoted 3 lines

include/linux/kmsg.h new file +3 -0

+#define kmsg_dev_alert(id, dev, format, arg...) \
+       printk(__KMSG_CHECK(ALERT, id) KMSG_COMPONENT \
+               ": %s: " format, (dev)->bus_id , ## arg)

Care to use dev_name(), bus_id will be removed soon.

Hm, just use dev_printk() instead, it handles all of these kinds of changes automatically.

Quoted 4 lines · Greg KH

Quoted 3 lines · Kay Sievers

On Wed, Jul 30, 2008 at 18:56, Martin Schwidefsky
<[email protected]> wrote:

Quoted 8 lines

Quoted 5 lines

include/linux/kmsg.h line 12

Quoted 3 lines

include/linux/kmsg.h new file +3 -0

+#define kmsg_dev_alert(id, dev, format, arg...) \
+       printk(__KMSG_CHECK(ALERT, id) KMSG_COMPONENT \
+               ": %s: " format, (dev)->bus_id , ## arg)

Care to use dev_name(), bus_id will be removed soon.

Hm, just use dev_printk() instead, it handles all of these kinds of changes automatically.

Using dev_printk won't work because of the order of the elements of the printk. The kmsg tag should not have a "random" position in the printk but should be the first element. If we use dev_printk the kmsg tag will be the third element, for other kmsg printks it will be the first. In addition the kmsg message tag for the device drivers already includes the driver name ..

Quoted 14 lines · Martin Schwidefsky

Quoted 4 lines · Greg KH

Quoted 3 lines · Kay Sievers

On Wed, Jul 30, 2008 at 18:56, Martin Schwidefsky
<[email protected]> wrote:

Quoted 8 lines

Quoted 5 lines

include/linux/kmsg.h line 12

Quoted 3 lines

include/linux/kmsg.h new file +3 -0

+#define kmsg_dev_alert(id, dev, format, arg...) \
+       printk(__KMSG_CHECK(ALERT, id) KMSG_COMPONENT \
+               ": %s: " format, (dev)->bus_id , ## arg)

Care to use dev_name(), bus_id will be removed soon.

Hm, just use dev_printk() instead, it handles all of these kinds of changes automatically.

Quoted 6 lines

Using dev_printk won't work because of the order of the elements of the printk. The kmsg tag should not have a "random" position in the printk but should be the first element. If we use dev_printk the kmsg tag will be the third element, for other kmsg printks it will be the first. In addition the kmsg message tag for the device drivers already includes the driver name ..

But the structure of dev_printk() is well definied and should be pretty trival to parse even with missing fields.

And if not, fix up dev_printk() to properly delinate when we have missing fields, why create a totally new thing here that we need to convert subsystems into?

Quoted 22 lines · Greg KH

Quoted 20 lines · Martin Schwidefsky

Quoted 4 lines · Greg KH

Quoted 3 lines · Kay Sievers

On Wed, Jul 30, 2008 at 18:56, Martin Schwidefsky
<[email protected]> wrote:

Quoted 8 lines

Quoted 5 lines

include/linux/kmsg.h line 12

Quoted 3 lines

include/linux/kmsg.h new file +3 -0

+#define kmsg_dev_alert(id, dev, format, arg...) \
+       printk(__KMSG_CHECK(ALERT, id) KMSG_COMPONENT \
+               ": %s: " format, (dev)->bus_id , ## arg)

Care to use dev_name(), bus_id will be removed soon.

Hm, just use dev_printk() instead, it handles all of these kinds of changes automatically.

Using dev_printk won't work because of the order of the elements of the printk. The kmsg tag should not have a "random" position in the printk but should be the first element. If we use dev_printk the kmsg tag will be the third element, for other kmsg printks it will be the first. In addition the kmsg message tag for the device drivers already includes the driver name ..

Quoted 2 lines

But the structure of dev_printk() is well definied and should be pretty trival to parse even with missing fields.

The missing field is the message tag. Which is the key for the message lookup. As far as the kmsg catalog is concerned this is a pretty important field that may not be missing from the printk itself.

Quoted 10 lines

Quoted 9 lines

Quoted 6 lines

Using dev_printk won't work because of the order of the elements of the printk. The kmsg tag should not have a "random" position in the printk but should be the first element. If we use dev_printk the kmsg tag will be the third element, for other kmsg printks it will be the first. In addition the kmsg message tag for the device drivers already includes the driver name ..

But the structure of dev_printk() is well definied and should be pretty trival to parse even with missing fields.

Quoted 3 lines

The missing field is the message tag. Which is the key for the message lookup. As far as the kmsg catalog is concerned this is a pretty important field that may not be missing from the printk itself.

No, I mean use dev_printk() as the base for your logging macro. Add your message tag as the first field after the dev_printk() information.

Quoted 2 lines · Greg KH

No, I mean use dev_printk() as the base for your logging macro. Add your message tag as the first field after the dev_printk() information.

Hmm, you are proposing to introduce a second format for the kmsg messages to avoid the need for some more printk wrapper macros. To me it seems that this has two problems:
1) The message tag is for the user of the system. If it does not have a fixed position it gets confusing.

2) The message tag for a driver message usually already includes the driver name, the dev_printk will print it again. This is ugly and reduces the quality of the message.

Quoted 8 lines · Martin Schwidefsky

Quoted 2 lines · Greg KH

No, I mean use dev_printk() as the base for your logging macro. Add your message tag as the first field after the dev_printk() information.

Hmm, you are proposing to introduce a second format for the kmsg messages to avoid the need for some more printk wrapper macros. To me it seems that this has two problems:
1) The message tag is for the user of the system. If it does not have a fixed position it gets confusing.

I think the message tag today is not useful for a user.

It seems a barely useful maintainer value.

Perhaps the message tag allows for run-time language translation and maybe a bit better selective logging.

Quoted 7 lines · Joe Perches

Quoted 2 lines · Martin Schwidefsky

1) The message tag is for the user of the system. If it does not have a fixed position it gets confusing.

I think the message tag today is not useful for a user. It seems a barely useful maintainer value. Perhaps the message tag allows for run-time language translation and maybe a bit better selective logging.

Aehhm, .. what ? If a user does not understand a message directly how can he NOT be interested in the kmsg tag? As a random example consider the situation when the user gets the message

monwriter.1: Writing monitor data failed with rc=5

Usually a user will go "Huh?" except if he is a z/VM expert. Looking at the source code won't help too much either:

static int monwrite_diag(struct monwrite_hdr *myhdr, char *buffer, int fcn)

{
        struct appldata_product_id id;
        int rc;

        strcpy(id.prod_nr, "LNXAPPL");
        id.prod_fn = myhdr->applid;
        id.record_nr = myhdr->record_num;
        id.version_nr = myhdr->version;
        id.release_nr = myhdr->release;
        id.mod_lvl = myhdr->mod_level;
        rc = appldata_asm(&id, fcn, (void *) buffer, myhdr->datalen);
        if (rc <= 0)
                return rc;
        if (rc == 5)
                return -EPERM;
        kmsg_err(1,"Writing monitor data failed with rc=%i\n", rc);
        return -EINVAL;
}

What the user should do is to cut-copy-paste the kmsg tag monwriter.1 and pass it to man:

# man monwriter.1

What the user will then get is the man page that has been generated from the kmsg comment with "make D=2":

--- snip
monwriter.1(9) monwriter.1(9)

Message
monwriter.1: Writing monitor data failed with rc=%i

Severity
Error

Parameters
@1: return code

Description
The monitor stream application device driver used the z/VM diagnose call DIAG X'DC' to start writing monitor data. z/VM returned an error and the monitor data cannot be written. If the return code is 5, your z/VM guest virtual machine is not authorized to write monitor data.

User action
If the return code is 5, ensure that your z/VM guest virtual machine's entry in the z/VM directory includes the OPTION APPLMON statement. For other return codes see the section about DIAGNOSE Code X'DC' in "z/VM CP Programming Services".

Doesn't this example better illustrate the barely useful user value of such a message and documentation?

In your example, monwriter.1 with a return code of 5 isn't possible.

Quoted 14 lines · Greg KH

Quoted 13 lines

Quoted 9 lines

Quoted 6 lines

Using dev_printk won't work because of the order of the elements of the printk. The kmsg tag should not have a "random" position in the printk but should be the first element. If we use dev_printk the kmsg tag will be the third element, for other kmsg printks it will be the first. In addition the kmsg message tag for the device drivers already includes the driver name ..

But the structure of dev_printk() is well definied and should be pretty trival to parse even with missing fields.

The missing field is the message tag. Which is the key for the message lookup. As far as the kmsg catalog is concerned this is a pretty important field that may not be missing from the printk itself.

Quoted 2 lines

No, I mean use dev_printk() as the base for your logging macro. Add your message tag as the first field after the dev_printk() information.

Hmm, you are proposing to introduce a second format for the kmsg messages to avoid the need for some more printk wrapper macros. To me it seems that this has two problems:

1) The message tag is for the user of the system. If it does not have a fixed position it gets confusing.

2) The message tag for a driver message usually already includes the driver name, the dev_printk will print it again. This is ugly and reduces the quality of the message.

Quoted 15 lines · Martin Schwidefsky

Quoted 14 lines · Greg KH

Quoted 13 lines

Quoted 9 lines

Quoted 6 lines

Using dev_printk won't work because of the order of the elements of the printk. The kmsg tag should not have a "random" position in the printk but should be the first element. If we use dev_printk the kmsg tag will be the third element, for other kmsg printks it will be the first. In addition the kmsg message tag for the device drivers already includes the driver name ..

But the structure of dev_printk() is well definied and should be pretty trival to parse even with missing fields.

The missing field is the message tag. Which is the key for the message lookup. As far as the kmsg catalog is concerned this is a pretty important field that may not be missing from the printk itself.

Quoted 6 lines

Quoted 2 lines

No, I mean use dev_printk() as the base for your logging macro. Add your message tag as the first field after the dev_printk() information.

Hmm, you are proposing to introduce a second format for the kmsg messages to avoid the need for some more printk wrapper macros. To me it seems that this has two problems:

No, only 1 format, use dev_printk() instead of printk() in your macro.

Quoted 2 lines

1) The message tag is for the user of the system. If it does not have a fixed position it gets confusing.

How would it not be in a fixed position with dev_printk()?

Quoted 3 lines

2) The message tag for a driver message usually already includes the driver name, the dev_printk will print it again. This is ugly and reduces the quality of the message.

Then the message needs to change and remove that "driver name", as it is redundant, saving a tiny ammount of space :)

Then lets look at how this will look like. First a standard printk message and its conversion to kmsg:

	printk(KERN_WARNING
               "cpcmd: could not allocate response buffer\n");
vs.
	kmsg_warn(1, "The cpcmd kernel function failed "
                     "to allocate a response buffer\n");

The message comes out as
cpcmd: cound not allocate response buffer
vs.
cpcmd.1: The cpcmd kernel function failed to allocate a response buffer

As an example for a dev_printk I use a message from the zfcp driver:

	dev_warn(&req->adapter->ccw_device->dev,
                 "The local link is down: no light detected.\n");
vs.
        kmsg_dev_warn(27, &req->adapter->ccw_device->dev,
                 "The local link is down: no light detected.\n");

The dev_printk versus the original kmsg macro comes out as
zfcp: 0.0.1234: The local link is down: no light detected.
vs.
zfcp.27: 0.0.1234: The local link is down: no light detected.

If I would just use dev_printk in kmsg_dev_warn as proposed by Greg:
zfcp: 0.0.1234: zfcp.27: The local link is down: no light detected.

If the message component is skipped from the message tag:
zfcp: 0.0.1234: 27: The local link is down: no light detected.

Is it just me who thinks that the later two message variants are crap?

include/linux/kmsg.h line 12

Quoted 3 lines · Martin Schwidefsky

include/linux/kmsg.h new file +3 -0

+#define kmsg_dev_alert(id, dev, format, arg...) \
+       printk(__KMSG_CHECK(ALERT, id) KMSG_COMPONENT \
+               ": %s: " format, (dev)->bus_id , ## arg)

Care to use dev_name(), bus_id will be removed soon.

From: Michael Holzheu <[email protected]>
From: Martin Schwidefsky <[email protected]>

Introduce a new family of printk macros which prefixes each kmsg message with a component name and allows to tag the printk with a message id.

Quoted 5 lines · Martin Schwidefsky

From: Michael Holzheu <[email protected]>
From: Martin Schwidefsky <[email protected]>

Introduce a new family of printk macros which prefixes each kmsg message with a component name and allows to tag the printk with a message id.

We've been talking about doing something like this, just this week. AKPM pointed me at this thread. Good timing. :)

include/linux/kmsg.h line 12

Quoted 3 lines

include/linux/kmsg.h new file +3 -0

+#define kmsg_dev_alert(id, dev, format, arg...) \
+    printk(__KMSG_CHECK(ALERT, id) KMSG_COMPONENT \
+        ": %s: " format, (dev)->bus_id , ## arg)

Rather than the gcc-specific '## arg', shouldn't we use the standard C99
form: '##__VA_ARGS__' ?

As a format, I would argue to leave out the spaces after colons, making the inevitable string splitting even easier.

I don't care for the magic file-global KMSG_COMPONENT. It just feels wrong. In most cases, I want the prefix to be hierarchical. For example, rather than "tg3" I want "net.tg3" or "netdev:tg3" or something.

Further, We want to make a clearer denotation between "just another printk()" and these things, which are really "important events". Maybe a global prefix that makes it easy to seperate them out? Or a parallel "event" interface that logs to printk() and a netlink socket?

Quoted 3 lines

I don't care for the magic file-global KMSG_COMPONENT. It just feels wrong. In most cases, I want the prefix to be hierarchical. For example, rather than "tg3" I want "net.tg3" or "netdev:tg3" or something.

But what is the alternative to the #define to specify the base name of the message tag? KBUILD_MODNAME has been proposed but I don't think this is a good idea since I want the message tags to be independent of the code structure.

Quoted 4 lines

Further, We want to make a clearer denotation between "just another printk()" and these things, which are really "important events". Maybe a global prefix that makes it easy to seperate them out? Or a parallel "event" interface that logs to printk() and a netlink socket?

Well the kmsg macros distinguish between id==0 and id>0 messages. For a id==0 message there is no documentation required and the kmsg prefix will be just the KMSG_COMPONENT. You could consider that to be "just another printk". For id>0 the kmsg macros will create the tuppel KMSG_COMPONENT.ID so you already can separate "normal" and "important" prinkts.

Quoted 6 lines

What I want to see is the elevation of these call-sites from "it's just a printk()" to "it's a report-worthy event and part of our user<->kernel API". I want people to add them when it's appropriate and then not change the strings later. I don't care if, in the end, they are structured strings. I do care if I have to run dozens of regexes against thousands of log-lines on thousands of systems.

Yes, and this has already happened with the s390 device driver messages. With the conversion to kmsg our developers removed a lot of crud from the code, this alone makes it worthwhile.

    // this is the actual implementation
    extern void send_kmsg(int level, const char *fmt, ...);
    // this is a convenience wrapper, maybe not needed as they get combinatoric
    #define kmsg_err(fmt, ...) \
    	send_kmsg(KERN_ERROR, fmt, ##__VA_ARGS__)

inside a subsystem-specific header:

    #define mysubsys_kmsg_err(fmt, ...) \
    	kmsg_err("mysubsys:" fmt "\n", ##__VA_ARGS__)
  inside each subsystem-driver:
    #define mydriver_kmsg_err(fmt, ...) \
    	mysubsys_kmsg_err("mydriver:" fmt, ##__VA_ARGS__)
  then the driver just calls:
    mydriver_kmsg_err("something happened");

Do you see *every* printk() becoming a kmsg?
Just printk() in drivers?
Or just some hand-chosen printk()s ?

Quoted 5 lines · Tim Hockin

    // this is the actual implementation
    extern void send_kmsg(int level, const char *fmt, ...);
    // this is a convenience wrapper, maybe not needed as they get combinatoric
    #define kmsg_err(fmt, ...) \
    	send_kmsg(KERN_ERROR, fmt, ##__VA_ARGS__)

Quoted 8 lines

inside a subsystem-specific header:

    #define mysubsys_kmsg_err(fmt, ...) \
    	kmsg_err("mysubsys:" fmt "\n", ##__VA_ARGS__)
  inside each subsystem-driver:
    #define mydriver_kmsg_err(fmt, ...) \
    	mysubsys_kmsg_err("mydriver:" fmt, ##__VA_ARGS__)
  then the driver just calls:
    mydriver_kmsg_err("something happened");

I think adding system/subsystem/driver prefixes
to function names is not good.

What is wrong with what we have already agreed to standardise on here people? dev_printk() for devices! It uniquely shows the device, what driver is bound to it (if any), the bus id, and everything else.

Quoted 4 lines · Greg KH

What is wrong with what we have already agreed to standardise on here people? dev_printk() for devices! It uniquely shows the device, what driver is bound to it (if any), the bus id, and everything else.

Part of the problem, imho, is the "if any" part. But I am more than happy to build on existing solutions. All the world is not a dev, though. I'd like to be able to report something like an OOM kill in (roughly) the same way as an ATA error, and I want (though could be talked out of) a way to tell these "events" (for lack of a better word) apart from plain-old-printk()s.

Quoted 11 lines · Tim Hockin

Quoted 4 lines · Greg KH

What is wrong with what we have already agreed to standardise on here people? dev_printk() for devices! It uniquely shows the device, what driver is bound to it (if any), the bus id, and everything else.

Part of the problem, imho, is the "if any" part. But I am more than happy to build on existing solutions. All the world is not a dev, though. I'd like to be able to report something like an OOM kill in (roughly) the same way as an ATA error, and I want (though could be talked out of) a way to tell these "events" (for lack of a better word) apart from plain-old-printk()s.

I don't think that he wants to unify all the printk's in the system. I don't think that reporting all errors "in the same way as an ATA error" makes any sense. That would just lead to very stupid and unnatural messages for all errors that are not like "ATA errors". Annotation of existing errors is a much more flexible and feasible solution to that problem.

Quoted 5 lines

I don't think that he wants to unify all the printk's in the system. I don't think that reporting all errors "in the same way as an ATA error" makes any sense. That would just lead to very stupid and unnatural messages for all errors that are not like "ATA errors". Annotation of existing errors is a much more flexible and feasible solution to that problem.

Please don't misinterpret. I don't want to make other errors parse like an ATA error, I want to make the plumbing be parallel. I want one umbrella mechanism for reporting things that are more important than just-another-printk().

Because frankly, "parse dmesg" is a pretty crappy way to have to monitor your system for failures, and I am tired of explaining to people why we still do that.

Hi!

Quoted 6 lines

Quoted 5 lines

I don't think that he wants to unify all the printk's in the system. I don't think that reporting all errors "in the same way as an ATA error" makes any sense. That would just lead to very stupid and unnatural messages for all errors that are not like "ATA errors". Annotation of existing errors is a much more flexible and feasible solution to that problem.

Quoted 8 lines

Please don't misinterpret. I don't want to make other errors parse like an ATA error, I want to make the plumbing be parallel. I want one umbrella mechanism for reporting things that are more important than just-another-printk().

Because frankly, "parse dmesg" is a pretty crappy way to have to monitor your system for failures, and I am tired of explaining to people why we still do that.

"parse dmesg" does not work for monitoring your system for failures; dmesg buffer can overflow.

If something fails, you should get errno returned for userspace, and that's where you should be doing the monitoring.

So... what parts don't return enough information to userspace so that you need to parse dmesg? Lets fix them.

Hi!

Quoted 6 lines

Quoted 5 lines

I don't think that he wants to unify all the printk's in the system. I don't think that reporting all errors "in the same way as an ATA error" makes any sense. That would just lead to very stupid and unnatural messages for all errors that are not like "ATA errors". Annotation of existing errors is a much more flexible and feasible solution to that problem.

Quoted 8 lines

Please don't misinterpret. I don't want to make other errors parse like an ATA error, I want to make the plumbing be parallel. I want one umbrella mechanism for reporting things that are more important than just-another-printk().

Because frankly, "parse dmesg" is a pretty crappy way to have to monitor your system for failures, and I am tired of explaining to people why we still do that.

"parse dmesg" does not work for monitoring your system for failures; dmesg buffer can overflow.

If something fails, you should get errno returned for userspace, and that's where you should be doing the monitoring.

So... what parts don't return enough information to userspace so that you need to parse dmesg? Lets fix them.

Quoted 4 lines · Greg KH

What is wrong with what we have already agreed to standardise on here people? dev_printk() for devices! It uniquely shows the device, what driver is bound to it (if any), the bus id, and everything else.

Part of the problem, imho, is the "if any" part. But I am more than happy to build on existing solutions. All the world is not a dev, though.

I'd like to be able to report something like an OOM kill in (roughly) the same way as an ATA error, and I want (though could be talked out of) a way to tell these "events" (for lack of a better word) apart from plain-old-printk()s.

Quoted 4 lines

I'd like to be able to report something like an OOM kill in (roughly) the same way as an ATA error, and I want (though could be talked out of) a way to tell these "events" (for lack of a better word) apart from plain-old-printk()s.

That's great, then create something that can handle both! Don't throw away some wonderful information that way over half the kernel has access to just because the minority doesn't. That would mean that we would loose information in those drivers overall.

Quoted 5 lines · Greg KH

Quoted 4 lines

I'd like to be able to report something like an OOM kill in (roughly) the same way as an ATA error, and I want (though could be talked out of) a way to tell these "events" (for lack of a better word) apart from plain-old-printk()s.

Quoted 4 lines

That's great, then create something that can handle both! Don't throw away some wonderful information that way over half the kernel has access to just because the minority doesn't. That would mean that we would loose information in those drivers overall.

Sorry, I never meant to imply that dev_printk() was to be ignored, my examples was meant to be trivial to demonstrate :) dev_printk() is good. We should keep it :)

From: Michael Holzheu <[email protected]>
From: Martin Schwidefsky <[email protected]>

Introduce a new family of printk macros which prefixes each kmsg message with a component name and allows to tag the printk with a message id.

Quoted 5 lines · On Thursday 31 July 2008 02:56:57 Martin Schwidefsky wrote:

From: Michael Holzheu <[email protected]>
From: Martin Schwidefsky <[email protected]>

Introduce a new family of printk macros which prefixes each kmsg message with a component name and allows to tag the printk with a message id.

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

Rusty.
--
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/

But there are a number of places that have different strings for the same meaning: "Can't" vs "can't" vs "can not" vs "failed to", etc.

Hi!

Quoted 2 lines

But there are a number of places that have different strings for the same meaning: "Can't" vs "can't" vs "can not" vs "failed to", etc.

Do you have examples?

That wastes space in bzImage (and hints of duplicated code), so we may want to fix it anyway.

But there are a number of places that have different strings for the same meaning: "Can't" vs "can't" vs "can not" vs "failed to", etc.

This is a big part of what hurts us - you end up with lots of loose regexes when parsing logs.

Quoted 2 lines · On Wednesday 13 August 2008 17:04:11 Tim Hockin wrote:

But there are a number of places that have ...Well, if they're to be the same messages under the manual-numbering scheme,

someone has to verify that they are really the same anyway. Not much more work to simply fix them in that case, is it?

If it improves normal kernel messages, then everyone wins.

Quoted 4 lines · Rusty Russell

Quoted 2 lines · On Wednesday 13 August 2008 17:04:11 Tim Hockin wrote:

But there are a number of places that have ...Well, if they're to be the same messages under the manual-numbering scheme,

Quoted 4 lines

someone has to verify that they are really the same anyway. Not much more work to simply fix them in that case, is it?

If it improves normal kernel messages, then everyone wins.

Oh, I agree with that. I personally don't care much for the msg ID part of this patch (I mean I don't care either way). It might be nice to have man pages, but I just see it becoming out of sync, duplicate numbers, etc.

What I want to see is the elevation of these call-sites from "it's just a printk()" to "it's a report-worthy event and part of our user<->kernel API". I want people to add them when it's appropriate and then not change the strings later. I don't care if, in the end, they are structured strings. I do care if I have to run dozens of regexes against thousands of log-lines on thousands of systems.

:)

Quoted 5 lines · On Thursday 31 July 2008 02:56:57 Martin Schwidefsky wrote:

From: Michael Holzheu <[email protected]>
From: Martin Schwidefsky <[email protected]>

Introduce a new family of printk macros which prefixes each kmsg message with a component name and allows to tag the printk with a message id.

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

Quoted 8 lines · Rusty Russell

Quoted 6 lines · On Thursday 31 July 2008 02:56:57 Martin Schwidefsky wrote:

From: Michael Holzheu <[email protected]>
From: Martin Schwidefsky <[email protected]>

Introduce a new family of printk macros which prefixes each kmsg message with a component name and allows to tag the printk with a message id.

Quoted 4 lines

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

First, how do you do that with the C preprocessor? Second the kmsg-doc script will check that the message format strings match if a kmsg with a specific id is used in multiple locations.

Quoted 14 lines · On Friday 15 August 2008 03:07:03 Martin Schwidefsky wrote:

Quoted 12 lines · Rusty Russell

Quoted 6 lines · On Thursday 31 July 2008 02:56:57 Martin Schwidefsky wrote:

From: Michael Holzheu <[email protected]>
From: Martin Schwidefsky <[email protected]>

Introduce a new family of printk macros which prefixes each kmsg message with a component name and allows to tag the printk with a message id.

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

Quoted 3 lines

First, how do you do that with the C preprocessor? Second the kmsg-doc script will check that the message format strings match if a kmsg with a specific id is used in multiple locations.

Well, why would you do it with the preprocessor? It's simplest to spit it out at runtime.

The (offline) catalogger will need to generate them, of course. Rusty.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

Ok, so a specialized version of printk will do the work to calculate the hash. Only, what will we do if there ever is a conflict? The message tag has to be unique. The shorter the hash is, the more likely a collision gets. Don't know if 6 hash digits is enough to just ignore the problem.

Quoted 5 lines

Ok, so a specialized version of printk will do the work to calculate the hash. Only, what will we do if there ever is a conflict? The message tag has to be unique. The shorter the hash is, the more likely a collision gets. Don't know if 6 hash digits is enough to just ignore the problem.

And if you ever need to change the text that is in the format string? The hash changes. That seems exactly counter to your goal...

On Sat, Aug 16, 2008 at 10:49 AM, Martin Schwidefsky

Quoted 5 lines · <[email protected]> wrote:

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

Quoted 5 lines

Ok, so a specialized version of printk will do the work to calculate the hash. Only, what will we do if there ever is a conflict? The message tag has to be unique. The shorter the hash is, the more likely a collision gets. Don't know if 6 hash digits is enough to just ignore the problem.

And if you ever need to change the text that is in the format string? The hash changes. That seems exactly counter to your goal...

Quoted 8 lines

Quoted 5 lines

Ok, so a specialized version of printk will do the work to calculate the hash. Only, what will we do if there ever is a conflict? The message tag has to be unique. The shorter the hash is, the more likely a collision gets. Don't know if 6 hash digits is enough to just ignore the problem.

And if you ever need to change the text that is in the format string? The hash changes. That seems exactly counter to your goal...

You need to catalogue them all anyway, so you can detect clashes at build time. Yes, you have to change the new string in that case, but that's easy.

And the number changing as the message changes is arguably a feature. You shouldn't (and we generally don't) idly change kernel messages.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

Quoted 5 lines

Ok, so a specialized version of printk will do the work to calculate the hash. Only, what will we do if there ever is a conflict? The message tag has to be unique. The shorter the hash is, the more likely a collision gets. Don't know if 6 hash digits is enough to just ignore the problem.

And if you ever need to change the text that is in the format string? The hash changes. That seems exactly counter to your goal...

Quoted 11 lines · Tim Hockin

Quoted 10 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

Ok, so a specialized version of printk will do the work to calculate the hash. Only, what will we do if there ever is a conflict? The message tag has to be unique. The shorter the hash is, the more likely a collision gets. Don't know if 6 hash digits is enough to just ignore the problem.

Quoted 2 lines

And if you ever need to change the text that is in the format string? The hash changes. That seems exactly counter to your goal...

The only problem here is trivial change like typos. Then the hash changes although the message is semantically still the same. If the message really changes, its id should change as well.

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

Quoted 3 lines · Rusty Russell

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

I second this suggestion (or propose something like it). A short hash or even an index into some off-line table lets the user hit the right page when looking for an explanation.

I've been using a similar (and perhaps ugly to some eyes) numeric message catalog for years (with token substitution and all). The catalogs can be translated and then replaced with local language so the users don't have to learn English (or French, or German, or Japanese, or Chinese).

Quoted 5 lines · On Thursday 31 July 2008 02:56:57 Martin Schwidefsky wrote:

From: Michael Holzheu <[email protected]>
From: Martin Schwidefsky <[email protected]>

Introduce a new family of printk macros which prefixes each kmsg message with a component name and allows to tag the printk with a message id.

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

Quoted 4 lines · Rusty Russell

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

If we go with hashes there is one more thing: kmsg(0, <string>) The variant where we manually assign the message ids knows about the "special" id 0. There is no documentation required for id 0 and none is wanted. If we replace the manual ids with hashes this will get lost. You could argue that a kmsg with id 0 is a normal printk so why not just use printk? What is lost is the information that this printk has been found to be not important enough to be documented.

Quoted 6 lines · On Tuesday 26 August 2008 01:56:30 Martin Schwidefsky wrote:

Quoted 4 lines · Rusty Russell

Can you hash the format string to generate the id? 6 hex digits should be enough, and your tool can check for clashes. As it's bad form to have identical strings for different semantics anyway, this seems to make sense.

Quoted 7 lines

If we go with hashes there is one more thing: kmsg(0, <string>) The variant where we manually assign the message ids knows about the "special" id 0. There is no documentation required for id 0 and none is wanted. If we replace the manual ids with hashes this will get lost. You could argue that a kmsg with id 0 is a normal printk so why not just use printk? What is lost is the information that this printk has been found to be not important enough to be documented.

Hmm, #define KERN_IGNORE KERN_DEBUG?

Quoted 1 line · Rusty Russell

Hmm, #define KERN_IGNORE KERN_DEBUG?

Hmm, shouldn't KERN_xxx levels be independent of the special id 0?

Quoted 4 lines · On Monday 01 September 2008 22:28:50 Martin Schwidefsky wrote:

Quoted 1 line · Rusty Russell

Hmm, #define KERN_IGNORE KERN_DEBUG?

Hmm, shouldn't KERN_xxx levels be independent of the special id 0?

I don't know. Why should something be untranslatable?

From: Michael Holzheu <[email protected]>
From: Martin Schwidefsky <[email protected]>

Add a script and the calls to the make process that allows to check the kmsg printk messages and to format man pages from the message descriptions.

The kmsg message description is a comment with the following format:

/*?
* Tag: <component>.<id>
* Text: "<kmsg message text>"
* Severity: <severity>
* Parameter:

 *   @1: <description of the first message parameter>
 *   @2: <description of the second message parameter>
 *   ...

* Description:
* <What is the kmsg message all about>
* User action:
* <What can the user do to fix the problem>
*/

The script looks for a kmsg comment for a kmsg printk at three places, the source file where the kmsg call is located, in the architecture dependent file Documentation/kmsg/$ARCH/<component> and in the common file Documentation/kmsg/<component>.

The kmsg check is invoked with "make D=1" and reads the source files for all objects that are built by the current configuration and searches for matching kmsg descriptions for the kmsg messages in the source which have a messages id > 0. If a message description can not be found the script prints a blueprint and causes a make error.

The kmsg man page creation is invoked with "make D=2" and reads the source files for all built objects, looks up the message description and writes a man page to $(objtree)/man.

/*?
* Tag: <component>.<id>
* Text: "<kmsg message text>"
* Severity: <severity>
* Parameter:

 *   @1: <description of the first message parameter>
 *   @2: <description of the second message parameter>
 *   ...

* Description:
* <What is the kmsg message all about>
* User action:
* <What can the user do to fix the problem> */

The script looks for a kmsg comment for a kmsg printk at three places, the source file where the kmsg call is located, in the architecture dependent file Documentation/kmsg/$ARCH/<component> and in the common file Documentation/kmsg/<component>.

The kmsg check is invoked with "make D=1" and reads the source files for all objects that are built by the current configuration and searches for matching kmsg descriptions for the kmsg messages in the source which have a messages id > 0. If a message description can not be found the script prints a blueprint and causes a make error.

The kmsg man page creation is invoked with "make D=2" and reads the source files for all built objects, looks up the message description and writes a man page to $(objtree)/man.

However, there is a problem of the scale in numbering of not only a
specific module but also all modules to the message. (There might be at least 34000 relating messages or more. )

From: Martin Schwidefsky <[email protected]>

Signed-off-by Martin Schwidefsky <[email protected]>

---

 Documentation/kmsg/s390/xpram |   54 ++++++++++++++++++++++++++++++++++++++++++
 drivers/s390/block/xpram.c    |   41 ++++++++++++++-----------------
 2 files changed, 73 insertions(+), 22 deletions(-)

Index: linux-2.6/Documentation/kmsg/s390/xpram
===================================================================

Documentation/kmsg/s390/xpram new file +6 -0

+ * Text: "%d is not a valid number of XPRAM devices"
+ * Severity: Error
+ * Parameter:
+ *   @1: number of partitions
+ * Description:
+ * The number of XPRAM partitions specified for the 'devs' module parameter

Documentation/kmsg/s390/xpram line 2

Quoted 7 lines · Martin Schwidefsky

Index: linux-2.6/Documentation/kmsg/s390/xpram
===================================================================

Documentation/kmsg/s390/xpram new file +2 -0

@@ -0,0 +1,54 @@
+/*?
+ * Tag: xpram.1

Ick, so you are going to have to define a message number per file? How is that going to work, it looks like you use ids 0-2 below in the .c file, yet in this documentation file they are 1-3. Off by one somewhere? :)

Quoted 6 lines

Documentation/kmsg/s390/xpram new file +6 -0

+ * Text: "%d is not a valid number of XPRAM devices"
+ * Severity: Error
+ * Parameter:
+ *   @1: number of partitions
+ * Description:
+ * The number of XPRAM partitions specified for the 'devs' module parameter

Documentation/kmsg/s390/xpram line 18

Quoted 10 lines

Documentation/kmsg/s390/xpram new file +10 -0

+ * or with the 'xpram.parts' kernel parameter must be an integer in the
+ * range 1 to 32. The XPRAM device driver created a maximum of 32 partitions
+ * that are probably not configured as intended.
+ * User action:
+ * If the XPRAM device driver has been compiled as a separate module,
+ * unload the module and load it again with a correct value for the
+ * 'devs' module parameter. If the XPRAM device driver has been compiled
+ * into the kernel, correct the 'xpram.parts' parameter in the kernel
+ * parameter line and restart Linux.
+ */

Any way to put this stuff in the .c file itself? It's hard enough getting people to update Documentation/ABI/, knowing to go modify something else in the Documentation directory is going to be _very_ difficult.

Documentation/kmsg/s390/xpram line 19

Quoted 3 lines

Documentation/kmsg/s390/xpram new file +1 -0

+#define KMSG_COMPONENT "xpram"

Can't you just use KBUILD_MODULE_NAME instead? That makes it one less thing you have to define in the code (and forget about when moving files around or cut-and-pasting).

Documentation/kmsg/s390/xpram

Quoted 5 lines

Documentation/kmsg/s390/xpram new file +1 -1

 	/* Check number of devices. */
 	if (devs <= 0 || devs > XPRAM_MAX_DEVS) {
-		PRINT_ERR("invalid number %d of devices\n",devs);
+		kmsg_err(1, "%d is not a valid number of XPRAM devices\n",devs);
 		return -EINVAL;

Magic number "1" here? See comment above about this.

Quoted 13 lines · Greg KH

Documentation/kmsg/s390/xpram line 2

Quoted 7 lines · Martin Schwidefsky

Index: linux-2.6/Documentation/kmsg/s390/xpram
===================================================================

Documentation/kmsg/s390/xpram new file +2 -0

@@ -0,0 +1,54 @@
+/*?
+ * Tag: xpram.1

Ick, so you are going to have to define a message number per file? How is that going to work, it looks like you use ids 0-2 below in the .c file, yet in this documentation file they are 1-3. Off by one somewhere? :)

The kmsg number 0 is special, the message tag will not include the message number for id 0. And the script won't complain that the message description is missing.

And yes, we WANT to define the message numbers per kmsg component.

Quoted 17 lines

Quoted 16 lines

Documentation/kmsg/s390/xpram new file +16 -0

+ * Text: "%d is not a valid number of XPRAM devices"
+ * Severity: Error
+ * Parameter:
+ *   @1: number of partitions
+ * Description:
+ * The number of XPRAM partitions specified for the 'devs' module parameter
+ * or with the 'xpram.parts' kernel parameter must be an integer in the
+ * range 1 to 32. The XPRAM device driver created a maximum of 32 partitions
+ * that are probably not configured as intended.
+ * User action:
+ * If the XPRAM device driver has been compiled as a separate module,
+ * unload the module and load it again with a correct value for the
+ * 'devs' module parameter. If the XPRAM device driver has been compiled
+ * into the kernel, correct the 'xpram.parts' parameter in the kernel
+ * parameter line and restart Linux.
+ */

Quoted 4 lines

Any way to put this stuff in the .c file itself? It's hard enough getting people to update Documentation/ABI/, knowing to go modify something else in the Documentation directory is going to be _very_ difficult.

Yes, already implemented. Just put the comment anywhere in the .c file where the message is printed. The Documentation/kmsg directory is just an additional option.

Quoted 5 lines

Documentation/kmsg/s390/xpram line 19

Quoted 1 line

Documentation/kmsg/s390/xpram new file +1 -0

+#define KMSG_COMPONENT "xpram"

Can't you just use KBUILD_MODULE_NAME instead? That makes it one less thing you have to define in the code (and forget about when moving files around or cut-and-pasting).

Two reason why we don't want to use KBUILD_MODULE_NAME:
1) the message tag (message component + message id) should never change, if you change the code structure the module name might change as well.

2) we want to be able to use the same kmsg component in multiple .c files.

Quoted 15 lines · Martin Schwidefsky

Quoted 13 lines · Greg KH

Documentation/kmsg/s390/xpram line 2

Quoted 7 lines · Martin Schwidefsky

Index: linux-2.6/Documentation/kmsg/s390/xpram
===================================================================

Documentation/kmsg/s390/xpram new file +2 -0

@@ -0,0 +1,54 @@
+/*?
+ * Tag: xpram.1

Ick, so you are going to have to define a message number per file? How is that going to work, it looks like you use ids 0-2 below in the .c file, yet in this documentation file they are 1-3. Off by one somewhere? :)

Quoted 3 lines

The kmsg number 0 is special, the message tag will not include the message number for id 0. And the script won't complain that the message description is missing.

Was "0 is special" defined anywhere that I missed?

Quoted 1 line

And yes, we WANT to define the message numbers per kmsg component.

But again, you have to manually match up numbers by hand, that's prone to get messy very quickly.

Quoted 22 lines

Quoted 21 lines

Quoted 6 lines

Documentation/kmsg/s390/xpram new file +6 -0

+ * Text: "%d is not a valid number of XPRAM devices"
+ * Severity: Error
+ * Parameter:
+ *   @1: number of partitions
+ * Description:
+ * The number of XPRAM partitions specified for the 'devs' module parameter

Documentation/kmsg/s390/xpram line 18

Quoted 10 lines

Documentation/kmsg/s390/xpram new file +10 -0

+ * or with the 'xpram.parts' kernel parameter must be an integer in the
+ * range 1 to 32. The XPRAM device driver created a maximum of 32 partitions
+ * that are probably not configured as intended.
+ * User action:
+ * If the XPRAM device driver has been compiled as a separate module,
+ * unload the module and load it again with a correct value for the
+ * 'devs' module parameter. If the XPRAM device driver has been compiled
+ * into the kernel, correct the 'xpram.parts' parameter in the kernel
+ * parameter line and restart Linux.
+ */

Any way to put this stuff in the .c file itself? It's hard enough getting people to update Documentation/ABI/, knowing to go modify something else in the Documentation directory is going to be _very_ difficult.

Quoted 3 lines

Yes, already implemented. Just put the comment anywhere in the .c file where the message is printed. The Documentation/kmsg directory is just an additional option.

Why not have your example show this?

Quoted 6 lines

Quoted 5 lines

Documentation/kmsg/s390/xpram line 19

Quoted 1 line

Documentation/kmsg/s390/xpram new file +1 -0

+#define KMSG_COMPONENT "xpram"

Can't you just use KBUILD_MODULE_NAME instead? That makes it one less thing you have to define in the code (and forget about when moving files around or cut-and-pasting).

Quoted 3 lines

Two reason why we don't want to use KBUILD_MODULE_NAME:
1) the message tag (message component + message id) should never change, if you change the code structure the module name might change as well.

Um, isn't that the point? If the code structure changes, then perhaps the message also should change? If not, it's trival to adjust.

Quoted 2 lines

2) we want to be able to use the same kmsg component in multiple .c files.

Why would this matter? It's just a "tag", who cares about the actual name?

Quoted 21 lines · Greg KH

Quoted 15 lines · Martin Schwidefsky

Quoted 13 lines · Greg KH

Documentation/kmsg/s390/xpram line 2

Quoted 7 lines · Martin Schwidefsky

Index: linux-2.6/Documentation/kmsg/s390/xpram
===================================================================

Documentation/kmsg/s390/xpram new file +2 -0

@@ -0,0 +1,54 @@
+/*?
+ * Tag: xpram.1

Ick, so you are going to have to define a message number per file? How is that going to work, it looks like you use ids 0-2 below in the .c file, yet in this documentation file they are 1-3. Off by one somewhere? :)

Quoted 3 lines

The kmsg number 0 is special, the message tag will not include the message number for id 0. And the script won't complain that the message description is missing.

Was "0 is special" defined anywhere that I missed?

Quoted 1 line

From the patch description of the kernel message catalog script:

The kmsg check is invoked with "make D=1" and reads the source files for all objects that are built by the current configuration and searches for matching kmsg descriptions for the kmsg messages in the source which have a messages id > 0. If a message description can not be found the script prints a blueprint and causes a make error.

Quoted 10 lines

Quoted 9 lines

Quoted 5 lines

Documentation/kmsg/s390/xpram line 3

Quoted 1 line

Documentation/kmsg/s390/xpram new file +1 -0

+#define KMSG_COMPONENT "xpram"

Can't you just use KBUILD_MODULE_NAME instead? That makes it one less thing you have to define in the code (and forget about when moving files around or cut-and-pasting).

Two reason why we don't want to use KBUILD_MODULE_NAME:
1) the message tag (message component + message id) should never change, if you change the code structure the module name might change as well.

Quoted 2 lines

Um, isn't that the point? If the code structure changes, then perhaps the message also should change? If not, it's trival to adjust.

NO! The message nor the message tag should change if the message semantically still reports the same thing. If the meaning of the message changes then change the message AND the message tag.

Quoted 5 lines

Quoted 2 lines

2) we want to be able to use the same kmsg component in multiple .c files.

Why would this matter? It's just a "tag", who cares about the actual name?

The actual name is not really important, but if the name is choosen wisely it does convey information. Guess what "dasd.17" tells you something about the dasd driver, "zfcp.42" about the zfcp driver and so on. The code structure should not dictate how the message tag is created.

Quoted 7 lines

Quoted 1 line

From the patch description of the kernel message catalog script:

The kmsg check is invoked with "make D=1" and reads the source files for all objects that are built by the current configuration and searches for matching kmsg descriptions for the kmsg messages in the source which have a messages id > 0. If a message description can not be found the script prints a blueprint and causes a make error.

Ok, but you don't describe messages with "0" in them :)

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Quoted 5 lines

Quoted 1 line

+#define KMSG_COMPONENT "xpram"

Can't you just use KBUILD_MODULE_NAME instead? That makes it one less thing you have to define in the code (and forget about when moving files around or cut-and-pasting).

Two reason why we don't want to use KBUILD_MODULE_NAME:
1) the message tag (message component + message id) should never change, if you change the code structure the module name might change as well.

Quoted 6 lines

Quoted 2 lines

Um, isn't that the point? If the code structure changes, then perhaps the message also should change? If not, it's trival to adjust.

NO! The message nor the message tag should change if the message semantically still reports the same thing. If the meaning of the message changes then change the message AND the message tag.

Why can't the message reporting change? What's the reluctance for change here for something that did happen to move to a different file? It shouldn't matter _at all_ as you are only looking at the tags/messages for a specific kernel version to ensure they match up. Any future kernel version might have different ones.

It's not like once you write a message/tag it will stay that way fixed for all time, that's just not going to fly with the way the Linux kernel is developed.

Quoted 11 lines

Quoted 5 lines

Quoted 2 lines

2) we want to be able to use the same kmsg component in multiple .c files.

Why would this matter? It's just a "tag", who cares about the actual name?

The actual name is not really important, but if the name is choosen wisely it does convey information. Guess what "dasd.17" tells you something about the dasd driver, "zfcp.42" about the zfcp driver and so on. The code structure should not dictate how the message tag is created.

The message tag should not dictate anything except how to look it up somehow. So it doesn't matter if the name changes, as long as the ability to get the real information is still there.

So the kernel could change the tags every other release and there would be no problem.

Quoted 23 lines · Greg KH

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Quoted 5 lines

Quoted 1 line

+#define KMSG_COMPONENT "xpram"

Can't you just use KBUILD_MODULE_NAME instead? That makes it one less thing you have to define in the code (and forget about when moving files around or cut-and-pasting).

Two reason why we don't want to use KBUILD_MODULE_NAME:
1) the message tag (message component + message id) should never change, if you change the code structure the module name might change as well.

Quoted 6 lines

Quoted 2 lines

Um, isn't that the point? If the code structure changes, then perhaps the message also should change? If not, it's trival to adjust.

NO! The message nor the message tag should change if the message semantically still reports the same thing. If the meaning of the message changes then change the message AND the message tag.

Why can't the message reporting change? What's the reluctance for change here for something that did happen to move to a different file? It shouldn't matter _at all_ as you are only looking at the tags/messages for a specific kernel version to ensure they match up. Any future kernel version might have different ones.

Quoted 3 lines

It's not like once you write a message/tag it will stay that way fixed for all time, that's just not going to fly with the way the Linux kernel is developed.

The message tag should uniquely identify the message so that the translation projects have something to work with. If we keep changing the message tag with each kernel release that will create a huge effort to keep track of the messages.

Quoted 16 lines

Quoted 11 lines

Quoted 5 lines

Quoted 2 lines

2) we want to be able to use the same kmsg component in multiple .c files.

Why would this matter? It's just a "tag", who cares about the actual name?

The actual name is not really important, but if the name is choosen wisely it does convey information. Guess what "dasd.17" tells you something about the dasd driver, "zfcp.42" about the zfcp driver and so on. The code structure should not dictate how the message tag is created.

The message tag should not dictate anything except how to look it up somehow. So it doesn't matter if the name changes, as long as the ability to get the real information is still there.

Quoted 2 lines

So the kernel could change the tags every other release and there would be no problem.

I think the unique message ids are very important. If we change them all the time this would severly limit the value of the kernel message catalog.

From: Martin Schwidefsky <[email protected]>

Signed-off-by Martin Schwidefsky <[email protected]>

---

 Documentation/kmsg/s390/xpram |   54 ++++++++++++++++++++++++++++++++++++++++++
 drivers/s390/block/xpram.c    |   41 ++++++++++++++-----------------
 2 files changed, 73 insertions(+), 22 deletions(-)

Index: linux-2.6/Documentation/kmsg/s390/xpram
===================================================================

Documentation/kmsg/s390/xpram new file +7 -0

@@ -0,0 +1,54 @@
+/*?
+ * Tag: xpram.1
+ * Text: "%d is not a valid number of XPRAM devices"
+ * Severity: Error
+ * Parameter:
+ *   @1: number of partitions
+ * Description:

It is "Kernel command line". Plus, I do not think I find "User action" section really helpful here.

...user that does not understand what "between 1 and 32 means" is unlikely to know if the driver is built-in or module...

Quoted 5 lines

The lastest changes:
* kmsg.h has been moved from include/asm-s390 to include/linux
* The s390 specific documentation is now in Documentation/kmsg/s390
* The script checks for messages in the source file, Documentation/kmsg and Documentation/kmsg/$SRCARCH

The tendency is that all ARCH specific files should be part of arch/$ARCH/*
What is the correct approach here I dunno.