On Sun, 2009-01-18 at 19:22 -0700, Grant Grundler wrote: > On Mon, Jan 19, 2009 at 12:07:04PM +1100, Michael Ellerman wrote: > > Weak functions aren't all they're cracked up to be. They lead to > > incorrect binaries with some toolchains, they require us to have empty > > functions we otherwise wouldn't, and the unused code is not elided > > (as of gcc 4.3.2 anyway). > > > > So replace the weak MSI arch hooks with the #define foo foo idiom. We no > > longer need empty versions of arch_setup/teardown_msi_irq(). > > Wait a moment. A week or so ago we just had a thread where I understood > the outcome was to: > > 1) black list known buggy gcc versions for this exact problem. > (http://lkml.org/lkml/2008/12/31/243) > > 2) add a sparse rule to catch other weak symbol abuse. > http://lkml.org/lkml/2008/12/31/189 > > > Start of thread: > http://lkml.org/lkml/2008/12/29/267 > > Linus shot down a particular use of weak symbols: > http://lkml.org/lkml/2008/12/31/174 > > The URL in (2) also includes this output: > drivers/pci/msi.c:49:27: warning: Calling weak function 'arch_setup_msi_irq' in file scope > drivers/pci/msi.c:69:25: warning: Calling weak function 'arch_teardown_msi_irq' in file scope > drivers/pci/msi.c:421:27: warning: Calling weak function 'arch_setup_msi_irqs' in file scope > drivers/pci/msi.c:492:27: warning: Calling weak function 'arch_setup_msi_irqs' in file scope > drivers/pci/msi.c:562:29: warning: Calling weak function 'arch_msi_check_device' in file scope > drivers/pci/msi.c:654:24: warning: Calling weak function 'arch_teardown_msi_irqs' in file scope > > Is this patch a response to fix the sparse warning? No it's not, but if it fixes them that's nice too. > > This is less source (by 1 line!), and results in smaller binaries too: > > > > text data bss dec hex filename > > 9354300 1693916 678424 11726640 b2ef30 build/powerpc/vmlinux-before > > 9354052 1693852 678424 11726328 b2edf8 build/powerpc/vmlinux-after > > The fact that gcc isn't removing unused code sounds like a different bug > and not a kernel problem per se. No it's not a kernel problem, but it's another tick against weak functions in my opinion. > I don't object to this patch in general - it's the same result as far > as I'm concerned. I'd just like a consistent story on "weak symbols". Sure. I wasn't part of the discussion you mentioned, but I don't think this patch is inconsistent with the outcome. The way I see it weak symbols can be useful, but there are several gotchas, and in this particular case we end up with less source & less object code by not using them. cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Attachment:
signature.asc
Description: This is a digitally signed message part
