2008-10-16 14:57:18

by Martin Schwidefsky

[permalink] [raw]
Subject: [GIT PULL] kernel message catalog patches

Hi Linus,

please pull from 'kmsg' branch of

git://git390.osdl.marist.edu/pub/scm/linux-2.6.git kmsg

to get the kernel message catalog code. This is been reworked half
a dozen times and this is the outcome. The bulk of this patch set
are the printk changes and documentation files for s390. The three
patches are:
"[S390] kmsg: tagged kernel messages.",
"[S390] kmsg: tagged device messages." and
"[S390] kmsg: Kernel message catalog script."

As info for Greg: I have removed the C99 variable argument changes for
device.h to avoid the patch conflict with the change for dynamic dev_dbg
printks. I'll send a seperate patch for the C99 variable arguments.

The short-log:

Christian Borntraeger (2):
[S390] kmsg: convert vmcp to kmsg api.
[S390] kmsg: convert cpcmd to kmsg api.

Christof Schmitt (1):
[S390] kmsg: convert zfcp printk messages to kmsg api.

Frank Blaschka (1):
[S390] kmsg: convert qeth print messages to kmsg api.

Frank Munzert (1):
[S390] kmsg: convert vmur to kmsg api.

Gerald Schaefer (2):
[S390] kmsg: convert appldata printk messages to kmsg api.
[S390] kmsg: convert monreader printk messages to kmsg api.

Hongjie Yang (1):
[S390] kmsg: convert dcssblk and extmem printk messages to kmsg api.

Jan Glauber (1):
[S390] kmsg: convert cpacf printk messages to kmsg api.

Martin Schwidefsky (11):
[S390] kmsg: tagged kernel messages.
[S390] kmsg: tagged device messages.
[S390] kmsg: Kernel message catalog script.
[S390] kmsg: convert xpram messages to kmsg api.
[S390] kmsg: convert lcs printk messages to kmsg api.
[S390] kmsg: convert time printk messages to kmsg api.
[S390] kmsg: convert setup printk messages to kmsg api.
[S390] kmsg: convert ap_bus to kmsg api.
[S390] kmsg: convert sclp printk messages to kmsg api.
[S390] kmsg: convert cpu related printks to kmsg api.
[S390] kmsg: convert vmlogrdr printk messages to kmsg api.

Melissa Howland (1):
[S390] kmsg: convert monwriter printk messages to kmsg api.

Michael Ernst (1):
[S390] kmsg: convert cio message to kmsg api.

Michael Holzheu (3):
[S390] kmsg: convert hypfs printk messages to kmsg api.
[S390] kmsg: convert s390 debug feature to kmsg api
[S390] kmsg: convert zfcp dumper printk messages to kmsg API.

Peter Tiedemann (1):
[S390] kmsg: convert ctcm printk messages to kmsg api.

Ursula Braun (1):
[S390] kmsg: convert iucv printk messages to kmsg api.

Documentation/kmsg/s390/aes_s390 | 32 ++
Documentation/kmsg/s390/af_iucv | 35 ++
Documentation/kmsg/s390/ap | 28 ++
Documentation/kmsg/s390/appldata | 93 ++++
Documentation/kmsg/s390/cio | 98 ++++
Documentation/kmsg/s390/cpcmd | 18 +
Documentation/kmsg/s390/cpu | 74 +++
Documentation/kmsg/s390/ctcm | 212 +++++++++
Documentation/kmsg/s390/dcssblk | 174 +++++++
Documentation/kmsg/s390/extmem | 311 ++++++++++++
Documentation/kmsg/s390/hypfs | 61 +++
Documentation/kmsg/s390/iucv | 21 +
Documentation/kmsg/s390/lcs | 170 +++++++
Documentation/kmsg/s390/monreader | 135 ++++++
Documentation/kmsg/s390/monwriter | 17 +
Documentation/kmsg/s390/netiucv | 149 ++++++
Documentation/kmsg/s390/qeth | 501 ++++++++++++++++++++
Documentation/kmsg/s390/s390dbf | 89 ++++
Documentation/kmsg/s390/sclp_cmd | 6 +
Documentation/kmsg/s390/sclp_config | 3 +
Documentation/kmsg/s390/sclp_cpi | 2 +
Documentation/kmsg/s390/sclp_sdias | 4 +
Documentation/kmsg/s390/setup | 164 +++++++
Documentation/kmsg/s390/time | 39 ++
Documentation/kmsg/s390/vmcp | 14 +
Documentation/kmsg/s390/vmlogrdr | 5 +
Documentation/kmsg/s390/vmur | 36 ++
Documentation/kmsg/s390/xpram | 61 +++
Documentation/kmsg/s390/zdump | 13 +
Documentation/kmsg/s390/zfcp | 888 +++++++++++++++++++++++++++++++++++
Makefile | 16 +
arch/s390/Kconfig | 9 +
arch/s390/appldata/appldata.h | 4 -
arch/s390/appldata/appldata_base.c | 12 +-
arch/s390/appldata/appldata_os.c | 21 +-
arch/s390/crypto/aes_s390.c | 14 +-
arch/s390/hypfs/hypfs_diag.c | 10 +-
arch/s390/hypfs/inode.c | 14 +-
arch/s390/kernel/Makefile | 4 +-
arch/s390/kernel/cpcmd.c | 7 +-
arch/s390/kernel/debug.c | 37 +-
arch/s390/kernel/processor.c | 98 ++++
arch/s390/kernel/setup.c | 166 ++-----
arch/s390/kernel/smp.c | 17 +-
arch/s390/kernel/time.c | 14 +-
arch/s390/kernel/topology.c | 5 +-
arch/s390/mm/extmem.c | 106 ++---
drivers/s390/block/dcssblk.c | 74 ++--
drivers/s390/block/xpram.c | 41 +-
drivers/s390/char/monreader.c | 41 +-
drivers/s390/char/monwriter.c | 5 +-
drivers/s390/char/sclp_cmd.c | 29 +-
drivers/s390/char/sclp_config.c | 10 +-
drivers/s390/char/sclp_cpi_sys.c | 12 +-
drivers/s390/char/sclp_sdias.c | 18 +-
drivers/s390/char/vmcp.c | 8 +-
drivers/s390/char/vmlogrdr.c | 26 +-
drivers/s390/char/vmur.c | 15 +-
drivers/s390/char/zcore.c | 14 +-
drivers/s390/cio/blacklist.c | 14 +-
drivers/s390/cio/chsc.c | 8 +-
drivers/s390/cio/cio.c | 5 +-
drivers/s390/cio/cmf.c | 8 +-
drivers/s390/cio/css.c | 8 +-
drivers/s390/crypto/ap_bus.c | 10 +-
drivers/s390/net/ctcm_fsms.c | 45 +-
drivers/s390/net/ctcm_main.c | 71 ++-
drivers/s390/net/ctcm_main.h | 7 +-
drivers/s390/net/ctcm_mpc.c | 14 +-
drivers/s390/net/ctcm_sysfs.c | 2 +
drivers/s390/net/lcs.c | 78 ++--
drivers/s390/net/netiucv.c | 64 ++-
drivers/s390/net/qeth_core.h | 8 +-
drivers/s390/net/qeth_core_main.c | 148 ++++---
drivers/s390/net/qeth_l2_main.c | 40 +-
drivers/s390/net/qeth_l3_main.c | 214 +++++----
drivers/s390/scsi/zfcp_aux.c | 11 +-
drivers/s390/scsi/zfcp_ccw.c | 2 +
drivers/s390/scsi/zfcp_cfdc.c | 2 +
drivers/s390/scsi/zfcp_dbf.c | 2 +
drivers/s390/scsi/zfcp_erp.c | 2 +
drivers/s390/scsi/zfcp_fc.c | 2 +
drivers/s390/scsi/zfcp_fsf.c | 2 +
drivers/s390/scsi/zfcp_qdio.c | 2 +
drivers/s390/scsi/zfcp_scsi.c | 2 +
drivers/s390/scsi/zfcp_sysfs.c | 2 +
include/linux/device.h | 27 +-
include/linux/kmsg.h | 42 ++
kernel/printk.c | 46 ++
net/iucv/af_iucv.c | 19 +-
net/iucv/iucv.c | 9 +-
scripts/Makefile.build | 14 +
scripts/kmsg-doc | 466 ++++++++++++++++++
93 files changed, 4954 insertions(+), 742 deletions(-)
create mode 100644 Documentation/kmsg/s390/aes_s390
create mode 100644 Documentation/kmsg/s390/af_iucv
create mode 100644 Documentation/kmsg/s390/ap
create mode 100644 Documentation/kmsg/s390/appldata
create mode 100644 Documentation/kmsg/s390/cio
create mode 100644 Documentation/kmsg/s390/cpcmd
create mode 100644 Documentation/kmsg/s390/cpu
create mode 100644 Documentation/kmsg/s390/ctcm
create mode 100644 Documentation/kmsg/s390/dcssblk
create mode 100644 Documentation/kmsg/s390/extmem
create mode 100644 Documentation/kmsg/s390/hypfs
create mode 100644 Documentation/kmsg/s390/iucv
create mode 100644 Documentation/kmsg/s390/lcs
create mode 100644 Documentation/kmsg/s390/monreader
create mode 100644 Documentation/kmsg/s390/monwriter
create mode 100644 Documentation/kmsg/s390/netiucv
create mode 100644 Documentation/kmsg/s390/qeth
create mode 100644 Documentation/kmsg/s390/s390dbf
create mode 100644 Documentation/kmsg/s390/sclp_cmd
create mode 100644 Documentation/kmsg/s390/sclp_config
create mode 100644 Documentation/kmsg/s390/sclp_cpi
create mode 100644 Documentation/kmsg/s390/sclp_sdias
create mode 100644 Documentation/kmsg/s390/setup
create mode 100644 Documentation/kmsg/s390/time
create mode 100644 Documentation/kmsg/s390/vmcp
create mode 100644 Documentation/kmsg/s390/vmlogrdr
create mode 100644 Documentation/kmsg/s390/vmur
create mode 100644 Documentation/kmsg/s390/xpram
create mode 100644 Documentation/kmsg/s390/zdump
create mode 100644 Documentation/kmsg/s390/zfcp
create mode 100644 arch/s390/kernel/processor.c
create mode 100644 include/linux/kmsg.h
create mode 100644 scripts/kmsg-doc

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.


2008-10-17 08:07:24

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [GIT PULL] kernel message catalog patches

Hi Linus,

On Thu, 2008-10-16 at 16:50 +0200, Martin Schwidefsky wrote:
> please pull from 'kmsg' branch of
>
> git://git390.osdl.marist.edu/pub/scm/linux-2.6.git kmsg
>
> to get the kernel message catalog code. This is been reworked half
> a dozen times and this is the outcome. The bulk of this patch set
> are the printk changes and documentation files for s390. The three
> patches are:
> "[S390] kmsg: tagged kernel messages.",
> "[S390] kmsg: tagged device messages." and
> "[S390] kmsg: Kernel message catalog script."

I've fixed the merge conflicts with todays git and recreated the kmsg
branch on git390.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2008-10-21 09:22:19

by Heiko Carstens

[permalink] [raw]
Subject: [GIT PULL/RESEND] kernel message catalog patches

Hi Linus,

since Martin is on vacation I send this pull request again.

please pull from 'kmsg' branch of

git://git390.osdl.marist.edu/pub/scm/linux-2.6.git kmsg

to get the kernel message catalog code. This has been reworked half
a dozen times and this is the outcome. The bulk of this patch set
are the printk changes and documentation files for s390. The three
patches are:
"[S390] kmsg: tagged kernel messages.",
"[S390] kmsg: tagged device messages." and
"[S390] kmsg: Kernel message catalog script."

The short-log:

Christian Borntraeger (2):
[S390] kmsg: convert vmcp to kmsg api.
[S390] kmsg: convert cpcmd to kmsg api.

Christof Schmitt (1):
[S390] kmsg: convert zfcp printk messages to kmsg api.

Frank Blaschka (1):
[S390] kmsg: convert qeth print messages to kmsg api.

Frank Munzert (1):
[S390] kmsg: convert vmur to kmsg api.

Gerald Schaefer (2):
[S390] kmsg: convert appldata printk messages to kmsg api.
[S390] kmsg: convert monreader printk messages to kmsg api.

Heiko Carstens (1):
Fix kmsg_doc script file permissions.

Hongjie Yang (1):
[S390] kmsg: convert dcssblk and extmem printk messages to kmsg api.

Jan Glauber (1):
[S390] kmsg: convert cpacf printk messages to kmsg api.

Klaus-D. Wacker (1):
[S390] kmsg: convert lcs printk messages to kmsg api.

Martin Schwidefsky (10):
[S390] kmsg: tagged kernel messages.
[S390] kmsg: tagged device messages.
[S390] kmsg: Kernel message catalog script.
[S390] kmsg: convert xpram messages to kmsg api.
[S390] kmsg: convert time printk messages to kmsg api.
[S390] kmsg: convert setup printk messages to kmsg api.
[S390] kmsg: convert ap_bus to kmsg api.
[S390] kmsg: convert sclp printk messages to kmsg api.
[S390] kmsg: convert cpu related printks to kmsg api.
[S390] kmsg: convert vmlogrdr printk messages to kmsg api.

Melissa Howland (1):
[S390] kmsg: convert monwriter printk messages to kmsg api.

Michael Ernst (1):
[S390] kmsg: convert cio message to kmsg api.

Michael Holzheu (3):
[S390] kmsg: convert hypfs printk messages to kmsg api.
[S390] kmsg: convert s390 debug feature to kmsg api
[S390] kmsg: convert zfcp dumper printk messages to kmsg API.

Peter Tiedemann (1):
[S390] kmsg: convert ctcm printk messages to kmsg api.

Ursula Braun (1):
[S390] kmsg: convert iucv printk messages to kmsg api.

Documentation/kmsg/s390/aes_s390 | 32 ++
Documentation/kmsg/s390/af_iucv | 35 ++
Documentation/kmsg/s390/ap | 28 ++
Documentation/kmsg/s390/appldata | 93 ++++
Documentation/kmsg/s390/cio | 98 ++++
Documentation/kmsg/s390/cpcmd | 18 +
Documentation/kmsg/s390/cpu | 74 +++
Documentation/kmsg/s390/ctcm | 212 +++++++++
Documentation/kmsg/s390/dcssblk | 174 +++++++
Documentation/kmsg/s390/extmem | 311 ++++++++++++
Documentation/kmsg/s390/hypfs | 61 +++
Documentation/kmsg/s390/iucv | 21 +
Documentation/kmsg/s390/lcs | 170 +++++++
Documentation/kmsg/s390/monreader | 135 ++++++
Documentation/kmsg/s390/monwriter | 17 +
Documentation/kmsg/s390/netiucv | 149 ++++++
Documentation/kmsg/s390/qeth | 501 ++++++++++++++++++++
Documentation/kmsg/s390/s390dbf | 89 ++++
Documentation/kmsg/s390/sclp_cmd | 6 +
Documentation/kmsg/s390/sclp_config | 3 +
Documentation/kmsg/s390/sclp_cpi | 2 +
Documentation/kmsg/s390/sclp_sdias | 4 +
Documentation/kmsg/s390/setup | 164 +++++++
Documentation/kmsg/s390/time | 39 ++
Documentation/kmsg/s390/vmcp | 14 +
Documentation/kmsg/s390/vmlogrdr | 5 +
Documentation/kmsg/s390/vmur | 36 ++
Documentation/kmsg/s390/xpram | 61 +++
Documentation/kmsg/s390/zdump | 13 +
Documentation/kmsg/s390/zfcp | 888 +++++++++++++++++++++++++++++++++++
Makefile | 16 +
arch/s390/Kconfig | 9 +
arch/s390/appldata/appldata.h | 4 -
arch/s390/appldata/appldata_base.c | 12 +-
arch/s390/appldata/appldata_os.c | 21 +-
arch/s390/crypto/aes_s390.c | 14 +-
arch/s390/hypfs/hypfs_diag.c | 10 +-
arch/s390/hypfs/inode.c | 14 +-
arch/s390/kernel/Makefile | 4 +-
arch/s390/kernel/cpcmd.c | 7 +-
arch/s390/kernel/debug.c | 37 +-
arch/s390/kernel/processor.c | 98 ++++
arch/s390/kernel/setup.c | 166 ++-----
arch/s390/kernel/smp.c | 17 +-
arch/s390/kernel/time.c | 14 +-
arch/s390/kernel/topology.c | 5 +-
arch/s390/mm/extmem.c | 106 ++---
drivers/s390/block/dcssblk.c | 74 ++--
drivers/s390/block/xpram.c | 41 +-
drivers/s390/char/monreader.c | 41 +-
drivers/s390/char/monwriter.c | 5 +-
drivers/s390/char/sclp_cmd.c | 29 +-
drivers/s390/char/sclp_config.c | 10 +-
drivers/s390/char/sclp_cpi_sys.c | 12 +-
drivers/s390/char/sclp_sdias.c | 18 +-
drivers/s390/char/vmcp.c | 8 +-
drivers/s390/char/vmlogrdr.c | 26 +-
drivers/s390/char/vmur.c | 15 +-
drivers/s390/char/zcore.c | 14 +-
drivers/s390/cio/blacklist.c | 14 +-
drivers/s390/cio/chsc.c | 8 +-
drivers/s390/cio/cio.c | 5 +-
drivers/s390/cio/cmf.c | 8 +-
drivers/s390/cio/css.c | 8 +-
drivers/s390/crypto/ap_bus.c | 10 +-
drivers/s390/net/ctcm_fsms.c | 45 +-
drivers/s390/net/ctcm_main.c | 71 ++-
drivers/s390/net/ctcm_main.h | 7 +-
drivers/s390/net/ctcm_mpc.c | 14 +-
drivers/s390/net/ctcm_sysfs.c | 2 +
drivers/s390/net/lcs.c | 78 ++--
drivers/s390/net/netiucv.c | 64 ++-
drivers/s390/net/qeth_core.h | 8 +-
drivers/s390/net/qeth_core_main.c | 148 ++++---
drivers/s390/net/qeth_l2_main.c | 40 +-
drivers/s390/net/qeth_l3_main.c | 214 +++++----
drivers/s390/scsi/zfcp_aux.c | 11 +-
drivers/s390/scsi/zfcp_ccw.c | 2 +
drivers/s390/scsi/zfcp_cfdc.c | 2 +
drivers/s390/scsi/zfcp_dbf.c | 2 +
drivers/s390/scsi/zfcp_erp.c | 2 +
drivers/s390/scsi/zfcp_fc.c | 2 +
drivers/s390/scsi/zfcp_fsf.c | 2 +
drivers/s390/scsi/zfcp_qdio.c | 2 +
drivers/s390/scsi/zfcp_scsi.c | 2 +
drivers/s390/scsi/zfcp_sysfs.c | 2 +
include/linux/device.h | 27 +-
include/linux/kmsg.h | 42 ++
kernel/printk.c | 46 ++
net/iucv/af_iucv.c | 19 +-
net/iucv/iucv.c | 9 +-
scripts/Makefile.build | 14 +
scripts/kmsg-doc | 470 ++++++++++++++++++
93 files changed, 4958 insertions(+), 742 deletions(-)
create mode 100644 Documentation/kmsg/s390/aes_s390
create mode 100644 Documentation/kmsg/s390/af_iucv
create mode 100644 Documentation/kmsg/s390/ap
create mode 100644 Documentation/kmsg/s390/appldata
create mode 100644 Documentation/kmsg/s390/cio
create mode 100644 Documentation/kmsg/s390/cpcmd
create mode 100644 Documentation/kmsg/s390/cpu
create mode 100644 Documentation/kmsg/s390/ctcm
create mode 100644 Documentation/kmsg/s390/dcssblk
create mode 100644 Documentation/kmsg/s390/extmem
create mode 100644 Documentation/kmsg/s390/hypfs
create mode 100644 Documentation/kmsg/s390/iucv
create mode 100644 Documentation/kmsg/s390/lcs
create mode 100644 Documentation/kmsg/s390/monreader
create mode 100644 Documentation/kmsg/s390/monwriter
create mode 100644 Documentation/kmsg/s390/netiucv
create mode 100644 Documentation/kmsg/s390/qeth
create mode 100644 Documentation/kmsg/s390/s390dbf
create mode 100644 Documentation/kmsg/s390/sclp_cmd
create mode 100644 Documentation/kmsg/s390/sclp_config
create mode 100644 Documentation/kmsg/s390/sclp_cpi
create mode 100644 Documentation/kmsg/s390/sclp_sdias
create mode 100644 Documentation/kmsg/s390/setup
create mode 100644 Documentation/kmsg/s390/time
create mode 100644 Documentation/kmsg/s390/vmcp
create mode 100644 Documentation/kmsg/s390/vmlogrdr
create mode 100644 Documentation/kmsg/s390/vmur
create mode 100644 Documentation/kmsg/s390/xpram
create mode 100644 Documentation/kmsg/s390/zdump
create mode 100644 Documentation/kmsg/s390/zfcp
create mode 100644 arch/s390/kernel/processor.c
create mode 100644 include/linux/kmsg.h
create mode 100755 scripts/kmsg-doc

2008-10-23 15:36:29

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches



On Tue, 21 Oct 2008, Heiko Carstens wrote:
>
> please pull from 'kmsg' branch of
>
> git://git390.osdl.marist.edu/pub/scm/linux-2.6.git kmsg
>
> to get the kernel message catalog code.

I'm not going to pull it, since I have no idea what it is, or why I'd
_want_ to pull it.

Linus

2008-10-23 21:06:32

by Heiko Carstens

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

On Thu, Oct 23, 2008 at 08:35:40AM -0700, Linus Torvalds wrote:
> On Tue, 21 Oct 2008, Heiko Carstens wrote:
> >
> > please pull from 'kmsg' branch of
> >
> > git://git390.osdl.marist.edu/pub/scm/linux-2.6.git kmsg
> >
> > to get the kernel message catalog code.
>
> I'm not going to pull it, since I have no idea what it is, or why I'd
> _want_ to pull it.

The whole point of the patch set is to add documentation for kernel
messages that are emitted via printk. The documentation is supposed to
help a sys admin to help understand what went wrong.

To achieve this each message gets a component name and a hash and so
gets a unique identifier with which the documentation for the message
can be found. The documentation can be within the source code or in
the Documentation/kmsg/<arch> folder. A script allows to create man
pages from the documentations (make D=2). The user can then just issue
'man <componentname>.<hash>' and gets a detailed description of what
went wrong and what to do.

To add component name and hash to the printk output new macros have to
be used (see patch "kmsg: tagged kernel messages."):

kmsg_emerg(format, args...);
kmsg_alert(format, args...);
kmsg_crit(format, args...);
kmsg_err(format, args...);
kmsg_warn(format, args...);
kmsg_notice(format, args...);
kmsg_info(format, args...);

If kernel message identifiers are disabled they just translate into
printk(KERN_EMERG, format, args...); etc.

Otherwise the code in question has to specify its compoment name
with a KMSG_COMPONENT define and kmesg_<level>() would translate into

printk_hash(level KMSG_COMPONENT ".%06x" ": ", format, ##__VA_ARGS__)

where printk_hash will generate a hash for the format string.

Example:

The s390 xpram driver now has:

#define KMSG_COMPONENT "xpram"
[...]
if (devs <= 0 || devs > XPRAM_MAX_DEVS) {
kmsg_err("%d is not a valid number of XPRAM devices\n",devs);

If the message would be printed it could look like:

xpram.ab9aa4 42 is not a valid number of XPRAM devices

The corresponding description for the message is in
Documentation/kmsg/s390/xpram :

/*?
* Tag: xpram.ab9aa4
* Text: "%d is not a valid number of XPRAM devices\n"
* 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
* command line and restart Linux.
*/

With "make D=2" a man page can be created from this:

.TH "xpram.ab9aa4" 9 "Linux Messages" LINUX
.SH Message
xpram.ab9aa4: %d is not a valid number of XPRAM devices\n
.SH Severity
Error
.SH Parameters
@1: number of partitions

.SH 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.
.SH 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
command line and restart Linux.


So with 'man <componentname>.<hash>' a sys admin would get information
of what went wrong.

Please note that this was initially an s390 only patch set but we moved
the infrastructure to generic code since it looks like others want a
facility like this too. iirc Andrew requested the move.

Thanks,
Heiko

2008-10-23 21:39:35

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches



On Thu, 23 Oct 2008, Heiko Carstens wrote:
>
> The whole point of the patch set is to add documentation for kernel
> messages that are emitted via printk. The documentation is supposed to
> help a sys admin to help understand what went wrong.

I'm still not convinced.

> Please note that this was initially an s390 only patch set but we moved
> the infrastructure to generic code since it looks like others want a
> facility like this too. iirc Andrew requested the move.

I do agree that it makes no sense as a s390 feature, but quite frankly,
I don't think it makes sense AT ALL. It introduces the notion of fixed
messages and people now suddenly need to worry about the hashes of the
contents etc. Exactly the kind of thing that I don't personally EVER want
to see, and certainly not inside the kernel.

If somebody really wants this, I seriously believe it would be better done
as a separate out-of-kernel package. Because I don't think it's worth
maintaining those hashed translations in-kernel, and I'm nto going to ask
people to even bother.

But if it's in-kernel, other people are then going to complain about them
not being maintained. And quite frankly, I'm neither willing nor
interested in hearing those complaints or making them more "valid".

So please keep the kernel message catalog external. Or try to convince me.
But don't send me a "please pull" without any explanation or any relevant
convincing argument.

Linus

2008-10-24 15:50:27

by Heiko Carstens

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

On Thu, Oct 23, 2008 at 02:37:24PM -0700, Linus Torvalds wrote:
> On Thu, 23 Oct 2008, Heiko Carstens wrote:
> > Please note that this was initially an s390 only patch set but we moved
> > the infrastructure to generic code since it looks like others want a
> > facility like this too. iirc Andrew requested the move.
>
> I do agree that it makes no sense as a s390 feature, but quite frankly,
> I don't think it makes sense AT ALL. It introduces the notion of fixed
> messages and people now suddenly need to worry about the hashes of the
> contents etc. Exactly the kind of thing that I don't personally EVER want
> to see, and certainly not inside the kernel.
>
> If somebody really wants this, I seriously believe it would be better done
> as a separate out-of-kernel package. Because I don't think it's worth
> maintaining those hashed translations in-kernel, and I'm nto going to ask
> people to even bother.
>
> But if it's in-kernel, other people are then going to complain about them
> not being maintained. And quite frankly, I'm neither willing nor
> interested in hearing those complaints or making them more "valid".

Ok, I agree that if people need to worry about hashes things become more
difficult. Also I don't think keeping the message descriptions external to
the kernel source would be too much of a burden for us.

More background for this patch series: on s390 sys admins want to have
descriptions for kernel messages. They are used to that, because all other
operating systems on that platform like z/OS, z/VM or z/VSE have message
catalogs with detailed descriptions about the semantics of the messages.

So how about if we just merge the infrastructure (= new printk wrappers)
upstream and convert our drivers to use these? The message catalog itself
would be out of the kernel tree and it would be our problem if hashes get
out of sync.

See below for the simple approach. I just added a single converted driver
for ease of review. The patch set is available at:

git://git390.osdl.marist.edu/pub/scm/linux-2.6.git kmsg

Martin Schwidefsky (3):
[S390] kmsg: tagged kernel messages.
[S390] kmsg: tagged device messages.
[S390] kmsg: convert xpram messages to kmsg api.

arch/s390/Kconfig | 9 ++++++++
drivers/s390/block/xpram.c | 41 ++++++++++++++++++---------------------
include/linux/device.h | 27 +++++++++++++++++++------
include/linux/kmsg.h | 42 ++++++++++++++++++++++++++++++++++++++++
kernel/printk.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 136 insertions(+), 29 deletions(-)
create mode 100644 include/linux/kmsg.h

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 70b7645..5566357 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -577,6 +577,15 @@ bool "s390 guest support for KVM (EXPERIMENTAL)"
the KVM hypervisor. This will add detection for KVM as well as a
virtio transport. If KVM is detected, the virtio console will be
the default console.
+
+config KMSG_IDS
+ bool "Kernel message numbers"
+ default n
+ 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.
+
endmenu

source "net/Kconfig"
diff --git a/drivers/s390/block/xpram.c b/drivers/s390/block/xpram.c
index 0391698..9bc4ae1 100644
--- a/drivers/s390/block/xpram.c
+++ b/drivers/s390/block/xpram.c
@@ -25,6 +25,8 @@
* generic hard disk support to replace ad-hoc partitioning
*/

+#define KMSG_COMPONENT "xpram"
+
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/ctype.h> /* isdigit, isxdigit */
@@ -36,18 +38,13 @@
#include <linux/hdreg.h> /* HDIO_GETGEO */
#include <linux/sysdev.h>
#include <linux/bio.h>
+#include <linux/kmsg.h>
#include <asm/uaccess.h>

#define XPRAM_NAME "xpram"
#define XPRAM_DEVS 1 /* one partition */
#define XPRAM_MAX_DEVS 32 /* maximal number of devices (partitions) */

-#define PRINT_DEBUG(x...) printk(KERN_DEBUG XPRAM_NAME " debug:" x)
-#define PRINT_INFO(x...) printk(KERN_INFO XPRAM_NAME " info:" x)
-#define PRINT_WARN(x...) printk(KERN_WARNING XPRAM_NAME " warning:" x)
-#define PRINT_ERR(x...) printk(KERN_ERR XPRAM_NAME " error:" x)
-
-
typedef struct {
unsigned int size; /* size of xpram segment in pages */
unsigned int offset; /* start page of xpram segment */
@@ -264,7 +261,7 @@ static int __init xpram_setup_sizes(unsigned long pages)

/* Check number of devices. */
if (devs <= 0 || devs > XPRAM_MAX_DEVS) {
- PRINT_ERR("invalid number %d of devices\n",devs);
+ kmsg_err("%d is not a valid number of XPRAM devices\n",devs);
return -EINVAL;
}
xpram_devs = devs;
@@ -295,22 +292,22 @@ static int __init xpram_setup_sizes(unsigned long pages)
mem_auto_no++;
}

- PRINT_INFO(" number of devices (partitions): %d \n", xpram_devs);
+ kmsg_info(" number of devices (partitions): %d \n", xpram_devs);
for (i = 0; i < xpram_devs; i++) {
if (xpram_sizes[i])
- PRINT_INFO(" size of partition %d: %u kB\n",
- i, xpram_sizes[i]);
+ kmsg_info(" size of partition %d: %u kB\n",
+ i, xpram_sizes[i]);
else
- PRINT_INFO(" size of partition %d to be set "
- "automatically\n",i);
+ kmsg_info(" size of partition %d to be set "
+ "automatically\n",i);
}
- PRINT_DEBUG(" memory needed (for sized partitions): %lu kB\n",
- mem_needed);
- PRINT_DEBUG(" partitions to be sized automatically: %d\n",
- mem_auto_no);
+ kmsg_info(" memory needed (for sized partitions): %lu kB\n",
+ mem_needed);
+ kmsg_info(" partitions to be sized automatically: %d\n",
+ mem_auto_no);

if (mem_needed > pages * 4) {
- PRINT_ERR("Not enough expanded memory available\n");
+ kmsg_err("Not enough expanded memory available\n");
return -EINVAL;
}

@@ -322,8 +319,8 @@ static int __init xpram_setup_sizes(unsigned long pages)
*/
if (mem_auto_no) {
mem_auto = ((pages - mem_needed / 4) / mem_auto_no) * 4;
- PRINT_INFO(" automatically determined "
- "partition size: %lu kB\n", mem_auto);
+ kmsg_info(" automatically determined "
+ "partition size: %lu kB\n", mem_auto);
for (i = 0; i < xpram_devs; i++)
if (xpram_sizes[i] == 0)
xpram_sizes[i] = mem_auto;
@@ -405,12 +402,12 @@ static int __init xpram_init(void)

/* Find out size of expanded memory. */
if (xpram_present() != 0) {
- PRINT_WARN("No expanded memory available\n");
+ kmsg_err("No expanded memory available\n");
return -ENODEV;
}
xpram_pages = xpram_highest_page_index() + 1;
- PRINT_INFO(" %u pages expanded memory found (%lu KB).\n",
- xpram_pages, (unsigned long) xpram_pages*4);
+ kmsg_info(" %u pages expanded memory found (%lu KB).\n",
+ xpram_pages, (unsigned long) xpram_pages*4);
rc = xpram_setup_sizes(xpram_pages);
if (rc)
return rc;
diff --git a/include/linux/device.h b/include/linux/device.h
index 1a3686d..588cf90 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -538,20 +538,33 @@ extern const char *dev_driver_string(const struct device *dev);
printk(level "%s %s: " format , dev_driver_string(dev) , \
dev_name(dev) , ## arg)

+/* dev_printk_hash for message documentation */
+#if defined(__KMSG_CHECKER) && defined(KMSG_COMPONENT)
+/* generate magic string for scripts/kmsg-doc to parse */
+#define dev_printk_hash(level, dev, format, arg...) \
+ __KMSG_DEV(level _FMT_ format _ARGS_ dev, ## arg _END_)
+#elif defined(CONFIG_KMSG_IDS) && defined(KMSG_COMPONENT)
+int printk_dev_hash(const char *, const struct device *, const char *, ...);
+#define dev_printk_hash(level, dev, format, arg...) \
+ printk_dev_hash(level "%s.%06x: %s: ", dev, format, ## arg)
+#else /* !defined(CONFIG_KMSG_IDS) */
+#define dev_printk_hash dev_printk
+#endif
+
#define dev_emerg(dev, format, arg...) \
- dev_printk(KERN_EMERG , dev , format , ## arg)
+ dev_printk_hash(KERN_EMERG , dev , format , ## arg)
#define dev_alert(dev, format, arg...) \
- dev_printk(KERN_ALERT , dev , format , ## arg)
+ dev_printk_hash(KERN_ALERT , dev , format , ## arg)
#define dev_crit(dev, format, arg...) \
- dev_printk(KERN_CRIT , dev , format , ## arg)
+ dev_printk_hash(KERN_CRIT , dev , format , ## arg)
#define dev_err(dev, format, arg...) \
- dev_printk(KERN_ERR , dev , format , ## arg)
+ dev_printk_hash(KERN_ERR , dev , format , ## arg)
#define dev_warn(dev, format, arg...) \
- dev_printk(KERN_WARNING , dev , format , ## arg)
+ dev_printk_hash(KERN_WARNING , dev , format , ## arg)
#define dev_notice(dev, format, arg...) \
- dev_printk(KERN_NOTICE , dev , format , ## arg)
+ dev_printk_hash(KERN_NOTICE , dev , format , ## arg)
#define dev_info(dev, format, arg...) \
- dev_printk(KERN_INFO , dev , format , ## arg)
+ dev_printk_hash(KERN_INFO , dev , format , ## arg)

#if defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
#define dev_dbg(dev, format, ...) do { \
diff --git a/include/linux/kmsg.h b/include/linux/kmsg.h
new file mode 100644
index 0000000..d356f77
--- /dev/null
+++ b/include/linux/kmsg.h
@@ -0,0 +1,42 @@
+#ifndef _LINUX_KMSG_H
+#define _LINUX_KMSG_H
+
+#define kmsg_printk(level, format, ...) \
+ printk(level KMSG_COMPONENT ": " format, ##__VA_ARGS__)
+
+#if defined(__KMSG_CHECKER)
+/* generate magic string for scripts/kmsg-doc to parse */
+#define kmsg_printk_hash(level, format, ...) \
+ __KMSG_PRINT(level _FMT_ format _ARGS_ ##__VA_ARGS__ _END_)
+#elif defined(CONFIG_KMSG_IDS)
+int printk_hash(const char *, const char *, ...);
+#define kmsg_printk_hash(level, format, ...) \
+ printk_hash(level KMSG_COMPONENT ".%06x" ": ", format, ##__VA_ARGS__)
+#else /* !defined(CONFIG_KMSG_IDS) */
+#define kmsg_printk_hash kmsg_printk
+#endif
+
+#define kmsg_emerg(fmt, ...) \
+ kmsg_printk_hash(KERN_EMERG, fmt, ##__VA_ARGS__)
+#define kmsg_alert(fmt, ...) \
+ kmsg_printk_hash(KERN_ALERT, fmt, ##__VA_ARGS__)
+#define kmsg_crit(fmt, ...) \
+ kmsg_printk_hash(KERN_CRIT, fmt, ##__VA_ARGS__)
+#define kmsg_err(fmt, ...) \
+ kmsg_printk_hash(KERN_ERR, fmt, ##__VA_ARGS__)
+#define kmsg_warn(fmt, ...) \
+ kmsg_printk_hash(KERN_WARNING, fmt, ##__VA_ARGS__)
+#define kmsg_notice(fmt, ...) \
+ kmsg_printk_hash(KERN_NOTICE, fmt, ##__VA_ARGS__)
+#define kmsg_info(fmt, ...) \
+ kmsg_printk_hash(KERN_INFO, fmt, ##__VA_ARGS__)
+
+#ifdef DEBUG
+#define kmsg_dbg(fmt, ...) \
+ kmsg_printk(KERN_DEBUG, fmt, ##__VA_ARGS__)
+#else
+#define kmsg_dbg(fmt, ...) \
+ ({ if (0) kmsg_printk(KERN_DEBUG, fmt, ##__VA_ARGS__); 0; })
+#endif
+
+#endif /* _LINUX_KMSG_H */
diff --git a/kernel/printk.c b/kernel/printk.c
index 6341af7..7c10b5b 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -32,6 +32,8 @@
#include <linux/security.h>
#include <linux/bootmem.h>
#include <linux/syscalls.h>
+#include <linux/jhash.h>
+#include <linux/device.h>

#include <asm/uaccess.h>

@@ -1341,3 +1343,47 @@ bool printk_timed_ratelimit(unsigned long *caller_jiffies,
}
EXPORT_SYMBOL(printk_timed_ratelimit);
#endif
+
+#if defined CONFIG_PRINTK && defined CONFIG_KMSG_IDS
+
+/**
+ * printk_hash - print a kernel message include a hash over the message
+ * @prefix: message prefix including the ".%06x" for the hash
+ * @fmt: format string
+ */
+asmlinkage int printk_hash(const char *prefix, const char *fmt, ...)
+{
+ va_list args;
+ int r;
+
+ r = printk(prefix, jhash(fmt, strlen(fmt), 0) & 0xffffff);
+ va_start(args, fmt);
+ r += vprintk(fmt, args);
+ va_end(args);
+
+ return r;
+}
+EXPORT_SYMBOL(printk_hash);
+
+/**
+ * printk_dev_hash - print a kernel message include a hash over the message
+ * @prefix: message prefix including the ".%06x" for the hash
+ * @dev: device this printk is all about
+ * @fmt: format string
+ */
+asmlinkage int printk_dev_hash(const char *prefix, const struct device *dev,
+ const char *fmt, ...)
+{
+ va_list args;
+ int r;
+
+ r = printk(prefix, dev_driver_string(dev),
+ jhash(fmt, strlen(fmt), 0) & 0xffffff, dev_name(dev));
+ va_start(args, fmt);
+ r += vprintk(fmt, args);
+ va_end(args);
+
+ return r;
+}
+EXPORT_SYMBOL(printk_dev_hash);
+#endif

2008-10-26 18:36:20

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

On Thu, 2008-10-23 at 14:37 -0700, Linus Torvalds wrote:
> On Thu, 23 Oct 2008, Heiko Carstens wrote:
> > The whole point of the patch set is to add documentation for kernel
> > messages that are emitted via printk. The documentation is supposed to
> > help a sys admin to help understand what went wrong.
>
> I'm still not convinced.

Too bad, let me try. For me there are three aspects to the kernel
messages: the user view, the developers task and what the distributions
have to do with the kernel package.
1) The user view is simple: they see a message on the console, go "wtf?"
and want to know more about the meaning of the message. Our aim is to
tag each message so that the user can cut-copy-paste the message id on a
call to 'man' and find the description which is hopefully useful.
2) The developers task should be as simple as possible. With the current
patches the best case is that the only code changed required is to add
the KMSG_COMPONENT #define to each source file of the driver. This is
the case if the driver exclusively uses the dev_xxx printk macros. If
other printks calls are used as well they should be replaced by the
kmsg_xxx macros. The next step is to do a "make D=1" and the blueprint
of all missing message descriptions is written to stdout. To write the
descriptions is naturally the tedious part, but that can not be helped.
We found with our s390 drivers that the need to provide the message
description actually improved the quality of the message because a lot
of crud has been removed.
3) For each kernel package that comes with the distribution an
additional binary package is supposed to be created from the kernel
source packages. The build rule is a "make D=2" and the install step
copies the man pages generated for this particular kernel build into a
seperate binary package. This way the man pages for the kernel message
will always be consistent with the binary kernel that is running on the
machine.

> > Please note that this was initially an s390 only patch set but we moved
> > the infrastructure to generic code since it looks like others want a
> > facility like this too. iirc Andrew requested the move.
>
> I do agree that it makes no sense as a s390 feature, but quite frankly,
> I don't think it makes sense AT ALL. It introduces the notion of fixed
> messages and people now suddenly need to worry about the hashes of the
> contents etc. Exactly the kind of thing that I don't personally EVER want
> to see, and certainly not inside the kernel.

Does it? The developers do need to worry about the text of the message
and the actual meaning of the message, no? If the semantics of a message
is changed the description has to change as well. The hash automatically
changes with the text so you won't get stale message descriptions for
modified messages. The next "make D=1" will find that the message lost
its description. The only text change that are "dangerous" are typo
fixes. The connection between the message and the description gets lost
but the description is actually still valid.

> If somebody really wants this, I seriously believe it would be better done
> as a separate out-of-kernel package. Because I don't think it's worth
> maintaining those hashed translations in-kernel, and I'm nto going to ask
> people to even bother.

There is no question that there are people using Linux on s390 who want
these message descriptions, for them they are valuable. We are not
talking about message translations though, only plain english
explanations about what the message is all about.

> But if it's in-kernel, other people are then going to complain about them
> not being maintained. And quite frankly, I'm neither willing nor
> interested in hearing those complaints or making them more "valid".

The usual answer to complaints of this sort is "send a patch", isn't it?
To write a message description is not rocket science, to spot slightly
changed messages where the description needs an update isn't hard as
well, the kmsg-doc script will tell you which messages lack a
description and then it is a simple grep.
As for the "don't bother me" reaction, you are not alone with that. As a
developer I am not interested in the message description, the source
code is good enough for me. But for a user who is not a developer things
are definitly different.

> So please keep the kernel message catalog external. Or try to convince me.
> But don't send me a "please pull" without any explanation or any relevant
> convincing argument.

Hmm, for s390 we can certainly keep the message catalog external as
there are not many device drivers who will be using the feature. I can
easily include the "make D=1" in my daily patch monkey business. But I
would like to get the code changes upstream.
For general use I do think that we need to have the message catalog in
the kernel tree to keep it consistent.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2008-10-26 19:14:49

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches



On Sun, 26 Oct 2008, Martin Schwidefsky wrote:
>
> > But if it's in-kernel, other people are then going to complain about them
> > not being maintained. And quite frankly, I'm neither willing nor
> > interested in hearing those complaints or making them more "valid".
>
> The usual answer to complaints of this sort is "send a patch", isn't it?

Yes. And why don't you do all that, and not involve me at all, and keep
all of this entirely out of the kernel?

Linus

2008-10-27 09:12:22

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

On Sun, 2008-10-26 at 12:12 -0700, Linus Torvalds wrote:
> On Sun, 26 Oct 2008, Martin Schwidefsky wrote:
> > > But if it's in-kernel, other people are then going to complain about them
> > > not being maintained. And quite frankly, I'm neither willing nor
> > > interested in hearing those complaints or making them more "valid".
> >
> > The usual answer to complaints of this sort is "send a patch", isn't it?
>
> Yes. And why don't you do all that, and not involve me at all, and keep
> all of this entirely out of the kernel?

Ok, understood. Not that the reaction surprises me, seems like nobody
likes documentation (including me). What I will do is to replace the
kmsg_xxx macros with the pr_xxx macros in the device drivers patches and
request the pull as a cleanup for 2.6.29. The out-of-tree kmsg patches
will then play tricks with pr_xxx analog to dev_xxx. That way the patch
should be minimal.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2008-10-27 15:12:45

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches



On Mon, 27 Oct 2008, Martin Schwidefsky wrote:
>
> Ok, understood. Not that the reaction surprises me, seems like nobody
> likes documentation (including me).

It's that I don't like out-of-line documentation. It's a damn pain to
maintain, and it's _especially_ so when it's for small details rather than
"big picture" issues.

I also consider this to be _exactly_ the same issue as translating kernel
messages into another language (which people have also wanted to do),
except the "other language" is a S390-specific "odd-speak" rather than a
real language.

I have to say that I also dislike the technical implementation. I don't
like having yet another printk() wrapper - your "kmsg_warn()" won't play
well with people who have messages they want to print, but that use helper
routines - or then you'd need to essentially change _every_ printk to a
kmsg_xyz().

So if you want to have a hash (so that you can identify the _format_
string rather than the printed out message), I personally think you'd be
better off thinking of it purely the same way as CONFIG_PRINTK_TIME, and
just have a config option that disables or enables the hashing of the
format string, the same way we have an option for disabling or enabling of
the timestamping of the printk.

I also suspect that it would be better to not _print_ it, but only put it
into the dmesg logs (the same way we do with the urgency level marker).

IOW, I think we could put a few lines of code in "vprintk" that just
hashes ove 'fmt' and then adds that to the output.

And as for the actual explanations: either they need to be totally outside
the kernel (in a project of their own), or they'd need to be "kernel-doc"
style things that are _in_ the source code. Not in Documentation/. Not
separate from the printk() that they are associated with.

Linus

2008-10-27 15:55:55

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

On Mon, 2008-10-27 at 08:05 -0700, Linus Torvalds wrote:
>
> On Mon, 27 Oct 2008, Martin Schwidefsky wrote:
> >
> > Ok, understood. Not that the reaction surprises me, seems like nobody
> > likes documentation (including me).
>
> It's that I don't like out-of-line documentation. It's a damn pain to
> maintain, and it's _especially_ so when it's for small details rather than
> "big picture" issues.

Yes, indeed. The farther away the documentation is from the source code
the harder it will be to maintain. That is why I would like to see it in
the linux source tree.

> I also consider this to be _exactly_ the same issue as translating kernel
> messages into another language (which people have also wanted to do),
> except the "other language" is a S390-specific "odd-speak" rather than a
> real language.

The message tag would be the way to find the translation in some
database from the plain english output you'll find on the screen.

> I have to say that I also dislike the technical implementation. I don't
> like having yet another printk() wrapper - your "kmsg_warn()" won't play
> well with people who have messages they want to print, but that use helper
> routines - or then you'd need to essentially change _every_ printk to a
> kmsg_xyz().

Today I've replaced kmsg_xyz() with pr_xyz(). The current code now plays
tricks with two families of printk macros: dev_xyz() and pr_xyz().
If you ask Greg every device driver should be using dev_xyz() for its
printks anyway..

> So if you want to have a hash (so that you can identify the _format_
> string rather than the printed out message), I personally think you'd be
> better off thinking of it purely the same way as CONFIG_PRINTK_TIME, and
> just have a config option that disables or enables the hashing of the
> format string, the same way we have an option for disabling or enabling of
> the timestamping of the printk.

In that case ALL printk messages would suddenly grow a hash. Which
precludes the use of the component name as part of the message since we
would need to add a component name for every single printk - that won't
happen.
Without a component name we are forced to use a larger number of bits
for the hash to avoid collisions. For 10,000 printks and a 32 bit hash
the likelyhood of a collision is already bigger than 2%, so we'd need
something bigger than 32 bit. With a component name in addition to the
hash you can split the printks into groups which greatly reduces the
danger of a collision.

> I also suspect that it would be better to not _print_ it, but only put it
> into the dmesg logs (the same way we do with the urgency level marker).
>
> IOW, I think we could put a few lines of code in "vprintk" that just
> hashes ove 'fmt' and then adds that to the output.

A dev_xyz() message would then look like this:

<level> <hash>: <driver-name> <device-name>: <the message text>

The prefix is rather lengthy, no ?

> And as for the actual explanations: either they need to be totally outside
> the kernel (in a project of their own), or they'd need to be "kernel-doc"
> style things that are _in_ the source code. Not in Documentation/. Not
> separate from the printk() that they are associated with.

The kmsg comments are already formatted in the kernel-doc style and you
can put the comment anywhere in the source file that contains the
printk. The Documentation/ is an extra path where the script looks for
the comments. I can easily drop that part. So yes, the concept is that
you can keep the message comment close to the printk.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2008-10-27 16:05:31

by Alan

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

> And as for the actual explanations: either they need to be totally outside
> the kernel (in a project of their own), or they'd need to be "kernel-doc"
> style things that are _in_ the source code. Not in Documentation/. Not
> separate from the printk() that they are associated with.

You really don't want 32 languages in mixed left/right rendering with
multiple fonts in your kernel source. At least not with most editing and
viewing tools....

User space uses message catalogues and has tools for maintaining them
which make using any other format somewhat dumb. They extract strings
using _("hello") type wrappers to identify what are translatable strings
so if the kernel source has _() macros all the tooling just works and can
live outside and way from the kernel.

Just needs _() to do the right thing compiled into the kernel and a
printk formatting to do the right thing with the result. In truth you
don't even need a hash in printk just the ability to make it produce
either

"Hello my name is 'fred3'"

and something like

"Hello my name is '%s%d'\0Fred\0\000\000\000\003"

The kernel contains the English which should be a pretty passable hash.

Alan

2008-10-27 16:13:14

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches



On Mon, 27 Oct 2008, Alan Cox wrote:
>
> > And as for the actual explanations: either they need to be totally outside
> > the kernel (in a project of their own), or they'd need to be "kernel-doc"
> > style things that are _in_ the source code. Not in Documentation/. Not
> > separate from the printk() that they are associated with.
>
> You really don't want 32 languages in mixed left/right rendering with
> multiple fonts in your kernel source. At least not with most editing and
> viewing tools....

I do agree. Another issue is that quite often the actual line to be
printed is likely fairly short, and often in an error path (so it's
indented and in an inconvenient place), but the explanation - even for a
_single_ language - may well be pretty involved and long.

Which is why I think for something like this, it really needs to be
entirely outside. Because putting it inside the kernel simply isn't going
to result in anything really useful. Either it can be close to the message
and be updated reasonably (never mind that if we really do support
multiple languages, that's not going to happen anyway, even if it's
close), or it's going to be somewhere else and not be updated when changes
happen - and then it migth as well be a separate project.

The 'separate project' is especially appropriate since any parsing won't
be done by the kernel anyway, but by some ksyslog thing.

Linus

2008-10-27 16:20:20

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

On Mon, Oct 27, 2008 at 04:52:06PM +0100, Martin Schwidefsky wrote:
> In that case ALL printk messages would suddenly grow a hash. Which
> precludes the use of the component name as part of the message since we
> would need to add a component name for every single printk - that won't
> happen.

Just as a suggestion, what about adding the component name the same
way we added the priority level --- i.e., by adding an optional
prefix, say "{COMPONENT}" to the printk string, which would be before
the urgency level marker. If it's not present, printk can generate a
64-bit hash; if it is present, printk can generate the component name
followed by a 32-bit hash.

That way we can gradually add component names in a completely
backwards compatible way, and only to the device drivers that care or
want it.

> > And as for the actual explanations: either they need to be totally outside
> > the kernel (in a project of their own), or they'd need to be "kernel-doc"
> > style things that are _in_ the source code. Not in Documentation/. Not
> > separate from the printk() that they are associated with.
>
> The kmsg comments are already formatted in the kernel-doc style and you
> can put the comment anywhere in the source file that contains the
> printk. The Documentation/ is an extra path where the script looks for
> the comments. I can easily drop that part. So yes, the concept is that
> you can keep the message comment close to the printk.

I would think keeping the kmsg comments as kernel-doc style in the
kernel source file makes a huge amount of sense.

Regards,

- Ted

2008-10-27 16:29:42

by Randy Dunlap

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

On Mon, 27 Oct 2008 12:19:23 -0400
Theodore Tso <[email protected]> wrote:

> On Mon, Oct 27, 2008 at 04:52:06PM +0100, Martin Schwidefsky wrote:
> > In that case ALL printk messages would suddenly grow a hash. Which
> > precludes the use of the component name as part of the message since we
> > would need to add a component name for every single printk - that won't
> > happen.
>
> Just as a suggestion, what about adding the component name the same
> way we added the priority level --- i.e., by adding an optional
> prefix, say "{COMPONENT}" to the printk string, which would be before
> the urgency level marker. If it's not present, printk can generate a
> 64-bit hash; if it is present, printk can generate the component name
> followed by a 32-bit hash.
>
> That way we can gradually add component names in a completely
> backwards compatible way, and only to the device drivers that care or
> want it.
>
> > > And as for the actual explanations: either they need to be totally outside
> > > the kernel (in a project of their own), or they'd need to be "kernel-doc"
> > > style things that are _in_ the source code. Not in Documentation/. Not
> > > separate from the printk() that they are associated with.
> >
> > The kmsg comments are already formatted in the kernel-doc style and you
> > can put the comment anywhere in the source file that contains the
> > printk. The Documentation/ is an extra path where the script looks for
> > the comments. I can easily drop that part. So yes, the concept is that
> > you can keep the message comment close to the printk.
>
> I would think keeping the kmsg comments as kernel-doc style in the
> kernel source file makes a huge amount of sense.

As I said a few months ago, please make it "almost kernel-doc style"
but something that can be distinguished from the current kernel-doc.
They aren't quite the same thing AFAICT.

--
~Randy

2008-10-27 16:35:58

by Alan

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

> Which is why I think for something like this, it really needs to be
> entirely outside. Because putting it inside the kernel simply isn't going
> to result in anything really useful. Either it can be close to the message
> and be updated reasonably (never mind that if we really do support
> multiple languages, that's not going to happen anyway, even if it's
> close), or it's going to be somewhere else and not be updated when changes
> happen - and then it migth as well be a separate project.

Small bits of it have to be inside the kernel
- Markup (_("Hello") To extract the strings)
- To format the data in a way that can be recovered by ksyslogd and
translated.

Alan

2008-10-27 19:37:53

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

On Mon, Oct 27, 2008 at 09:10:56AM -0700, Linus Torvalds wrote:
>
> I do agree. Another issue is that quite often the actual line to be
> printed is likely fairly short, and often in an error path (so it's
> indented and in an inconvenient place), but the explanation - even for a
> _single_ language - may well be pretty involved and long.

It's likely that the explanation would need to be just outside the
containing function, and not in-line where it might need to be deeply
indented.

And I think we do need separate out the question of multiple langauges
from the explanation in a single canonical language (i.e., English).
Does keeping the explanation just before the containing function mean
that it's "so far away" that it might as well be kept completely
outside of the kernel? I don't think so, but it's probably not worth
a huge amount of effort debating it; tools for keeping an external
database in sync is a pain, but they've certainly been done before for
related problems (especially in the case of translations, where they
would have to be done that way anyway).

- Ted

2008-10-28 08:28:49

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [GIT PULL/RESEND] kernel message catalog patches

On Mon, 2008-10-27 at 09:27 -0700, Randy Dunlap wrote:
> As I said a few months ago, please make it "almost kernel-doc style"
> but something that can be distinguished from the current kernel-doc.
> They aren't quite the same thing AFAICT.

The current format is this:

/*?
* Tag: <component>.<hash>
* 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>
*/

See the "/*?" vs "/**" ? The kmsg comments are kernel-doc style but not
the same as kernel-doc.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.