[PATCH 0/2] kallsyms updates for Blackfin

[PATCH 0/2] kallsyms updates for Blackfin

[Bryan Wu]

These patches were sent out before and acked by you.
Could you please merge these?

Thanks
-Bryan

[PATCH 1/2] kallsyms: Allow kernel symbols in L1 to be found in Blackfin architecture

[Bryan Wu]

From: Robin Getz <robin.getz@analog.com>

Add _stext_l1, _etext_l1 for the L1 memory section in Blackfin.

Signed-off-by: Robin Getz <robin.getz@analog.com>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
 kernel/kallsyms.c  |    6 +++++-
 scripts/kallsyms.c |    8 +++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index f091d13..456d1f6 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -55,7 +55,11 @@ static inline int is_kernel_inittext(unsigned long addr)
 
 static inline int is_kernel_text(unsigned long addr)
 {
-	if (addr >= (unsigned long)_stext && addr <= (unsigned long)_etext)
+	if ((addr >= (unsigned long)_stext && addr <= (unsigned long)_etext)
+#if defined(CONFIG_BLACKFIN)
+             || (addr >= (unsigned long)_stext_l1 && addr <= (unsigned long)_etext_l1)
+#endif
+	)
 		return 1;
 	return in_gate_area_no_task(addr);
 }
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index c912137..0f7269f 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -41,6 +41,7 @@ struct sym_entry {
 static struct sym_entry *table;
 static unsigned int table_size, table_cnt;
 static unsigned long long _text, _stext, _etext, _sinittext, _einittext;
+static unsigned long long _stext_l1, _etext_l1;
 static int all_symbols = 0;
 static char symbol_prefix_char = '\0';
 
@@ -98,6 +99,10 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 		_sinittext = s->addr;
 	else if (strcmp(sym, "_einittext") == 0)
 		_einittext = s->addr;
+	 else if (strcmp(sym, "_stext_l1" ) == 0)
+		_stext_l1 = s->addr;
+	else if (strcmp(sym, "_etext_l1" ) == 0)
+		_etext_l1 = s->addr;
 	else if (toupper(stype) == 'A')
 	{
 		/* Keep these useful absolute symbols */
@@ -160,7 +165,8 @@ static int symbol_valid(struct sym_entry *s)
 	 * and inittext sections are discarded */
 	if (!all_symbols) {
 		if ((s->addr < _stext || s->addr > _etext)
-		    && (s->addr < _sinittext || s->addr > _einittext))
+		    && (s->addr < _sinittext || s->addr > _einittext)
+		    && (s->addr < _stext_l1 || s->addr > _etext_l1))
 			return 0;
 		/* Corner case.  Discard any symbols with the same value as
 		 * _etext _einittext; they can move between pass 1 and 2 when
-- 
1.5.5

[PATCH 2/2] kallsyms: Nuke all ChangeLog, this should be logged by git

[Bryan Wu]

Pointed out by Paulo:
"When I wrote this initially, it was a mistake to add a Changelog
in the first place, but I didn't know better at the time.

If you're going to make changes to this file, please remove all the
Changelog, instead of adding more entries to it. The "Changelog"
should be kept by the version control system, and not the source code itself."

Cc: Paulo Marques <pmarques@grupopie.com>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
 scripts/kallsyms.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 0f7269f..868a4c8 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -7,12 +7,6 @@
  *
  * Usage: nm -n vmlinux | scripts/kallsyms [--all-symbols] > symbols.S
  *
- * ChangeLog:
- *
- * (25/Aug/2004) Paulo Marques <pmarques@grupopie.com>
- *      Changed the compression method from stem compression to "table lookup"
- *      compression
- *
  *      Table compression uses all the unused char codes on the symbols and
  *  maps these to the most used substrings (tokens). For instance, it might
  *  map char code 0xF7 to represent "write_" and then in every symbol where
-- 
1.5.5

Re: [PATCH 0/2] kallsyms updates for Blackfin

[Paulo Marques]

Bryan Wu wrote:
> These patches were sent out before and acked by you.
> Could you please merge these?

I don't actually merge patches... these will have to go through Andrew 
(CC'ed).

Just make sure they are still compatible with the latest -mm tree, 
because there have been a few patches floating around lately that 
touched kallsyms.

-- 
Paulo Marques - www.grupopie.com

"To know recursion, you must first know recursion."

Re: [PATCH 1/2] kallsyms: Allow kernel symbols in L1 to be found in Blackfin architecture

[Paulo Marques]

Bryan Wu wrote:
> From: Robin Getz <robin.getz@analog.com>
> 
> Add _stext_l1, _etext_l1 for the L1 memory section in Blackfin.
> 
> Signed-off-by: Robin Getz <robin.getz@analog.com>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>
> ---

Signed-off-by: Paulo Marques <pmarques@grupopie.com>

-- 
Paulo Marques - www.grupopie.com

"Who is general Failure and why is he reading my disk?"

Re: [PATCH 2/2] kallsyms: Nuke all ChangeLog, this should be logged by git

[Paulo Marques]

Bryan Wu wrote:
> Pointed out by Paulo:
> "When I wrote this initially, it was a mistake to add a Changelog
> in the first place, but I didn't know better at the time.
> 
> If you're going to make changes to this file, please remove all the
> Changelog, instead of adding more entries to it. The "Changelog"
> should be kept by the version control system, and not the source code itself."
> 
> Cc: Paulo Marques <pmarques@grupopie.com>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>
> ---

Signed-off-by: Paulo Marques <pmarques@grupopie.com>

-- 
Paulo Marques - www.grupopie.com

"God is love. Love is blind. Ray Charles is blind. Ray Charles is God."

Re: [PATCH 1/2] kallsyms: Allow kernel symbols in L1 to be found in Blackfin architecture

[Andrew Morton]

On Fri, 25 Apr 2008 18:28:18 +0800
Bryan Wu <cooloney@kernel.org> wrote:

> From: Robin Getz <robin.getz@analog.com>
> 
> Add _stext_l1, _etext_l1 for the L1 memory section in Blackfin.
> 
> Signed-off-by: Robin Getz <robin.getz@analog.com>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>
> ---
>  kernel/kallsyms.c  |    6 +++++-
>  scripts/kallsyms.c |    8 +++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index f091d13..456d1f6 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -55,7 +55,11 @@ static inline int is_kernel_inittext(unsigned long addr)
>  
>  static inline int is_kernel_text(unsigned long addr)
>  {
> -	if (addr >= (unsigned long)_stext && addr <= (unsigned long)_etext)
> +	if ((addr >= (unsigned long)_stext && addr <= (unsigned long)_etext)
> +#if defined(CONFIG_BLACKFIN)
> +             || (addr >= (unsigned long)_stext_l1 && addr <= (unsigned long)_etext_l1)
> +#endif
> +	)

That's a bit grubby.  I suppose we could pretend not to have noticed, but
this really isn't the preferred way of putting arch-specific hooks into
arch-neutral code.

Nicer might be to add

void __weak arch_is_kernel_text(unsigned long addr)
{
	return 0;
}

with a declaration in linux/kallsyms.h and then override
arch_is_kernel_text() in arch/blackfin/ code.


>  		return 1;
>  	return in_gate_area_no_task(addr);
>  }
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index c912137..0f7269f 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -41,6 +41,7 @@ struct sym_entry {
>  static struct sym_entry *table;
>  static unsigned int table_size, table_cnt;
>  static unsigned long long _text, _stext, _etext, _sinittext, _einittext;
> +static unsigned long long _stext_l1, _etext_l1;
>  static int all_symbols = 0;
>  static char symbol_prefix_char = '\0';
>  
> @@ -98,6 +99,10 @@ static int read_symbol(FILE *in, struct sym_entry *s)
>  		_sinittext = s->addr;
>  	else if (strcmp(sym, "_einittext") == 0)
>  		_einittext = s->addr;
> +	 else if (strcmp(sym, "_stext_l1" ) == 0)
        ^ stray space
                                         ^ and another one

> +		_stext_l1 = s->addr;
> +	else if (strcmp(sym, "_etext_l1" ) == 0)
> +		_etext_l1 = s->addr;
>  	else if (toupper(stype) == 'A')
>  	{
>  		/* Keep these useful absolute symbols */
> @@ -160,7 +165,8 @@ static int symbol_valid(struct sym_entry *s)
>  	 * and inittext sections are discarded */
>  	if (!all_symbols) {
>  		if ((s->addr < _stext || s->addr > _etext)
> -		    && (s->addr < _sinittext || s->addr > _einittext))
> +		    && (s->addr < _sinittext || s->addr > _einittext)
> +		    && (s->addr < _stext_l1 || s->addr > _etext_l1))

so... for non-blackfin, the above addition is equivalent to

	&& (s->addr < 0 || s->addr > 0)

which is always true, except for s->addr==0, in which case we just broke
it.

I guess addr==0 isn't an interesting case here.

Re: [PATCH 1/2] kallsyms: Allow kernel symbols in L1 to be found in Blackfin architecture

[Robin Getz]

On Tue 29 Apr 2008 18:00, Andrew Morton pondered: 
> That's a bit grubby.  I suppose we could pretend not to have noticed,
> but this really isn't the preferred way of putting arch-specific hooks into
> arch-neutral code.
> 
> Nicer might be to add
> 
> void __weak arch_is_kernel_text(unsigned long addr)
> {
> 	return 0;
> }
> 
> with a declaration in linux/kallsyms.h and then override
> arch_is_kernel_text() in arch/blackfin/ code.

I will refactor, and send again.

-Robin

Re: [PATCH 1/2] kallsyms: Allow kernel symbols in L1 to be found in Blackfin architecture

[Andrew Morton]

On Wed, 30 Apr 2008 09:51:43 -0400 Robin Getz <rgetz@blackfin.uclinux.org> wrote:

> On Tue 29 Apr 2008 18:00, Andrew Morton pondered: 
> > That's a bit grubby.  I suppose we could pretend not to have noticed,
> > but this really isn't the preferred way of putting arch-specific hooks into
> > arch-neutral code.
> > 
> > Nicer might be to add
> > 
> > void __weak arch_is_kernel_text(unsigned long addr)
> > {
> > 	return 0;
> > }
> > 
> > with a declaration in linux/kallsyms.h and then override
> > arch_is_kernel_text() in arch/blackfin/ code.
> 
> I will refactor, and send again.
> 

Thanks.

There are of course lots of other ways of doing it, apart from __weak. 
Many involve unpleasing ARCH_HAVE_FOO things.

Possibly cleaner would be, in kallsyms.c:

#ifndef arch_is_kernel_text
static inline int arch_is_kernel_text(unsigned long addr)
{
	return 0;
}
#define arch_is_kernel_text(addr) arch_is_kernel_text(addr)	// not really needed
#endif



and then, in a blackfin header file do

extern int arch_is_kernel_text(unsigned long addr);
#define arch_is_kernel_text(addr) arch_is_kernel_text(addr)


But personally I find all these party tricks a bit unpleasant.  The
definitive approach to implementing a per-arch kernel->arch interface is to
just implement it, dammit.  That means adding

static inline int arch_is_kernel_text(unsigned long addr)
{
	return 0;
}

to every architecture's include/asm/kallsyms.h, then doing blackfin's one
differently.

Verbose, but complete.