public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH*] show last kernel-image symbol in /proc/kallsyms
@ 2004-05-10  0:14 Randy.Dunlap
  2004-05-10  4:14 ` Rusty Russell
  0 siblings, 1 reply; 5+ messages in thread
From: Randy.Dunlap @ 2004-05-10  0:14 UTC (permalink / raw)
  To: lkml; +Cc: zwane, rusty


'cat' or 'tail' of /proc/kallsyms (2.6.6-rc2 or -rc3, & probably much
earlier) does not include the last kernel-image symbol (_einittext).

_einittext is the last symbol generated in .tmp_kallsyms2.S
and the symbol count in that file also appears to be correct,
but the iterator code for /proc/kallsyms comes up 1 short somehow.

Here are 2 patches.  Either one of them "fixes" the problem.
Neither of them is the correct fix AFAIK.
Any other suggestions for fixes?

Thanks,
--
~Randy



// linux-266-rc3
// print the last (kernel image) symbol of /proc/kallsyms
// by making the valid symbol count 1 larger than symbols found;

diffstat:=
 scripts/kallsyms.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)


diff -Naurp ./scripts/kallsyms.c~incr_count ./scripts/kallsyms.c
--- ./scripts/kallsyms.c~incr_count	2004-04-03 19:38:23.000000000 -0800
+++ ./scripts/kallsyms.c	2004-05-09 16:33:07.000000000 -0700
@@ -127,7 +127,7 @@ write_src(void)
 	printf(".globl kallsyms_num_syms\n");
 	printf("\tALGN\n");
 	printf("kallsyms_num_syms:\n");
-	printf("\tPTR\t%d\n", valid);
+	printf("\tPTR\t%d\n", valid + 1);
 	printf("\n");
 
 	printf(".globl kallsyms_names\n");



// linux-266-rc3
// print the last (kernel image) symbol of /proc/kallsyms
// by using '>' instead of '>=' in the limiting test
// of the iterator.

diffstat:=
 kernel/kallsyms.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)


diff -Naurp ./kernel/kallsyms.c~use_gtr_not_geq ./kernel/kallsyms.c
--- ./kernel/kallsyms.c~use_gtr_not_geq	2004-04-03 19:38:21.000000000 -0800
+++ ./kernel/kallsyms.c	2004-05-09 16:00:11.000000000 -0700
@@ -199,7 +199,7 @@ static void reset_iter(struct kallsym_it
 static int update_iter(struct kallsym_iter *iter, loff_t pos)
 {
 	/* Module symbols can be accessed randomly. */
-	if (pos >= kallsyms_num_syms) {
+	if (pos > kallsyms_num_syms) {
 		iter->pos = pos;
 		return get_ksymbol_mod(iter);
 	}

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH*] show last kernel-image symbol in /proc/kallsyms
  2004-05-10  0:14 [PATCH*] show last kernel-image symbol in /proc/kallsyms Randy.Dunlap
@ 2004-05-10  4:14 ` Rusty Russell
  2004-05-10 17:56   ` Randy.Dunlap
  0 siblings, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2004-05-10  4:14 UTC (permalink / raw)
  To: Randy.Dunlap; +Cc: lkml - Kernel Mailing List, Zwane Mwaikambo

On Mon, 2004-05-10 at 10:14, Randy.Dunlap wrote:
> 'cat' or 'tail' of /proc/kallsyms (2.6.6-rc2 or -rc3, & probably much
> earlier) does not include the last kernel-image symbol (_einittext).
> 
> _einittext is the last symbol generated in .tmp_kallsyms2.S
> and the symbol count in that file also appears to be correct,
> but the iterator code for /proc/kallsyms comes up 1 short somehow.
> 
> Here are 2 patches.  Either one of them "fixes" the problem.
> Neither of them is the correct fix AFAIK.

Ah, I see you are a student of the Morton school of patch extraction. 
Well, it worked.

Name: Show Last Symbol in /proc/kallsyms
Status: Tested on 2.6.6-rc3.bk11

The current code doesn't show the last symbol (usually _einittext) in
/proc/kallsyms.  The reason for this is subtle: s_start() returns an
empty string for position 0 (ignored by s_show()), and s_next()
returns the first symbol for position 1.

What should happen is that update_iter() for position 0 should fill in
the first symbol.  Unfortunately, the get_ksymbol_core() fills in the
symbol information, *and* updates the iterator: we have to split these
functions, which we do by making it return the length of the name
offset.

Then we can call get_ksymbol_core() without moving the iterator,
meaning that we can call it at position 0 (ie. s_start()).

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.6.6-rc3-bk11/kernel/kallsyms.c working-2.6.6-rc3-bk11/kernel/kallsyms.c
--- linux-2.6.6-rc3-bk11/kernel/kallsyms.c	2004-03-12 07:57:28.000000000 +1100
+++ working-2.6.6-rc3-bk11/kernel/kallsyms.c	2004-05-10 13:11:06.000000000 +1000
@@ -171,21 +171,23 @@ static int get_ksymbol_mod(struct kallsy
 	return 1;
 }
 
-static void get_ksymbol_core(struct kallsym_iter *iter)
+/* Returns space to next name. */
+static unsigned long get_ksymbol_core(struct kallsym_iter *iter)
 {
-	unsigned stemlen;
+	unsigned stemlen, off = iter->nameoff;
 
 	/* First char of each symbol name indicates prefix length
 	   shared with previous name (stem compression). */
-	stemlen = kallsyms_names[iter->nameoff++];
+	stemlen = kallsyms_names[off++];
 
-	strlcpy(iter->name+stemlen, kallsyms_names+iter->nameoff, 128-stemlen);
-	iter->nameoff += strlen(kallsyms_names + iter->nameoff) + 1;
+	strlcpy(iter->name+stemlen, kallsyms_names + off, 128-stemlen);
+	off += strlen(kallsyms_names + off) + 1;
 	iter->owner = NULL;
 	iter->value = kallsyms_addresses[iter->pos];
 	iter->type = 't';
 
 	upcase_if_global(iter);
+	return off - iter->nameoff;
 }
 
 static void reset_iter(struct kallsym_iter *iter)
@@ -210,16 +212,16 @@ static int update_iter(struct kallsym_it
 
 	/* We need to iterate through the previous symbols: can be slow */
 	for (; iter->pos != pos; iter->pos++) {
-		get_ksymbol_core(iter);
+		iter->nameoff += get_ksymbol_core(iter);
 		cond_resched();
 	}
+	get_ksymbol_core(iter);
 	return 1;
 }
 

-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH*] show last kernel-image symbol in /proc/kallsyms
  2004-05-10  4:14 ` Rusty Russell
@ 2004-05-10 17:56   ` Randy.Dunlap
  2004-05-10 23:24     ` Randy.Dunlap
  0 siblings, 1 reply; 5+ messages in thread
From: Randy.Dunlap @ 2004-05-10 17:56 UTC (permalink / raw)
  To: Rusty Russell; +Cc: linux-kernel, zwane

On Mon, 10 May 2004 14:14:10 +1000 Rusty Russell wrote:

| On Mon, 2004-05-10 at 10:14, Randy.Dunlap wrote:
| > 'cat' or 'tail' of /proc/kallsyms (2.6.6-rc2 or -rc3, & probably much
| > earlier) does not include the last kernel-image symbol (_einittext).
| > 
| > _einittext is the last symbol generated in .tmp_kallsyms2.S
| > and the symbol count in that file also appears to be correct,
| > but the iterator code for /proc/kallsyms comes up 1 short somehow.
| > 
| > Here are 2 patches.  Either one of them "fixes" the problem.
| > Neither of them is the correct fix AFAIK.
| 
| Ah, I see you are a student of the Morton school of patch extraction. 
| Well, it worked.

Well.... yes.

And thanks for taking time to fix it.
It now works as expected.  :)

'patch' complains about a malformed input file (the @@ line counts
don't match the patch content).
Correction is below.  Or were more patch lines needed (& missing)?


| Name: Show Last Symbol in /proc/kallsyms
| Status: Tested on 2.6.6-rc3.bk11
| 
| The current code doesn't show the last symbol (usually _einittext) in
| /proc/kallsyms.  The reason for this is subtle: s_start() returns an
| empty string for position 0 (ignored by s_show()), and s_next()
| returns the first symbol for position 1.
| 
| What should happen is that update_iter() for position 0 should fill in
| the first symbol.  Unfortunately, the get_ksymbol_core() fills in the
| symbol information, *and* updates the iterator: we have to split these
| functions, which we do by making it return the length of the name
| offset.
| 
| Then we can call get_ksymbol_core() without moving the iterator,
| meaning that we can call it at position 0 (ie. s_start()).
| 
| diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.6.6-rc3-bk11/kernel/kallsyms.c working-2.6.6-rc3-bk11/kernel/kallsyms.c
| --- linux-2.6.6-rc3-bk11/kernel/kallsyms.c	2004-03-12 07:57:28.000000000 +1100
| +++ working-2.6.6-rc3-bk11/kernel/kallsyms.c	2004-05-10 13:11:06.000000000 +1000
| @@ -210,16 +212,16 @@ static int update_iter(struct kallsym_it
***** should be:  @@ -210,9 +212,10 @@

|  
|  	/* We need to iterate through the previous symbols: can be slow */
|  	for (; iter->pos != pos; iter->pos++) {
| -		get_ksymbol_core(iter);
| +		iter->nameoff += get_ksymbol_core(iter);
|  		cond_resched();
|  	}
| +	get_ksymbol_core(iter);
|  	return 1;
|  }
|  

--
~Randy
(Again.  Sometimes I think ln -s /usr/src/linux/.config .signature) -- akpm

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH*] show last kernel-image symbol in /proc/kallsyms
  2004-05-10 17:56   ` Randy.Dunlap
@ 2004-05-10 23:24     ` Randy.Dunlap
  2004-05-11  2:50       ` Rusty Russell
  0 siblings, 1 reply; 5+ messages in thread
From: Randy.Dunlap @ 2004-05-10 23:24 UTC (permalink / raw)
  To: lkml, akpm; +Cc: rusty, zwane, sam

On Mon, 10 May 2004 10:56:06 -0700 Randy.Dunlap wrote:

| On Mon, 10 May 2004 14:14:10 +1000 Rusty Russell wrote:
| 
| | On Mon, 2004-05-10 at 10:14, Randy.Dunlap wrote:
| | > 'cat' or 'tail' of /proc/kallsyms (2.6.6-rc2 or -rc3, & probably much
| | > earlier) does not include the last kernel-image symbol (_einittext).
| | > 
| | > _einittext is the last symbol generated in .tmp_kallsyms2.S
| | > and the symbol count in that file also appears to be correct,
| | > but the iterator code for /proc/kallsyms comes up 1 short somehow.
| | > 
| | > Here are 2 patches.  Either one of them "fixes" the problem.
| | > Neither of them is the correct fix AFAIK.
| | 
| | Ah, I see you are a student of the Morton school of patch extraction. 
| | Well, it worked.
| 
| Well.... yes.
| 
| And thanks for taking time to fix it.
| It now works as expected.  :)

Welllll, almost as expected.  I now see _einittext as hoped and
expected.  However, sometimes I don't see _etext... if it has the
same address as another (previous) symbol, which it can.

so I think that you may want this kind of patch, especially for
CONFIG_KALLSYMS_ALL.

Please apply/merge.

--
~Randy



// linux-266
// Always include the _etext symbol in /proc/kallsyms.

diffstat:=
 scripts/kallsyms.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)


diff -Naurp ./scripts/kallsyms.c~force_etext ./scripts/kallsyms.c
--- ./scripts/kallsyms.c~force_etext	2004-05-09 19:33:21.000000000 -0700
+++ ./scripts/kallsyms.c	2004-05-10 16:01:02.000000000 -0700
@@ -115,7 +115,10 @@ write_src(void)
 		if (!symbol_valid(&table[i]))
 			continue;
 		
-		if (table[i].addr == last_addr)
+		if (table[i].addr == last_addr &&
+		    last_addr != _etext)
+			/* don't duplicate addresses, except always
+			 * make sure that _etext is in kallsyms */
 			continue;
 
 		printf("\tPTR\t%#llx\n", table[i].addr);
@@ -140,7 +143,8 @@ write_src(void)
 		if (!symbol_valid(&table[i]))
 			continue;
 		
-		if (table[i].addr == last_addr)
+		if (table[i].addr == last_addr &&
+		    last_addr != _etext)
 			continue;
 
 		for (k = 0; table[i].sym[k] && table[i].sym[k] == prev[k]; ++k)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH*] show last kernel-image symbol in /proc/kallsyms
  2004-05-10 23:24     ` Randy.Dunlap
@ 2004-05-11  2:50       ` Rusty Russell
  0 siblings, 0 replies; 5+ messages in thread
From: Rusty Russell @ 2004-05-11  2:50 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: lkml - Kernel Mailing List, akpm, Zwane Mwaikambo, Sam Ravnborg

On Tue, 2004-05-11 at 09:24, Randy.Dunlap wrote:
> On Mon, 10 May 2004 10:56:06 -0700 Randy.Dunlap wrote:
> | And thanks for taking time to fix it.
> | It now works as expected.  :)
> 
> Welllll, almost as expected.  I now see _einittext as hoped and
> expected.  However, sometimes I don't see _etext... if it has the
> same address as another (previous) symbol, which it can.

I've been reconsidering this optimization anyway: there are only a few
symbols which are aliases for other symbols.

How's this?

Rusty.

Name: Include Aliases in kallsyms
Status: Booted on 2.6.6-mm1
Version: -mm

Kallsyms discards symbols with the same address, but these are
sometimes useful.  Skip this minor optimization and make
kallsyms_lookup deal with aliases

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .31104-linux-2.6.6-mm1/kernel/kallsyms.c .31104-linux-2.6.6-mm1.updated/kernel/kallsyms.c
--- .31104-linux-2.6.6-mm1/kernel/kallsyms.c	2004-03-12 07:57:28.000000000 +1100
+++ .31104-linux-2.6.6-mm1.updated/kernel/kallsyms.c	2004-05-11 10:42:52.000000000 +1000
@@ -88,14 +88,20 @@ const char *kallsyms_lookup(unsigned lon
 			name += strlen(name) + 1;
 		}
 
-		/* Base symbol size on next symbol. */
-		if (best + 1 < kallsyms_num_syms)
-			symbol_end = kallsyms_addresses[best + 1];
-		else if (is_kernel_inittext(addr))
+		/* At worst, symbol ends at end of section. */
+		if (is_kernel_inittext(addr))
 			symbol_end = (unsigned long)_einittext;
 		else
 			symbol_end = (unsigned long)_etext;
 
+		/* Search for next non-aliased symbol */
+		for (i = best+1; i < kallsyms_num_syms; i++) {
+			if (kallsyms_addresses[i] > kallsyms_addresses[best]) {
+				symbol_end = kallsyms_addresses[i];
+				break;
+			}
+		}
+
 		*symbolsize = symbol_end - kallsyms_addresses[best];
 		*modname = NULL;
 		*offset = addr - kallsyms_addresses[best];
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .31104-linux-2.6.6-mm1/scripts/kallsyms.c .31104-linux-2.6.6-mm1.updated/scripts/kallsyms.c
--- .31104-linux-2.6.6-mm1/scripts/kallsyms.c	2003-09-22 10:07:21.000000000 +1000
+++ .31104-linux-2.6.6-mm1.updated/scripts/kallsyms.c	2004-05-11 10:39:14.000000000 +1000
@@ -93,7 +93,6 @@ read_map(FILE *in)
 static void
 write_src(void)
 {
-	unsigned long long last_addr;
 	int i, valid = 0;
 	char *prev;
 
@@ -111,16 +110,12 @@ write_src(void)
 	printf(".globl kallsyms_addresses\n");
 	printf("\tALGN\n");
 	printf("kallsyms_addresses:\n");
-	for (i = 0, last_addr = 0; i < cnt; i++) {
+	for (i = 0; i < cnt; i++) {
 		if (!symbol_valid(&table[i]))
 			continue;
-		
-		if (table[i].addr == last_addr)
-			continue;
 
 		printf("\tPTR\t%#llx\n", table[i].addr);
 		valid++;
-		last_addr = table[i].addr;
 	}
 	printf("\n");
 
@@ -134,20 +129,16 @@ write_src(void)
 	printf("\tALGN\n");
 	printf("kallsyms_names:\n");
 	prev = ""; 
-	for (i = 0, last_addr = 0; i < cnt; i++) {
+	for (i = 0; i < cnt; i++) {
 		int k;
 
 		if (!symbol_valid(&table[i]))
 			continue;
-		
-		if (table[i].addr == last_addr)
-			continue;
 
 		for (k = 0; table[i].sym[k] && table[i].sym[k] == prev[k]; ++k)
 			; 
 
 		printf("\t.byte 0x%02x\n\t.asciz\t\"%s\"\n", k, table[i].sym + k);
-		last_addr = table[i].addr;
 		prev = table[i].sym;
 	}
 	printf("\n");

-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2004-05-11  2:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-05-10  0:14 [PATCH*] show last kernel-image symbol in /proc/kallsyms Randy.Dunlap
2004-05-10  4:14 ` Rusty Russell
2004-05-10 17:56   ` Randy.Dunlap
2004-05-10 23:24     ` Randy.Dunlap
2004-05-11  2:50       ` Rusty Russell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox