[PATCH 002/001] /kernel: /proc/kallsyms reports lower-case types for some non-exported symbols (Resend #1)

[PATCH 002/001] /kernel: /proc/kallsyms reports lower-case types for some non-exported symbols (Resend #1)

[Adam Jerome]

From: Adam B. Jerome <abj@novell.com>

This patch addresses incorrect symbol type information reported through /proc/kallsyms.
A lowercase character should designate the symbol as local (or non-exported).  An
uppercase character should designate the symbol as global (or external). Without this
patch, some non-exported symbols are incorrectly assigned an upper-case designation in
/proc/kallsyms.  This patch corrects this condition by converting non-exported symbols
types to lower case when appropriate and eliminates the superfluous upcase_if_global
function

Signed-off-by: Adam B. Jerome <abj@novell.com>

---

I do not subscribe to the list. Please CC posted answers/comments to me <abj@novell.com>.
Thanks; -adam

diff -urpN linux-2.6-git/kernel/kallsyms.c linux-2.6-cur/kernel/kallsyms.c
--- linux-2.6-git/kernel/kallsyms.c	2006-10-02 15:20:25.000000000 -0600
+++ linux-2.6-cur/kernel/kallsyms.c	2006-10-02 15:58:03.000000000 -0600
@@ -20,6 +20,7 @@
 #include <linux/proc_fs.h>
 #include <linux/sched.h>	/* for cond_resched */
 #include <linux/mm.h>
+#include <linux/ctype.h>
 
 #include <asm/sections.h>
 
@@ -264,13 +265,6 @@ struct kallsym_iter
 	char name[KSYM_NAME_LEN+1];
 };
 
-/* Only label it "global" if it is exported. */
-static void upcase_if_global(struct kallsym_iter *iter)
-{
-	if (is_exported(iter->name, iter->owner))
-		iter->type += 'A' - 'a';
-}
-
 static int get_ksymbol_mod(struct kallsym_iter *iter)
 {
 	iter->owner = module_get_kallsym(iter->pos - kallsyms_num_syms,
@@ -279,7 +273,10 @@ static int get_ksymbol_mod(struct kallsy
 	if (iter->owner == NULL)
 		return 0;
 
-	upcase_if_global(iter);
+	/* Label it "global" if it ix exported, "local" if not exported. */
+	iter->type = is_exported(iter->name, iter->owner)
+		? toupper(iter->type) : tolower(iter->type);
+
 	return 1;
 }

Re: [PATCH 002/001] /kernel: /proc/kallsyms reports lower-case types for some non-exported symbols (Resend #1)

[Paulo Marques]

Adam Jerome wrote:
> This patch addresses incorrect symbol type information reported through /proc/kallsyms.
> A lowercase character should designate the symbol as local (or non-exported).  An
> uppercase character should designate the symbol as global (or external). Without this
> patch, some non-exported symbols are incorrectly assigned an upper-case designation in
> /proc/kallsyms.  This patch corrects this condition by converting non-exported symbols
> types to lower case when appropriate and eliminates the superfluous upcase_if_global
> function

After looking at this patch I thought "this is all wrong: we should fix 
this at build time and not at run-time". The module infrastructure is a 
little hard to follow, though (Rusty CC'ed).

Anyway, it looks like doing the case correction in add_kallsyms should 
at least avoid the "is_exported" call at every module symbol at every 
"cat /proc/kallsyms". It would be done only once at module loading 
(doing it at build time would be even better, though).

"is_exported" does a linear search inside the symbols of one module, so 
the time it takes to "cat" all the symbols in a module grows O(N^2). 
Although there aren't that many symbols in each module, a quick grep 
shows that "usbcore", for instance, has 876 symbols in /proc/kallsyms.

The st_info field seems to be used only to display the type in 
"/proc/kallsyms" and in mod_find_symname to ignore undefined symbols 
(this would have to be adjusted if the case of the 'U' symbol is changed).

This has actually very little to do with the patch in question, because 
this behavior was already in place before the patch. Maybe some 
benchmarks of "cat /proc/kallsyms" with both the is_exported call and 
without it can show if this really matters in the end.

-- 
Paulo Marques - www.grupopie.com

"The face of a child can say it all, especially the
mouth part of the face."