| Message ID | 20161017100130.GA16013@angband.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 17, 2016 at 1:01 PM, Adam Borowski <kilobyte@angband.pl> wrote: > On Mon, Oct 17, 2016 at 05:59:51PM +1100, Nicholas Piggin wrote: >> On Mon, 17 Oct 2016 08:51:31 +0200 >> Adam Borowski <kilobyte@angband.pl> wrote: > --- /dev/null > +++ b/include/asm-generic/asm-prototypes.h > @@ -0,0 +1,7 @@ > +#include <linux/bitops.h> > +extern void *__memset(void *, int, __kernel_size_t); > +extern void *__memcpy(void *, const void *, __kernel_size_t); > +extern void *__memmove(void *, const void *, __kernel_size_t); > +extern void *memset(void *, int, __kernel_size_t); > +extern void *memcpy(void *, const void *, __kernel_size_t); > +extern void *memmove(void *, const void *, __kernel_size_t); Before too late, those extern keywords aren't needed and only slowdown compilation. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Alexey, On Mon, Oct 17, 2016 at 1:12 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote: > On Mon, Oct 17, 2016 at 1:01 PM, Adam Borowski <kilobyte@angband.pl> wrote: >> On Mon, Oct 17, 2016 at 05:59:51PM +1100, Nicholas Piggin wrote: >>> On Mon, 17 Oct 2016 08:51:31 +0200 >>> Adam Borowski <kilobyte@angband.pl> wrote: > >> --- /dev/null >> +++ b/include/asm-generic/asm-prototypes.h >> @@ -0,0 +1,7 @@ >> +#include <linux/bitops.h> >> +extern void *__memset(void *, int, __kernel_size_t); >> +extern void *__memcpy(void *, const void *, __kernel_size_t); >> +extern void *__memmove(void *, const void *, __kernel_size_t); >> +extern void *memset(void *, int, __kernel_size_t); >> +extern void *memcpy(void *, const void *, __kernel_size_t); >> +extern void *memmove(void *, const void *, __kernel_size_t); > > Before too late, those extern keywords aren't needed and > only slowdown compilation. Do you have any profiling data backing this? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 17, 2016 at 2:17 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Alexey, > > On Mon, Oct 17, 2016 at 1:12 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote: >> On Mon, Oct 17, 2016 at 1:01 PM, Adam Borowski <kilobyte@angband.pl> wrote: >>> On Mon, Oct 17, 2016 at 05:59:51PM +1100, Nicholas Piggin wrote: >>>> On Mon, 17 Oct 2016 08:51:31 +0200 >>>> Adam Borowski <kilobyte@angband.pl> wrote: >> >>> --- /dev/null >>> +++ b/include/asm-generic/asm-prototypes.h >>> @@ -0,0 +1,7 @@ >>> +#include <linux/bitops.h> >>> +extern void *__memset(void *, int, __kernel_size_t); >>> +extern void *__memcpy(void *, const void *, __kernel_size_t); >>> +extern void *__memmove(void *, const void *, __kernel_size_t); >>> +extern void *memset(void *, int, __kernel_size_t); >>> +extern void *memcpy(void *, const void *, __kernel_size_t); >>> +extern void *memmove(void *, const void *, __kernel_size_t); >> >> Before too late, those extern keywords aren't needed and >> only slowdown compilation. > > Do you have any profiling data backing this? It is easy to obtain. Here is top 10 with runtime bigger than 0.50%: _int_malloc ht_lookup_with_hash _cpp_lex_direct cpp_get_token_1 ggc_internal_alloc_statm _int_free malloc_consolidate lex_identifier enter_macro_context c_lex_with_flags bitmap_set_bit malloc grokdeclarator htab_find_slot_with_hash c_lex_one_token _cpp_lex_token pop_scopev c_parser_declspecs Imagine you're a compiler. Those extern's are tokens. They need to be parsed (byte-by-byte), allocated, and, in case of "extern" with prototype, thrown away. They make line longer forcing people to dance around with splitting per 80 characters and generally making everything somewhat slower. Might as well don't new ones. bitops.h is wrong header as well. Why do you need bitops for bunch of function prototypes? -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> +#include <asm/uaccess.h> > +#include <asm/uaccess.h> Included twice. > +#include <asm/string.h> > +#include <asm/page.h> > +#include <asm/checksum.h> > + > +#include <asm-generic/asm-prototypes.h> > + > +#include <asm/page.h> > +#include <asm/pgtable.h> > +#include <asm/special_insns.h> > +#include <asm/preempt.h> No <asm/arch_hweight.h> for __sw_hweight32 and __sw_hweight64 ? -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/17/2016, 12:01 PM, Adam Borowski wrote: > Anyway, here's my stab at x86: Hi, what happened to this? I had to apply this to fix 4.9-pae kernel here. > From db746df65b920591606398b4b244f5b6dc9eea04 Mon Sep 17 00:00:00 2001 > From: Adam Borowski <kilobyte@angband.pl> > Date: Mon, 17 Oct 2016 11:42:35 +0200 > Subject: [PATCH] kbuild: provide include/asm/asm-prototypes.h for x86 > > Nicholas Piggin wrote: >> Architectures will need to have an include/asm/asm-prototypes.h that >> defines or #include<>s C-style prototypes for exported asm functions. >> We can do an asm-generic version for the common ones like memset so >> there's not a lot of pointless duplication there. > > Signed-off-by: Adam Borowski <kilobyte@angband.pl> > --- > arch/x86/include/asm/asm-prototypes.h | 13 +++++++++++++ > include/asm-generic/asm-prototypes.h | 7 +++++++ > 2 files changed, 20 insertions(+) > create mode 100644 arch/x86/include/asm/asm-prototypes.h > create mode 100644 include/asm-generic/asm-prototypes.h > > diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h > new file mode 100644 > index 0000000..072c97c > --- /dev/null > +++ b/arch/x86/include/asm/asm-prototypes.h > @@ -0,0 +1,13 @@ > +#include <asm/ftrace.h> > +#include <asm/uaccess.h> > +#include <asm/uaccess.h> > +#include <asm/string.h> > +#include <asm/page.h> > +#include <asm/checksum.h> > + > +#include <asm-generic/asm-prototypes.h> > + > +#include <asm/page.h> > +#include <asm/pgtable.h> > +#include <asm/special_insns.h> > +#include <asm/preempt.h> > diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h > new file mode 100644 > index 0000000..df13637 > --- /dev/null > +++ b/include/asm-generic/asm-prototypes.h > @@ -0,0 +1,7 @@ > +#include <linux/bitops.h> > +extern void *__memset(void *, int, __kernel_size_t); > +extern void *__memcpy(void *, const void *, __kernel_size_t); > +extern void *__memmove(void *, const void *, __kernel_size_t); > +extern void *memset(void *, int, __kernel_size_t); > +extern void *memcpy(void *, const void *, __kernel_size_t); > +extern void *memmove(void *, const void *, __kernel_size_t); > thanks,
On Fri, Dec 16, 2016 at 11:55 AM, Jiri Slaby <jslaby@suse.cz> wrote: > > what happened to this? I had to apply this to fix 4.9-pae kernel here. Did you actually have to do that? Because a missing CRC shouldn't be fatal in 4.9. What was the failure mode? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/16/2016, 08:57 PM, Linus Torvalds wrote: > On Fri, Dec 16, 2016 at 11:55 AM, Jiri Slaby <jslaby@suse.cz> wrote: >> >> what happened to this? I had to apply this to fix 4.9-pae kernel here. > > Did you actually have to do that? Yes, disk drivers won't load: [ 2.141973] virtio_pci: disagrees about version of symbol mcount [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) [ 2.164547] virtio_pci: disagrees about version of symbol mcount [ 2.166309] virtio_pci: Unknown symbol mcount (err -22) [ 2.180651] virtio_pci: disagrees about version of symbol mcount [ 2.182823] virtio_pci: Unknown symbol mcount (err -22) [ 2.210943] virtio_pci: disagrees about version of symbol mcount [ 2.220097] virtio_pci: Unknown symbol mcount (err -22) [ 2.220173] ata_piix: disagrees about version of symbol mcount [ 2.220174] ata_piix: Unknown symbol mcount (err -22) and whole machine gets stuck with systemd waiting for /dev/sd*. > Because a missing CRC shouldn't be fatal in 4.9. > > What was the failure mode? I am not sure what you mean? The kernel is rpm-ized 4.9 vanilla and this is the config: http://kernel.suse.com/cgit/kernel-source/tree/config/i386/default?h=stable thanks,
On Sat, Dec 17, 2016 at 09:57:47AM +0100, Jiri Slaby wrote: > On 12/16/2016, 08:57 PM, Linus Torvalds wrote: > > On Fri, Dec 16, 2016 at 11:55 AM, Jiri Slaby <jslaby@suse.cz> wrote: > >> > >> what happened to this? I had to apply this to fix 4.9-pae kernel here. > > > > Did you actually have to do that? > > Yes, disk drivers won't load: > [ 2.141973] virtio_pci: disagrees about version of symbol mcount > [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) > and whole machine gets stuck with systemd waiting for /dev/sd*. > > > Because a missing CRC shouldn't be fatal in 4.9. Most of us get just a scary-looking warning, but whatever the problem is for you, it's good to hear this patch works around it. Whatever the long-term solution will be, for 4.10 an updated[1] version of this fix is on kbuild/kbuild (and kbuild/for-next). I guess we'll bother stable@ once it is merged. Note that it handles only x86, there's a bunch of other architectures affected, alpha m68k s390 sparc ia64 might still need fixing. Meow! [1]. Turns out there was a missing symbol on 486; people build-test those but don't try to actually boot, and even when they do, they don't read warnings.
On Sat, Dec 17, 2016 at 12:57 AM, Jiri Slaby <jslaby@suse.cz> wrote: > > Yes, disk drivers won't load: > [ 2.141973] virtio_pci: disagrees about version of symbol mcount > [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) This makes no sense. mcount isn't even one of the symbols that the patch by Adam is touching. There's something else screwed up here. Not to mention that others don't have your issue. Do you have some other hacks in this area? Are you testing actual plain 4.9, or do you (for example) still carry Arnd's patch around that turned out to not work (reverted by f27c2f69cc8e in my tree)? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/18/2016, 12:59 AM, Linus Torvalds wrote: > On Sat, Dec 17, 2016 at 12:57 AM, Jiri Slaby <jslaby@suse.cz> wrote: >> >> Yes, disk drivers won't load: >> [ 2.141973] virtio_pci: disagrees about version of symbol mcount >> [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) > > This makes no sense. > > mcount isn't even one of the symbols that the patch by Adam is touching. asm-prototypes.h in his patch includes asm/ftrace.h, where the function is declared. That should be enough IIUC scripts/Makefile.build. > There's something else screwed up here. Not to mention that others > don't have your issue. I suppose people don't run i386 kernels or have different config. > Do you have some other hacks in this area? Are you testing actual > plain 4.9, or do you (for example) still carry Arnd's patch around > that turned out to not work (reverted by f27c2f69cc8e in my tree)? Not at all. This was plain 4.9 packaged by suse -- only rpm-related fixes. I tried plain 4.9 without rpm right now with the same output: # insmod soundcore.ko [ 31.582326] soundcore: disagrees about version of symbol mcount [ 31.586183] soundcore: Unknown symbol mcount (err -22) insmod: ERROR: could not insert module soundcore.ko: Invalid parameters $ git describe @ v4.9 $ git status HEAD detached at v4.9 thanks,
On 18-12-2016 11:49, Jiri Slaby wrote: > On 12/18/2016, 12:59 AM, Linus Torvalds wrote: >> On Sat, Dec 17, 2016 at 12:57 AM, Jiri Slaby <jslaby@suse.cz> wrote: >>> >>> Yes, disk drivers won't load: >>> [ 2.141973] virtio_pci: disagrees about version of symbol mcount >>> [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) >> >> This makes no sense. >> >> mcount isn't even one of the symbols that the patch by Adam is touching. > > asm-prototypes.h in his patch includes asm/ftrace.h, where the function > is declared. That should be enough IIUC scripts/Makefile.build. > >> There's something else screwed up here. Not to mention that others >> don't have your issue. > > I suppose people don't run i386 kernels or have different config. > >> Do you have some other hacks in this area? Are you testing actual >> plain 4.9, or do you (for example) still carry Arnd's patch around >> that turned out to not work (reverted by f27c2f69cc8e in my tree)? > > Not at all. This was plain 4.9 packaged by suse -- only rpm-related > fixes. I tried plain 4.9 without rpm right now with the same output: > # insmod soundcore.ko > [ 31.582326] soundcore: disagrees about version of symbol mcount > [ 31.586183] soundcore: Unknown symbol mcount (err -22) > insmod: ERROR: could not insert module soundcore.ko: Invalid parameters I hit an mcount issue a while back (years?) which was due to building a driver with gcc v4.x while kernel was built using gcc v4.y. Not claiming that is your issue though. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18.12.2016 13:03, Arend Van Spriel wrote: > On 18-12-2016 11:49, Jiri Slaby wrote: >> On 12/18/2016, 12:59 AM, Linus Torvalds wrote: >>> On Sat, Dec 17, 2016 at 12:57 AM, Jiri Slaby <jslaby@suse.cz> wrote: >>>> >>>> Yes, disk drivers won't load: >>>> [ 2.141973] virtio_pci: disagrees about version of symbol mcount >>>> [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) >>> >>> This makes no sense. >>> >>> mcount isn't even one of the symbols that the patch by Adam is touching. >> >> asm-prototypes.h in his patch includes asm/ftrace.h, where the function >> is declared. That should be enough IIUC scripts/Makefile.build. >> >>> There's something else screwed up here. Not to mention that others >>> don't have your issue. >> >> I suppose people don't run i386 kernels or have different config. >> >>> Do you have some other hacks in this area? Are you testing actual >>> plain 4.9, or do you (for example) still carry Arnd's patch around >>> that turned out to not work (reverted by f27c2f69cc8e in my tree)? >> >> Not at all. This was plain 4.9 packaged by suse -- only rpm-related >> fixes. I tried plain 4.9 without rpm right now with the same output: >> # insmod soundcore.ko >> [ 31.582326] soundcore: disagrees about version of symbol mcount >> [ 31.586183] soundcore: Unknown symbol mcount (err -22) >> insmod: ERROR: could not insert module soundcore.ko: Invalid parameters > > I hit an mcount issue a while back (years?) which was due to building a > driver with gcc v4.x while kernel was built using gcc v4.y. Not claiming > that is your issue though. I've usually had the same thing happen to me if things were compiled with different gcc versions . Essentially in newer gcc (starting with 4.6 I believe) CC_USING_FENTRY is defined, meaning that there is no mcount() symbol but rather __fentry__. This is the likely problem here. > > Regards, > Arend > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/18/2016, 02:27 PM, Nikolay Borisov wrote:
> This is the likely problem here.
No, it is not. How could a rpm be built with two compilers?
Moreover, with some modules, __put_user_1 and others are reported
instead of mcount.
On 18.12.2016 16:45, Jiri Slaby wrote: > Moreover, with some modules, __put_user_1 and others are reported > instead of mcount. nm vmlinux | grep __fentry__ nm vmlinux | grep mcount What do these report ? I bet you that in your vmlinux the first one would return something like : ffffffff822f1810 T __fentry__ ffffffff827fdc20 r __kcrctab___fentry__ ffffffff82809461 r __kstrtab___fentry__ ffffffff827e6c20 R __ksymtab___fentry__ and nothing for the second. Whereas doing nm on the module in question would give nothing for __fentry__ and something like: U mcount -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/18/2016, 03:54 PM, Nikolay Borisov wrote: > > > On 18.12.2016 16:45, Jiri Slaby wrote: >> Moreover, with some modules, __put_user_1 and others are reported >> instead of mcount. > > > nm vmlinux | grep __fentry__ > nm vmlinux | grep mcount > > What do these report ? I bet you that in your vmlinux the first one > would return something like : > > ffffffff822f1810 T __fentry__ > ffffffff827fdc20 r __kcrctab___fentry__ > ffffffff82809461 r __kstrtab___fentry__ > ffffffff827e6c20 R __ksymtab___fentry__ > and nothing for the second. Whereas doing nm on the module in question > would give nothing for __fentry__ and something like: U mcount Well, I have just won a beer: $ nm vmlinux | grep mcount w __crc_mcount c0b3bd34 r __kcrctab_mcount c0b41692 r __kstrtab_mcount c0b2dd04 R __ksymtab_mcount c0896130 T mcount c0c9ee20 T __start_mcount_loc c0cba89c T __stop_mcount_loc $ nm vmlinux | grep __fentry__ $ nm sound/soundcore.ko | grep mcount U mcount No, I am really not stupid. We compile the kernels like this for over a decade and it really broke with 4.9. Applying the patch fixes the problem. Reverting it, makes it recur. regards,
diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h new file mode 100644 index 0000000..072c97c --- /dev/null +++ b/arch/x86/include/asm/asm-prototypes.h @@ -0,0 +1,13 @@ +#include <asm/ftrace.h> +#include <asm/uaccess.h> +#include <asm/uaccess.h> +#include <asm/string.h> +#include <asm/page.h> +#include <asm/checksum.h> + +#include <asm-generic/asm-prototypes.h> + +#include <asm/page.h> +#include <asm/pgtable.h> +#include <asm/special_insns.h> +#include <asm/preempt.h> diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h new file mode 100644 index 0000000..df13637 --- /dev/null +++ b/include/asm-generic/asm-prototypes.h @@ -0,0 +1,7 @@ +#include <linux/bitops.h> +extern void *__memset(void *, int, __kernel_size_t); +extern void *__memcpy(void *, const void *, __kernel_size_t); +extern void *__memmove(void *, const void *, __kernel_size_t); +extern void *memset(void *, int, __kernel_size_t); +extern void *memcpy(void *, const void *, __kernel_size_t); +extern void *memmove(void *, const void *, __kernel_size_t);