| Message ID | 7hwq1v4iq4.fsf@deeprootsystems.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 01, 2015 at 02:54:59PM -0700, Kevin Hilman wrote: > Your patch on top of Geert's still compiles fine for me with gcc-4.7.3. > However, I'm not sure how specific we can be on the versions. > > /me goes to test a few more compilers... OK... > > ICE: 4.7.1, 4.7.3, 4.8.3 > OK: 4.6.3, 4.9.2, 4.9.3 > > The diff below[2] on top of yours compiles fine here and at least covers > the compilers I *know* to trigger the ICE. Interesting. I'm using stock gcc 4.7.4 here, though I'm not building -next (only mainline + my tree + arm-soc) and it hasn't shown a problem yet. I think we need to ask the question: is the bug in stock GCC or Linaro GCC? If it's not in stock GCC, then it's a GCC vendor problem :)
Hi Russell, On Wed, Apr 1, 2015 at 11:59 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Wed, Apr 01, 2015 at 02:54:59PM -0700, Kevin Hilman wrote: >> Your patch on top of Geert's still compiles fine for me with gcc-4.7.3. >> However, I'm not sure how specific we can be on the versions. >> >> /me goes to test a few more compilers... OK... >> >> ICE: 4.7.1, 4.7.3, 4.8.3 >> OK: 4.6.3, 4.9.2, 4.9.3 >> >> The diff below[2] on top of yours compiles fine here and at least covers >> the compilers I *know* to trigger the ICE. > > Interesting. I'm using stock gcc 4.7.4 here, though I'm not building > -next (only mainline + my tree + arm-soc) and it hasn't shown a problem > yet. Mainline doesn't fail. > I think we need to ask the question: is the bug in stock GCC or Linaro > GCC? If it's not in stock GCC, then it's a GCC vendor problem :) Can you please try -next (e.g. next-20150320)? make bockw_defconfig make mm/migrate.o Thanks! 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
On Wed, Apr 01 2015 at 15:57 -0600, Kevin Hilman wrote: >Andrew Morton <akpm@linux-foundation.org> writes: > >> On Wed, 01 Apr 2015 10:47:49 +0100 Marc Zyngier <marc.zyngier@arm.com> wrote: >> >>> > -static int unmap_and_move(new_page_t get_new_page, free_page_t put_new_page, >>> > - unsigned long private, struct page *page, int force, >>> > - enum migrate_mode mode) >>> > +static noinline int unmap_and_move(new_page_t get_new_page, >>> > + free_page_t put_new_page, >>> > + unsigned long private, struct page *page, >>> > + int force, enum migrate_mode mode) >>> > { >>> > int rc = 0; >>> > int *result = NULL; >>> > >>> >>> Ouch. That's really ugly. And on 32bit ARM, we end-up spilling half of >>> the parameters on the stack, which is not going to help performance >>> either (not that this would be useful on 32bit ARM anyway...). >>> >>> Any chance you could make this dependent on some compiler detection >>> mechanism? >> >> With my arm compiler (gcc-4.4.4) the patch makes no difference - >> unmap_and_move() isn't being inlined anyway. >> >> How does this look? >> >> Kevin, could you please retest? I might have fat-fingered something... > >Your patch on top of Geert's still compiles fine for me with gcc-4.7.3. >However, I'm not sure how specific we can be on the versions. > >/me goes to test a few more compilers... OK... > >ICE: 4.7.1, 4.7.3, 4.8.3 >OK: 4.6.3, 4.9.2, 4.9.3 > >The diff below[2] on top of yours compiles fine here and at least covers >the compilers I *know* to trigger the ICE. I see ICE on arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.4-2ubuntu1) 4.7.4 > >Kevin > > >[1] >diff --git a/mm/migrate.c b/mm/migrate.c >index 25fd7f6291de..6e15ae3248e0 100644 >--- a/mm/migrate.c >+++ b/mm/migrate.c >@@ -901,10 +901,10 @@ out: > } > > /* >- * gcc-4.7.3 on arm gets an ICE when inlining unmap_and_move(). Work around >+ * gcc 4.7 and 4.8 on arm gets an ICE when inlining unmap_and_move(). Work around > * it. > */ >-#if GCC_VERSION == 40703 && defined(CONFIG_ARM) >+#if (GCC_VERSION >= 40700 && GCC_VERSION < 40900) && defined(CONFIG_ARM) > #define ICE_noinline noinline > #else > #define ICE_noinline > >_______________________________________________ >linux-arm-kernel mailing list >linux-arm-kernel@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Apr 2, 2015 at 12:12 PM, Lina Iyer <lina.iyer@linaro.org> wrote: > On Wed, Apr 01 2015 at 15:57 -0600, Kevin Hilman wrote: >> >> Andrew Morton <akpm@linux-foundation.org> writes: >> >>> On Wed, 01 Apr 2015 10:47:49 +0100 Marc Zyngier <marc.zyngier@arm.com> >>> wrote: >>> >>>> > -static int unmap_and_move(new_page_t get_new_page, free_page_t >>>> > put_new_page, >>>> > - unsigned long private, struct page *page, int >>>> > force, >>>> > - enum migrate_mode mode) >>>> > +static noinline int unmap_and_move(new_page_t get_new_page, >>>> > + free_page_t put_new_page, >>>> > + unsigned long private, struct page >>>> > *page, >>>> > + int force, enum migrate_mode mode) >>>> > { >>>> > int rc = 0; >>>> > int *result = NULL; >>>> > >>>> >>>> Ouch. That's really ugly. And on 32bit ARM, we end-up spilling half of >>>> the parameters on the stack, which is not going to help performance >>>> either (not that this would be useful on 32bit ARM anyway...). >>>> >>>> Any chance you could make this dependent on some compiler detection >>>> mechanism? >>> >>> >>> With my arm compiler (gcc-4.4.4) the patch makes no difference - >>> unmap_and_move() isn't being inlined anyway. >>> >>> How does this look? >>> >>> Kevin, could you please retest? I might have fat-fingered something... >> >> >> Your patch on top of Geert's still compiles fine for me with gcc-4.7.3. >> However, I'm not sure how specific we can be on the versions. >> >> /me goes to test a few more compilers... OK... >> >> ICE: 4.7.1, 4.7.3, 4.8.3 >> OK: 4.6.3, 4.9.2, 4.9.3 >> >> The diff below[2] on top of yours compiles fine here and at least covers >> the compilers I *know* to trigger the ICE. > > > I see ICE on arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.4-2ubuntu1) 4.7.4 > Thanks for checking. I'm assuming my patch fixes it for your since that should catch any 4.7.x compiler. Kevin
On Thu, Apr 02 2015 at 15:12 -0600, Kevin Hilman wrote: >On Thu, Apr 2, 2015 at 12:12 PM, Lina Iyer <lina.iyer@linaro.org> wrote: >> On Wed, Apr 01 2015 at 15:57 -0600, Kevin Hilman wrote: >>> >>> Andrew Morton <akpm@linux-foundation.org> writes: >>> >>>> On Wed, 01 Apr 2015 10:47:49 +0100 Marc Zyngier <marc.zyngier@arm.com> >>>> wrote: >>>> >>>>> > -static int unmap_and_move(new_page_t get_new_page, free_page_t >>>>> > put_new_page, >>>>> > - unsigned long private, struct page *page, int >>>>> > force, >>>>> > - enum migrate_mode mode) >>>>> > +static noinline int unmap_and_move(new_page_t get_new_page, >>>>> > + free_page_t put_new_page, >>>>> > + unsigned long private, struct page >>>>> > *page, >>>>> > + int force, enum migrate_mode mode) >>>>> > { >>>>> > int rc = 0; >>>>> > int *result = NULL; >>>>> > >>>>> >>>>> Ouch. That's really ugly. And on 32bit ARM, we end-up spilling half of >>>>> the parameters on the stack, which is not going to help performance >>>>> either (not that this would be useful on 32bit ARM anyway...). >>>>> >>>>> Any chance you could make this dependent on some compiler detection >>>>> mechanism? >>>> >>>> >>>> With my arm compiler (gcc-4.4.4) the patch makes no difference - >>>> unmap_and_move() isn't being inlined anyway. >>>> >>>> How does this look? >>>> >>>> Kevin, could you please retest? I might have fat-fingered something... >>> >>> >>> Your patch on top of Geert's still compiles fine for me with gcc-4.7.3. >>> However, I'm not sure how specific we can be on the versions. >>> >>> /me goes to test a few more compilers... OK... >>> >>> ICE: 4.7.1, 4.7.3, 4.8.3 >>> OK: 4.6.3, 4.9.2, 4.9.3 >>> >>> The diff below[2] on top of yours compiles fine here and at least covers >>> the compilers I *know* to trigger the ICE. >> >> >> I see ICE on arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.4-2ubuntu1) 4.7.4 >> > >Thanks for checking. I'm assuming my patch fixes it for your since >that should catch any 4.7.x compiler. Yes, thank you. This fixes it on 4.7.4 > >Kevin
Hi Andrew, On Wed, Apr 1, 2015 at 2:54 PM, Kevin Hilman <khilman@kernel.org> wrote: > Andrew Morton <akpm@linux-foundation.org> writes: > >> On Wed, 01 Apr 2015 10:47:49 +0100 Marc Zyngier <marc.zyngier@arm.com> wrote: >> >>> > -static int unmap_and_move(new_page_t get_new_page, free_page_t put_new_page, >>> > - unsigned long private, struct page *page, int force, >>> > - enum migrate_mode mode) >>> > +static noinline int unmap_and_move(new_page_t get_new_page, >>> > + free_page_t put_new_page, >>> > + unsigned long private, struct page *page, >>> > + int force, enum migrate_mode mode) >>> > { >>> > int rc = 0; >>> > int *result = NULL; >>> > >>> >>> Ouch. That's really ugly. And on 32bit ARM, we end-up spilling half of >>> the parameters on the stack, which is not going to help performance >>> either (not that this would be useful on 32bit ARM anyway...). >>> >>> Any chance you could make this dependent on some compiler detection >>> mechanism? >> >> With my arm compiler (gcc-4.4.4) the patch makes no difference - >> unmap_and_move() isn't being inlined anyway. >> >> How does this look? >> >> Kevin, could you please retest? I might have fat-fingered something... > > Your patch on top of Geert's still compiles fine for me with gcc-4.7.3. > However, I'm not sure how specific we can be on the versions. > > /me goes to test a few more compilers... OK... > > ICE: 4.7.1, 4.7.3, 4.8.3 > OK: 4.6.3, 4.9.2, 4.9.3 > > The diff below[2] on top of yours compiles fine here and at least covers > the compilers I *know* to trigger the ICE. I see my fix in your mmots since last Thurs (4/2), but it's not in mmotm (last updated today) so today's linux-next still has the ICE for anything other than gcc-4.7.3. Just checking to see when you plan to update mmotm. Thanks, Kevin
On Tue, 7 Apr 2015 10:57:52 -0700 Kevin Hilman <khilman@kernel.org> wrote: > > The diff below[2] on top of yours compiles fine here and at least covers > > the compilers I *know* to trigger the ICE. > > I see my fix in your mmots since last Thurs (4/2), but it's not in > mmotm (last updated today) so today's linux-next still has the ICE for > anything other than gcc-4.7.3. Just checking to see when you plan to > update mmotm. It should all be there today?
Andrew Morton <akpm@linux-foundation.org> writes: > On Tue, 7 Apr 2015 10:57:52 -0700 Kevin Hilman <khilman@kernel.org> wrote: > >> > The diff below[2] on top of yours compiles fine here and at least covers >> > the compilers I *know* to trigger the ICE. >> >> I see my fix in your mmots since last Thurs (4/2), but it's not in >> mmotm (last updated today) so today's linux-next still has the ICE for >> anything other than gcc-4.7.3. Just checking to see when you plan to >> update mmotm. > > It should all be there today? Nope. In mmotm, only the original patch plus your first fix is there: $ curl -sO http://www.ozlabs.org/~akpm/mmotm/broken-out.tar.gz $ tar -tavf broken-out.tar.gz |grep gcc-473 -rw-r----- akpm/eng 1838 2015-04-01 14:41 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473.patch -rw-r----- akpm/eng 1309 2015-04-01 14:41 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473-fix.patch but in mmots, the additional ptch from me, plus another comment fixup from you are also there: $ curl -sO http://www.ozlabs.org/~akpm/mmots/broken-out.tar.gz $ tar -tavf broken-out.tar.gz |grep gcc-473 -rw-r----- akpm/eng 1882 2015-04-06 16:24 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473.patch -rw-r----- akpm/eng 1271 2015-04-06 16:24 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473-fix.patch -rw-r----- akpm/eng 1382 2015-04-06 16:24 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473-fix-fix.patch -rw-r----- akpm/eng 968 2015-04-06 16:24 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473-fix-fix-fix.patch Kevin
On Tue, 07 Apr 2015 15:41:32 -0700 Kevin Hilman <khilman@kernel.org> wrote: > Andrew Morton <akpm@linux-foundation.org> writes: > > > On Tue, 7 Apr 2015 10:57:52 -0700 Kevin Hilman <khilman@kernel.org> wrote: > > > >> > The diff below[2] on top of yours compiles fine here and at least covers > >> > the compilers I *know* to trigger the ICE. > >> > >> I see my fix in your mmots since last Thurs (4/2), but it's not in > >> mmotm (last updated today) so today's linux-next still has the ICE for > >> anything other than gcc-4.7.3. Just checking to see when you plan to > >> update mmotm. > > > > It should all be there today? > > Nope. huh, I swear I did an mmotm yesterday. Let me see if I can sort out the watchdog mess and produce something releasable...
Andrew Morton <akpm@linux-foundation.org> writes: > On Tue, 07 Apr 2015 15:41:32 -0700 Kevin Hilman <khilman@kernel.org> wrote: > >> Andrew Morton <akpm@linux-foundation.org> writes: >> >> > On Tue, 7 Apr 2015 10:57:52 -0700 Kevin Hilman <khilman@kernel.org> wrote: >> > >> >> > The diff below[2] on top of yours compiles fine here and at least covers >> >> > the compilers I *know* to trigger the ICE. >> >> >> >> I see my fix in your mmots since last Thurs (4/2), but it's not in >> >> mmotm (last updated today) so today's linux-next still has the ICE for >> >> anything other than gcc-4.7.3. Just checking to see when you plan to >> >> update mmotm. >> > >> > It should all be there today? >> >> Nope. > > huh, I swear I did an mmotm yesterday. Well, based on the timestamp of the mmotm dir on ozlabs, it appears you did. That's why I was confused why the gcc-473 patches from mmots aren't there. > Let me see if I can sort out the watchdog mess and produce something > releasable... OK, thanks. Kevin
On Tue, 07 Apr 2015 16:27:44 -0700 Kevin Hilman <khilman@kernel.org> wrote: > >> > It should all be there today? > >> > >> Nope. > > > > huh, I swear I did an mmotm yesterday. > > Well, based on the timestamp of the mmotm dir on ozlabs, it appears you > did. That's why I was confused why the gcc-473 patches from mmots aren't > there. Things look a bit better now.
Andrew Morton <akpm@linux-foundation.org> writes: > On Tue, 07 Apr 2015 16:27:44 -0700 Kevin Hilman <khilman@kernel.org> wrote: > >> >> > It should all be there today? >> >> >> >> Nope. >> > >> > huh, I swear I did an mmotm yesterday. >> >> Well, based on the timestamp of the mmotm dir on ozlabs, it appears you >> did. That's why I was confused why the gcc-473 patches from mmots aren't >> there. > > Things look a bit better now. Yup, I can confirm all 4 patches are there now. Things should be in good shape for the next -next. Thanks, Kevin
diff --git a/mm/migrate.c b/mm/migrate.c index 25fd7f6291de..6e15ae3248e0 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -901,10 +901,10 @@ out: } /* - * gcc-4.7.3 on arm gets an ICE when inlining unmap_and_move(). Work around + * gcc 4.7 and 4.8 on arm gets an ICE when inlining unmap_and_move(). Work around * it. */ -#if GCC_VERSION == 40703 && defined(CONFIG_ARM) +#if (GCC_VERSION >= 40700 && GCC_VERSION < 40900) && defined(CONFIG_ARM) #define ICE_noinline noinline #else #define ICE_noinline