Re: [PATCH] dump_stack: convert generic dump_stack into a weak symbol

[Petr Mladek <pmladek@suse.com>]

On Wed 2018-03-07 11:21:27, Sergey Senozhatsky wrote:
> Hello Arnd,
> 
> On (03/06/18 14:27), Arnd Bergmann wrote:
> [..]
> > As we are now removing blackfin, based on the latest discussion, this
> > part should no longer be necessary.
> 
> When is this going to happen? 4.17?
> 
> [..]
> > nds32 currently only exists in linux-next, not in the mainline kernel.
> > If it's the only architecture that does something different from everyone
> > else, I think we should change nds32.
> > 
> > I looked at the nds32 show_stack() implementation now and it
> > seems to me that it is completely unnecessary, as the implementation
> > from lib/dump_stack.c does basically the same thing (by calling
> > show_stack(NULL, NULL)).
> 
> Interesting point. I'll leave it to nds32 maintainers.
> OTOH blackfin is still in linux-next, so I assume we need
> that __weak dump_stack() for the time being.

My understanding is that blacfin will not be removed in the first
wave. Therefore we would need to do something about it for 4.17.
Or is there any new info, Arnd?


> [..]
> > > +asmlinkage __weak __visible void dump_stack(void)
> > >  {
> > >         __dump_stack();
> > >  }
> > 
> > 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.

I agree that using weak symbols might be confusing. But I wonder what
is the preferred alternative when only few architectures do something
slightly different.


> 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.

The trick used for dump_stack() is really confusing. I mean the
linking of lib/dump_stack.o only when there is no arch-specific
variant.

Best Regards,
Petr