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

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

On (03/07/18 09:46), Arnd Bergmann wrote:
> >
> > When is this going to happen? 4.17?
> 
> Originally I planned to wait a few more releases, but the last maintainer
> has commented that he will now send a patch for immediate removal,
> so 4.17 is almost certain at this point.

Would be great to get it removed as soon as possible then. Otherwise we
will get broken blackfin build errors from Stephen (or would need to hold
off Dave's patch).

> > [..]
> I did the review of all the nds32 patches, and would have commented
> on this if I had noticed it earlier. I see no reason not to change it,
> and would suggest that you continue under the assumption that
> nds32 is going to be fixed, leaving it up to Greentime to add a fix
> to his tree before he sends the pull request.

Good.

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

Yeah.
But, admittedly, I learned that "obj-y vs lib-y" stuff quite recently.

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

Interesting point. Didn't check it. But I checked that we have at least
one reference to lib/dump_stack from every arch so __weak could work its
magic. The function is show_regs_print_info(). AFAICT, every arch calls
it (we have it in lib/dump_stack now, so we will link with lib/dump_stack).
Anyway, I'll be happy to drop my patch. Thanks for taking a look.

	-ss