2018-03-07 17:09 GMT+08:00 Petr Mladek <pmla...@suse.com>:
> Ah, there was a mid-air collision. Arnd already answered most of my
> questions and even more.
>
> On Wed 2018-03-07 09:46:27, Arnd Bergmann wrote:
>> On Wed, Mar 7, 2018 at 3:21 AM, Sergey Senozhatsky
>> <sergey.senozhatsky.w...@gmail.com> wrote:
>> > On (03/06/18 14:27), Arnd Bergmann wrote:
>> >> Weak symbols are generally discouraged in the kernel. We have
>> >> them in a couple of places, but I find them rather confusing as they
>> >> make it harder to figure out what is actually going on.
>> >
>> > Honestly, I kind of find __weak less confusing than
>> > EXPORT_SYMBOL(dump_stack)
>> > in 3 different places. __weak hints that the symbol likely will be
>> > overridden
>> > somewhere, while EXPORT_SYMBOL() does not (at least not to me). Dunno.
>>
>> It's not either/or, they are both wrong ;-)
>>
>> The EXPORT_SYMBOL() is not the thing that makes it work. The duplicate
>> declaration today only works because lib/dump_stack.o is listed as lib-y
>> in the Makefile instead of obj-y. On blackfin and nds32, this causes the
>> entire
>> file to just not be included in the final vmlinux, because there are no
>> references to it.
>>
>> With your patch, I would actually expect the lib/dump_stack.o file
>> to still not be picked up, so now you have a missing EXPORT_SYMBOL()
>> on the two unusual architectures until the point when you add another
>> (referenced) symbol to it.
>
> Great catch! We should change it from lib-y to obj-y. Of course,
> the best solution would be if nds32 uses the generic implementation
> and we could avoid adding __weak.
>
I agree too.
I think I can add a patch to remove the dump_stack() implementation in
nds32 and use the generic one.
diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
index 8828b4aeb72b..6e34eb9824a4 100644
--- a/arch/nds32/kernel/traps.c
+++ b/arch/nds32/kernel/traps.c
@@ -156,18 +156,6 @@ static void __dump(struct task_struct *tsk,
unsigned long *base_reg)
pr_emerg("\n");
}
-void dump_stack(void)
-{
- unsigned long *base_reg;
- if (!IS_ENABLED(CONFIG_FRAME_POINTER))
- __asm__ __volatile__("\tori\t%0, $sp, #0\n":"=r"(base_reg));
- else
- __asm__ __volatile__("\tori\t%0, $fp, #0\n":"=r"(base_reg));
- __dump(NULL, base_reg);
-}
-
-EXPORT_SYMBOL(dump_stack);
-
