linux-2.4.0-test11 and sysklogd-1.3-31

sysklogd 1.3-31 no longer compiles using the latest headers in test11.

Strictly speaking this isn't a kernel bug...

sysklogd's ksym_mod.c includes <linux/module.h>

In test11, <linux/module.h> added struct inter_module_entry. Its first member is "struct list_head list;". This necessitates the inclusion of <linux/list.h>.

The trouble is that <linux/list.h> is almost completely protected by #ifdef __KERNEL__.

sysklogd, obviously, doesn't compile with __KERNEL__ so the struct inter_module_entry declaration is impossible and the compilation fails.

It's not clear to me who's code needs changing so I'm sending both to linux-kernel and to some of the people that have the misfortune of being listed on the sysklogd man page.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Speaking as the modutils maintainer and the person who added list.h to module.h, sysklogd should *not* be including linux/module.h. Linus has spoken, it is an error for user space applications to include kernel headers. Even modutils does not include linux/module.h, instead it has a portable (2.0 through 2.4) local definition of struct module.

ksym_mod.c is only present to try to decode oops reports in klogd. klogd only handles some architectures, it often gets the oops decode wrong and it destroys the log information that is needed by other oops decoders. IMNSHO ksymoops does a much better job of decoding the oops, but I maintain ksymoops so I am just a little biased ;)

I would prefer to see the oops decoding completely removed from klogd. The only justification for klogd converting the oops is to save users from running ksymoops by hand. I would not mind klogd capturing the oops text, forking to run ksymoops then logging the ksymoops output. Just as along as the original text was left alone and the ksymoops output was obviously distinguished from real kernel output.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

KO> I would prefer to see the oops decoding completely removed from KO> klogd. The only justification for klogd converting the oops is KO> to save users from running ksymoops by hand. I would not mind KO> klogd capturing the oops text, forking to run ksymoops then KO> logging the ksymoops output. Just as along as the original text KO> was left alone and the ksymoops output was obviously KO> distinguished from real kernel output.

Since nobody else has weighed in on this issue, I quickly did the necessary to effect Keith's suggestion. What follows is a patch to sysklogd-1.3-31 (which after applying, ksym_mod.c can be removed):

# This is a BitKeeper generated patch for the following project:
# Project Name: Trans-lab fsimage sub-gate
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.60 -> 1.62
# src/sysklogd-1.3-31/ksym.c 1.1 -> 1.2
# src/sysklogd-1.3-31/klogd.c 1.1 -> 1.2
# src/sysklogd-1.3-31/ksyms.h 1.1 -> 1.2
# src/sysklogd-1.3-31/Makefile 1.1 -> 1.2
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 00/12/07 [email protected] 1.61
# Remove ksym_mod.c to fix sysklogd build
# --------------------------------------------
# 00/12/07 [email protected] 1.62
# Remove a remaining prototype associated with the now deleted ksym_mod.c # --------------------------------------------
#

diff -Nru a/src/sysklogd-1.3-31/Makefile b/src/sysklogd-1.3-31/Makefile
@@ -63,9 +63,8 @@
 syslogd: syslogd.o pidfile.o
 	${CC} ${LDFLAGS} -o syslogd syslogd.o pidfile.o ${LIBS}
 
-klogd:	klogd.o syslog.o pidfile.o ksym.o ksym_mod.o
-	${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o \
-		ksym_mod.o ${LIBS}
+klogd:	klogd.o syslog.o pidfile.o ksym.o
+	${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o ${LIBS}
 
 syslog_tst: syslog_tst.o
 	${CC} ${LDFLAGS} -o syslog_tst syslog_tst.o

diff -Nru a/src/sysklogd-1.3-31/klogd.c b/src/sysklogd-1.3-31/klogd.c
@@ -415,7 +415,6 @@
 	if (symbol_lookup) {
 		if ( reload_symbols > 1 )
 			InitKsyms(symfile);
-		InitMsyms();
 	}
 	reload_symbols = change_state = 0;
 	return;

@@ -1059,7 +1058,6 @@
 	{
 		if (symbol_lookup) {
 			InitKsyms(symfile);
-			InitMsyms();
 		}
 		if ( (logsrc = GetKernelLogSrc()) == kernel )
 			LogKernelLine();

@@ -1075,7 +1073,6 @@
 	logsrc = GetKernelLogSrc();
 	if (symbol_lookup) {
 		InitKsyms(symfile);
-		InitMsyms();
 	}
 
         /* The main loop. */

diff -Nru a/src/sysklogd-1.3-31/ksym.c b/src/sysklogd-1.3-31/ksym.c
@@ -656,9 +656,6 @@
 		last = sym_array[lp].name;
 	}
 
-	if ( (last = LookupModuleSymbol(value, sym)) != (char *) 0 )
-		return(last);
-
 	return((char *) 0);
 }
 

@@ -749,7 +746,7 @@
 	 * open for patches.
 	 */
 	if ( i_am_paranoid &&
-	     (strstr(line, "Oops:") != (char *) 0) && !InitMsyms() )
+	     (strstr(line, "Oops:") != (char *) 0) )
 		Syslog(LOG_WARNING, "Cannot load kernel module symbols.\n");
 	
 

diff -Nru a/src/sysklogd-1.3-31/ksyms.h b/src/sysklogd-1.3-31/ksyms.h
@@ -32,4 +32,3 @@
 
 /* Function prototypes. */
 extern char * LookupSymbol(unsigned long, struct symbol *);
-extern char * LookupModuleSymbol(unsigned long int, struct symbol *);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

You only removed the module symbol handling. The problem is that the entire klogd oops handling is out of date and broken. I recommend removing all oops processing from klogd, which means that klogd does not need any symbols nor System.map.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

KO> You only removed the module symbol handling. The problem is that KO> the entire klogd oops handling is out of date and broken. I KO> recommend removing all oops processing from klogd, which means KO> that klogd does not need any symbols nor System.map.

You're right.

My goal was to quickly get our build working again. I had no expectation that anybody else on the planet would give a crap about my patch...

But since you seem to and while we're doing extreme surgery, why have klogd at all? Every other unix, kernel messages are handled by the syslog system. What problem did klogd solve and does that problem still exist today? Stated differently, aren't you just proposing to reduce klogd to:

cat < /proc/kmsg > /dev/log &

(I know that this won't work on my box, but you get the idea)

Anyway, I'll look into simplifying klogd as you suggest...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

klogd maps the kernel messages from <n>text to syslog levels and does some fiddling with kernel log levels at start up. It needs to be more than a simple 'cat'. When symbol handling was added to klogd, ksymoops was built into the kernel and very unreliable. Since then ksymoops has been moved to a separate package and is now reliable. Alas oops handling in sysklogd has not kept up to date and is now the problem area.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

KO> klogd maps the kernel messages from <n>text to syslog levels and KO> does some fiddling with kernel log levels at start up. It needs KO> to be more than a simple 'cat'. When symbol handling was added KO> to klogd, ksymoops was built into the kernel and very unreliable. KO> Since then ksymoops has been moved to a separate package and is KO> now reliable. Alas oops handling in sysklogd has not kept up to KO> date and is now the problem area.

Thanks for the background.

KO> Linus, please do not apply.

KO> User space applications _must_ not include kernel headers. Even KO> modutils does not include linux/module.h, it has its own portable KO> (kernels 2.0 - 2.4) version.

KO> I have strongly recommended to the sysklogd maintainers that they KO> strip all the symbol handling from klogd. The oops decoding in KO> klogd is hopelessly out of date and broken.

Here's a patch (against sysklogd-1.3-31) that completely tear out the symbol processing code.

Additionally, after application, the following files may be removed:

ksyms.h
ksym.c
ksym_mod.c

diff -Nru a/src/sysklogd-1.3-31/Makefile b/src/sysklogd-1.3-31/Makefile
@@ -63,9 +63,8 @@
 syslogd: syslogd.o pidfile.o
 	${CC} ${LDFLAGS} -o syslogd syslogd.o pidfile.o ${LIBS}
 
-klogd:	klogd.o syslog.o pidfile.o ksym.o ksym_mod.o
-	${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o \
-		ksym_mod.o ${LIBS}
+klogd:	klogd.o syslog.o pidfile.o ksym.o
+	${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o ${LIBS}
 
 syslog_tst: syslog_tst.o
 	${CC} ${LDFLAGS} -o syslog_tst syslog_tst.o

diff -Nru a/src/sysklogd-1.3-31/klogd.c b/src/sysklogd-1.3-31/klogd.c
@@ -415,7 +415,6 @@
 	if (symbol_lookup) {
 		if ( reload_symbols > 1 )
 			InitKsyms(symfile);
-		InitMsyms();
 	}
 	reload_symbols = change_state = 0;
 	return;

@@ -1059,7 +1058,6 @@
 	{
 		if (symbol_lookup) {
 			InitKsyms(symfile);
-			InitMsyms();
 		}
 		if ( (logsrc = GetKernelLogSrc()) == kernel )
 			LogKernelLine();

@@ -1075,7 +1073,6 @@
 	logsrc = GetKernelLogSrc();
 	if (symbol_lookup) {
 		InitKsyms(symfile);
-		InitMsyms();
 	}
 
         /* The main loop. */

diff -Nru a/src/sysklogd-1.3-31/ksym.c b/src/sysklogd-1.3-31/ksym.c
@@ -656,9 +656,6 @@
 		last = sym_array[lp].name;
 	}
 
-	if ( (last = LookupModuleSymbol(value, sym)) != (char *) 0 )
-		return(last);
-
 	return((char *) 0);
 }
 

@@ -749,7 +746,7 @@
 	 * open for patches.
 	 */
 	if ( i_am_paranoid &&
-	     (strstr(line, "Oops:") != (char *) 0) && !InitMsyms() )
+	     (strstr(line, "Oops:") != (char *) 0) )
 		Syslog(LOG_WARNING, "Cannot load kernel module symbols.\n");
 	
 

diff -Nru a/src/sysklogd-1.3-31/ksyms.h b/src/sysklogd-1.3-31/ksyms.h
@@ -32,4 +32,3 @@
 
 /* Function prototypes. */
 extern char * LookupSymbol(unsigned long, struct symbol *);
-extern char * LookupModuleSymbol(unsigned long int, struct symbol *);

diff -Nru a/src/sysklogd-1.3-31/klogd.h b/src/sysklogd-1.3-31/klogd.h
@@ -33,8 +33,4 @@
 
 
 /* Function prototypes. */
-extern int InitKsyms(char *);
-extern int InitMsyms(void);
-extern char * ExpandKadds(char *, char *);
-extern void SetParanoiaLevel(int);
 extern void Syslog(int priority, char *fmt, ...);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

GN> Here's a patch (against sysklogd-1.3-31) that completely tear out GN> the symbol processing code.

Doh! Forgot a chunk (to be applied after the others):

diff -Nru a/src/sysklogd-1.3-31/Makefile b/src/sysklogd-1.3-31/Makefile
@@ -63,8 +63,8 @@
 syslogd: syslogd.o pidfile.o
 	${CC} ${LDFLAGS} -o syslogd syslogd.o pidfile.o ${LIBS}
 
-klogd:	klogd.o syslog.o pidfile.o ksym.o
-	${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o ${LIBS}
+klogd:	klogd.o syslog.o pidfile.o
+	${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ${LIBS}
 
 syslog_tst: syslog_tst.o
 	${CC} ${LDFLAGS} -o syslog_tst syslog_tst.o

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Looks good, except that you need to keep the option flags for backwards compatibility. There are a *lot* of scripts out there which invoke klogd with various options and they will fail with this change. It is OK to issue a warning message "klogd options -[iIpkx] are no longer supported" as long as klogd continues to run. Otherwise you will get a lot of irate users complaining that klogd is failing at boot time.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

KO> Looks good, except that you need to keep the option flags for KO> backwards compatibility. There are a *lot* of scripts out there KO> which invoke klogd with various options and they will fail with KO> this change. It is OK to issue a warning message "klogd options KO> -[iIpkx] are no longer supported" as long as klogd continues to KO> run. Otherwise you will get a lot of irate users complaining KO> that klogd is failing at boot time.

You're right. Here's YAP:

diff -Nru a/src/sysklogd-1.3-31/klogd.c b/src/sysklogd-1.3-31/klogd.c
@@ -763,7 +763,7 @@
 	chdir ("/");
 #endif
 	/* Parse the command-line. */
-	while ((ch = getopt(argc, argv, "c:df:nosv")) != EOF)
+	while ((ch = getopt(argc, argv, "c:df:nosviIk:px")) != EOF)
 		switch((char)ch)
 		{
 		    case 'c':		/* Set console message level. */

@@ -788,6 +788,20 @@
 		    case 'v':
 			printf("klogd %s-%s\n", VERSION, PATCHLEVEL);
 			exit (1);
+
+		    /* Obsolete options */
+		    case 'i':
+			/* FALLTHRU */
+		    case 'I':
+			/* FALLTHRU */
+		    case 'k':
+			/* FALLTHRU */
+		    case 'p':
+			/* FALLTHRU */
+		    case 'x':
+			fprintf(stderr,
+				"klogd: %c option is obsolete.  Ignoring\n", ch);
+			break;
 		}
 
 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

[Georg Nikodym]