Re: linux-next: build failure after merge of the origin tree
Hi all, On Fri, 31 Jul 2020 08:58:36 +1000 Stephen Rothwell wrote: > > After merging the origin tree, today's linux-next build (arm > multi_v7_defconfig) failed like this: > > In file included from include/linux/random.h:14, > from arch/arm/kernel/signal.c:8: > arch/arm/include/asm/percpu.h: In function '__my_cpu_offset': > arch/arm/include/asm/percpu.h:29:34: error: 'current_stack_pointer' > undeclared (first use in this function); did you mean 'user_stack_pointer'? >29 | : "Q" (*(const unsigned long *)current_stack_pointer)); > | ^ > | user_stack_pointer > > Presumably aused by commit > > 1c9df907da83 ("random: fix circular include dependency on arm64 after > addition of percpu.h") > > I have added this fix patch for today: Looks like I sould read further ahead in my email :-) -- Cheers, Stephen Rothwell pgpBsgtY4HeLL.pgp Description: OpenPGP digital signature
Re: linux-next: build failure after merge of the origin tree
On Thu, Jul 30, 2020 at 11:24:44AM -0700, Linus Torvalds wrote: > On Wed, Jul 29, 2020 at 8:17 PM Kees Cook wrote: > > > > I'll look into this more tomorrow. (But yes, __latent_entropy is > > absolutely used for globals already, as you found, but this is the first > > percpu it was applied to...) > > Note that it was always per-cpu. > > The only thing that changed was that it was declared static in > lib/random.c vs being externally visible. Yup, thanks. I realized that a bit after sending my email. :) > Unrelated side note: I notice that the plugins could be simplified a > bit now that we require gcc 4.9 or later. There's a fair amount of > cruft for the earlier gcc versions. Yup -- Masahiro keeps poking the build system, but I haven't cleaned up the header file macros to keep up with the recent jumps. (It falls a bit low on my TODO list since it's a bit of a mechanical cleanup. I'm open to anyone that would like to send patches, though!) > I'm not sure how seriously the gcc plugins are actually maintained (no > offense) aside from just keeping them limping along. Does anybody > actually use them in production? I thought google had mostly moved on > to clang. They're part of regular testing, and there is ongoing development (e.g. see Alex Popov's recent series[1], which is in -next waiting for the v5.9 merge window). I hear regularly from folks using randstruct, stackleak, structleak, and latent_entropy. But yes, Google has moved to Clang where we're using Clang's implementation of structleak (auto-var-init) but there has been work to get randstruct ported (as desired by at least one Android vendor), though it's currently stalled. -Kees [1] https://lore.kernel.org/lkml/20200624123330.83226-1-alex.po...@linux.com/ -- Kees Cook
Re: linux-next: build failure after merge of the origin tree
On Wed, Jul 29, 2020 at 8:17 PM Kees Cook wrote: > > I'll look into this more tomorrow. (But yes, __latent_entropy is > absolutely used for globals already, as you found, but this is the first > percpu it was applied to...) Note that it was always per-cpu. The only thing that changed was that it was declared static in lib/random.c vs being externally visible. So it's not about the percpu part - although that then showed the arm64 circular include file problem. It's literally that now the exact same thing is declared in a header file and not marked "static". Now, I don't think the __latent_entropy code ever really worked all that well for per-cpu initializations. It ends up generating one single initializer, which obviously isn't optimal. But I guess it's as good as it gets. Unrelated side note: I notice that the plugins could be simplified a bit now that we require gcc 4.9 or later. There's a fair amount of cruft for the earlier gcc versions. I'm not sure how seriously the gcc plugins are actually maintained (no offense) aside from just keeping them limping along. Does anybody actually use them in production? I thought google had mostly moved on to clang. Linus
Re: linux-next: build failure after merge of the origin tree
On Thu, Jul 30, 2020 at 08:14:07AM +0200, Willy Tarreau wrote: > On Thu, Jul 30, 2020 at 05:22:50AM +0200, Willy Tarreau wrote: > > On Wed, Jul 29, 2020 at 08:17:48PM -0700, Kees Cook wrote: > > > And just another heads-up, the patch[1] (which was never sent to a public > > > list) also breaks arm64 (circular header needs?): > > (...) > > > > Definitely, we've just got a report about this, I'll have a look once > > I'm at the office. I'd like to check that we don't obviously break > > another arch by just removing percpu. If at least shuffling them around > > is sufficient that'd be nice. Otherwise we'll likely need a separate > > header (which is not a bad thing for the long term). > > So Linus proposed a clean solution which might be harder to backport > but looks better for 5.8. However the attached one addresses the issue > for me on arm64 and still works on x86_64, arm, mips. I think we should > go with this one first then apply Linus' one on top of it to be long > term proof, and backport only the first one. Linus ? > > Willy > From 18fba9e2dfb16605a722e01f95d9e2d020efaa42 Mon Sep 17 00:00:00 2001 > From: Willy Tarreau > Date: Thu, 30 Jul 2020 07:59:24 +0200 > Subject: random: fix circular include dependency on arm64 after addition of > percpu.h > MIME-Version: 1.0 > Content-Type: text/plain; charset=latin1 > Content-Transfer-Encoding: 8bit > > Daniel Díaz and Kees Cook independently reported that commit f227e3ec3b5c > ("random32: update the net random state on interrupt and activity") broke > arm64 due to a circular dependency on include files since the addition of > percpu.h in random.h. > > The correct fix would definitely be to move all the prandom32 stuff out > of random.h but for backporting, a smaller solution is preferred. This > one replaces linux/percpu.h with asm/percpu.h, and this fixes the problem > on x86_64, arm64, arm, and mips. Note that moving percpu.h around didn't > change anything and that removing it entirely broke differently. When > backporting, such options might still be considered if this patch fails > to help. > > Reported-by: Daniel Díaz > Reported-by: Kees Cook FWIW, I was only a messenger. Sami (in Cc) pointed it out to me right before I got the email from Linus for the x86 plugin breakage. :) But yes, thanks, this seems to work for me. > Fixes: f227e3ec3b5c nit: Fixes: f227e3ec3b5c ("random32: update the net random state on interrupt and activity") -Kees > Cc: Stephen Rothwell > Cc: Linus Torvalds > Signed-off-by: Willy Tarreau > --- > include/linux/random.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/random.h b/include/linux/random.h > index f310897f051d..9ab7443bd91b 100644 > --- a/include/linux/random.h > +++ b/include/linux/random.h > @@ -11,7 +11,7 @@ > #include > #include > #include > -#include > +#include > > #include > > -- > 2.20.1 > -- Kees Cook
Re: linux-next: build failure after merge of the origin tree
On Thu, Jul 30, 2020 at 11:09:23AM +0100, Catalin Marinas wrote: > On Thu, Jul 30, 2020 at 10:59:09AM +0100, Marc Zyngier wrote: > > From 33d819f4efa0a4474b5dc2e4bcaef1b886ca30c3 Mon Sep 17 00:00:00 2001 > > From: Marc Zyngier > > Date: Thu, 30 Jul 2020 10:53:05 +0100 > > Subject: [PATCH] arm64: Drop unnecessary include from asm/smp.h > > > > asm/pointer_auth.h is not needed anymore in asm/smp.h, as 62a679cb2825 > > ("arm64: simplify ptrauth initialization") removed the keys from the > > secondary_data structure. > > > > This also cures a compilation issue introduced by f227e3ec3b5c > > ("random32: update the net random state on interrupt and activity"). > > > > Fixes: 62a679cb2825 ("arm64: simplify ptrauth initialization") > > Fixes: f227e3ec3b5c ("random32: update the net random state on interrupt and > > activity") > > Signed-off-by: Marc Zyngier > > --- > > arch/arm64/include/asm/smp.h | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h > > index ea268d88b6f7..a0c8a0b65259 100644 > > --- a/arch/arm64/include/asm/smp.h > > +++ b/arch/arm64/include/asm/smp.h > > @@ -30,7 +30,6 @@ > > #include > > #include > > #include > > -#include > > > > DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number); > > I think this arm64 patch makes sense irrespective of any other generic > fixes. If Will wants to take it as a fix: > > Acked-by: Catalin Marinas > > (otherwise I'll queue it for 5.9) Cheers, I'll pick this up asap. Will
Re: linux-next: build failure after merge of the origin tree
On 2020-07-30 07:14, Willy Tarreau wrote: On Thu, Jul 30, 2020 at 05:22:50AM +0200, Willy Tarreau wrote: On Wed, Jul 29, 2020 at 08:17:48PM -0700, Kees Cook wrote: > And just another heads-up, the patch[1] (which was never sent to a public > list) also breaks arm64 (circular header needs?): (...) Definitely, we've just got a report about this, I'll have a look once I'm at the office. I'd like to check that we don't obviously break another arch by just removing percpu. If at least shuffling them around is sufficient that'd be nice. Otherwise we'll likely need a separate header (which is not a bad thing for the long term). So Linus proposed a clean solution which might be harder to backport but looks better for 5.8. However the attached one addresses the issue for me on arm64 and still works on x86_64, arm, mips. I think we should go with this one first then apply Linus' one on top of it to be long term proof, and backport only the first one. Linus ? So for what it's worth, this patch fixes the arm64 compilation problem for me: Tested-by: Marc Zyngier I had come up with a different fix, only touching arm64 (see below). Thanks, M. From 33d819f4efa0a4474b5dc2e4bcaef1b886ca30c3 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Thu, 30 Jul 2020 10:53:05 +0100 Subject: [PATCH] arm64: Drop unnecessary include from asm/smp.h asm/pointer_auth.h is not needed anymore in asm/smp.h, as 62a679cb2825 ("arm64: simplify ptrauth initialization") removed the keys from the secondary_data structure. This also cures a compilation issue introduced by f227e3ec3b5c ("random32: update the net random state on interrupt and activity"). Fixes: 62a679cb2825 ("arm64: simplify ptrauth initialization") Fixes: f227e3ec3b5c ("random32: update the net random state on interrupt and activity") Signed-off-by: Marc Zyngier --- arch/arm64/include/asm/smp.h | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h index ea268d88b6f7..a0c8a0b65259 100644 --- a/arch/arm64/include/asm/smp.h +++ b/arch/arm64/include/asm/smp.h @@ -30,7 +30,6 @@ #include #include #include -#include DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number); -- 2.27.0 -- Who you jivin' with that Cosmik Debris?
Re: linux-next: build failure after merge of the origin tree
On Thu, Jul 30, 2020 at 10:59:09AM +0100, Marc Zyngier wrote: > From 33d819f4efa0a4474b5dc2e4bcaef1b886ca30c3 Mon Sep 17 00:00:00 2001 > From: Marc Zyngier > Date: Thu, 30 Jul 2020 10:53:05 +0100 > Subject: [PATCH] arm64: Drop unnecessary include from asm/smp.h > > asm/pointer_auth.h is not needed anymore in asm/smp.h, as 62a679cb2825 > ("arm64: simplify ptrauth initialization") removed the keys from the > secondary_data structure. > > This also cures a compilation issue introduced by f227e3ec3b5c > ("random32: update the net random state on interrupt and activity"). > > Fixes: 62a679cb2825 ("arm64: simplify ptrauth initialization") > Fixes: f227e3ec3b5c ("random32: update the net random state on interrupt and > activity") > Signed-off-by: Marc Zyngier > --- > arch/arm64/include/asm/smp.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h > index ea268d88b6f7..a0c8a0b65259 100644 > --- a/arch/arm64/include/asm/smp.h > +++ b/arch/arm64/include/asm/smp.h > @@ -30,7 +30,6 @@ > #include > #include > #include > -#include > > DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number); I think this arm64 patch makes sense irrespective of any other generic fixes. If Will wants to take it as a fix: Acked-by: Catalin Marinas (otherwise I'll queue it for 5.9)
Re: linux-next: build failure after merge of the origin tree
On Thu, Jul 30, 2020 at 05:22:50AM +0200, Willy Tarreau wrote: > On Wed, Jul 29, 2020 at 08:17:48PM -0700, Kees Cook wrote: > > And just another heads-up, the patch[1] (which was never sent to a public > > list) also breaks arm64 (circular header needs?): > (...) > > Definitely, we've just got a report about this, I'll have a look once > I'm at the office. I'd like to check that we don't obviously break > another arch by just removing percpu. If at least shuffling them around > is sufficient that'd be nice. Otherwise we'll likely need a separate > header (which is not a bad thing for the long term). So Linus proposed a clean solution which might be harder to backport but looks better for 5.8. However the attached one addresses the issue for me on arm64 and still works on x86_64, arm, mips. I think we should go with this one first then apply Linus' one on top of it to be long term proof, and backport only the first one. Linus ? Willy >From 18fba9e2dfb16605a722e01f95d9e2d020efaa42 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 30 Jul 2020 07:59:24 +0200 Subject: random: fix circular include dependency on arm64 after addition of percpu.h MIME-Version: 1.0 Content-Type: text/plain; charset=latin1 Content-Transfer-Encoding: 8bit Daniel Díaz and Kees Cook independently reported that commit f227e3ec3b5c ("random32: update the net random state on interrupt and activity") broke arm64 due to a circular dependency on include files since the addition of percpu.h in random.h. The correct fix would definitely be to move all the prandom32 stuff out of random.h but for backporting, a smaller solution is preferred. This one replaces linux/percpu.h with asm/percpu.h, and this fixes the problem on x86_64, arm64, arm, and mips. Note that moving percpu.h around didn't change anything and that removing it entirely broke differently. When backporting, such options might still be considered if this patch fails to help. Reported-by: Daniel Díaz Reported-by: Kees Cook Fixes: f227e3ec3b5c Cc: Stephen Rothwell Cc: Linus Torvalds Signed-off-by: Willy Tarreau --- include/linux/random.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/random.h b/include/linux/random.h index f310897f051d..9ab7443bd91b 100644 --- a/include/linux/random.h +++ b/include/linux/random.h @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include -- 2.20.1
Re: linux-next: build failure after merge of the origin tree
Hi Kees, On Wed, Jul 29, 2020 at 08:17:48PM -0700, Kees Cook wrote: > And just another heads-up, the patch[1] (which was never sent to a public > list) also breaks arm64 (circular header needs?): (...) Definitely, we've just got a report about this, I'll have a look once I'm at the office. I'd like to check that we don't obviously break another arch by just removing percpu. If at least shuffling them around is sufficient that'd be nice. Otherwise we'll likely need a separate header (which is not a bad thing for the long term). Thanks! Willy
Re: linux-next: build failure after merge of the origin tree
On Wed, Jul 29, 2020 at 04:43:04PM -0700, Linus Torvalds wrote: > On Wed, Jul 29, 2020 at 4:08 PM Stephen Rothwell > wrote: > > > > include/linux/random.h:123:24: error: variable 'net_rand_state' with > > 'latent_entropy' attribute must not be local > > 123 | DECLARE_PER_CPU(struct rnd_state, net_rand_state) __latent_entropy; > > Hmm. > > Ok, this shows a limitation of my allmodconfig testing (and all my > normal builds) - no plugins. So that problem wasn't as obvious as it > should have been. I'll look into this more tomorrow. (But yes, __latent_entropy is absolutely used for globals already, as you found, but this is the first percpu it was applied to...) > Adding the gcc plugin people. Otherwise the only option seems to be to > just remove that __latent_entropy marker. And just another heads-up, the patch[1] (which was never sent to a public list) also breaks arm64 (circular header needs?): $ make CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64 defconfig ... $ make -j$(getconf _NPROCESSORS_ONLN) CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64 ... In file included from ./arch/arm64/include/asm/smp.h:33, from ./include/linux/smp.h:82, from ./include/linux/percpu.h:7, from ./include/linux/random.h:14, from arch/arm64/kernel/pointer_auth.c:5: ./arch/arm64/include/asm/pointer_auth.h: In function ‘ptrauth_keys_init_user’: ./arch/arm64/include/asm/pointer_auth.h:40:3: error: implicit declaration of function ‘get_random_bytes’; did you mean ‘get_random_once’? [-Werror=implicit-function-declaration] 40 | get_random_bytes(>apia, sizeof(keys->apia)); | ^ [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f227e3ec3b5cad859ad15666874405e8c1bbc1d4 -- Kees Cook
Re: linux-next: build failure after merge of the origin tree
On Wed, Jul 29, 2020 at 07:12:58PM -0700, Linus Torvalds wrote: > On Wed, Jul 29, 2020 at 5:09 PM Linus Torvalds > wrote: > > > > Removing the __latent_entropy marker obviously fixes things. > > Ok, I did that for now. I spent a few minutes looking at the gcc > plugin in case I'd be hit by some sudden stroke of genius, but that > didn't happen, so let's avoid the issue until somebody who knows the > gcc plugins better can come up with what the right solution is. I've looked if we couldn't we work around this by declaring another static variable with __latent_entropy and use it to initialize net_rand_state early, for example in prandom_init(), but there we already fill net_rand_state with randoms so I'm wondering if that __latent_entropy is used before prandom_init() or if its sole purpose is to provide extra initial entropy to be combined with the one prandom_init() will add. Willy
Re: linux-next: build failure after merge of the origin tree
On Wed, Jul 29, 2020 at 5:09 PM Linus Torvalds wrote: > > Removing the __latent_entropy marker obviously fixes things. Ok, I did that for now. I spent a few minutes looking at the gcc plugin in case I'd be hit by some sudden stroke of genius, but that didn't happen, so let's avoid the issue until somebody who knows the gcc plugins better can come up with what the right solution is. Linus
Re: linux-next: build failure after merge of the origin tree
On Wed, Jul 29, 2020 at 4:43 PM Linus Torvalds wrote: > > Ok, this shows a limitation of my allmodconfig testing (and all my > normal builds) - no plugins. So that problem wasn't as obvious as it > should have been. Ok, that was easy to install and get the coverage, and now I see the error. Except I still don't know the gcc plugins well enough to fix it at the plugin level. And the gcc docs only talk about TREE_STATIC() for functions, not for variables. Apparently variables should use DECL_THIS_EXTERN or DECL_THIS_STATIC according to the docs I find, but .. Removing the __latent_entropy marker obviously fixes things. Linus
Re: linux-next: build failure after merge of the origin tree
On Wed, Jul 29, 2020 at 4:08 PM Stephen Rothwell wrote: > > include/linux/random.h:123:24: error: variable 'net_rand_state' with > 'latent_entropy' attribute must not be local > 123 | DECLARE_PER_CPU(struct rnd_state, net_rand_state) __latent_entropy; Hmm. Ok, this shows a limitation of my allmodconfig testing (and all my normal builds) - no plugins. So that problem wasn't as obvious as it should have been. That error isn't very helpful, in that I think it actually is very wrong. The variable really isn't local at all. I think what the plugin *means* by "local" is "automatic", and I think it uses the wrong test for it. IOW, looking at the plugin, it does if (!TREE_STATIC(*node)) { *no_add_attrs = true; error("variable %qD with %qE attribute must not be local", *node, name); and what I think it really wants is that it has a static address - so a global variable is fine - as opposed to being an actual static declaration. Also looking at the plugin, I suspect it is going to be very unhappy about the fact that the attribute is there both on a declaration and on the actual definition. The code later seems to really only want to work on the definition, since it's creating an initializer.. IOW, I get the feeling that the plugin is confused, and it so happened that the only variables we'd marked for latent entropy were static ones. But I haven't done gcc plugins, so... Adding the gcc plugin people. Otherwise the only option seems to be to just remove that __latent_entropy marker. Linus
Re: linux-next: build failure after merge of the origin tree
+++ Josh Poimboeuf [04/06/20 19:04 -0500]: On Fri, Jun 05, 2020 at 08:37:15AM +1000, Stephen Rothwell wrote: Hi all, After merging the origin tree, today's linux-next build (powerpc ppc64_defconfig) failed like this: kernel/module.c: In function 'do_init_module': kernel/module.c:3593:2: error: implicit declaration of function 'module_enable_ro'; did you mean 'module_enable_x'? [-Werror=implicit-function-declaration] 3593 | module_enable_ro(mod, true); | ^~~~ | module_enable_x Caused by commit e6eff4376e28 ("module: Make module_enable_ro() static again") This config has neither CONFIG_ARCH_HAS_STRICT_MODULE_RWX or CONFIG_ARCH_HAS_STRICT_MODULE_RWX set. This failure was hidden in linux-next due to commit db991af02f11 ("module: break nested ARCH_HAS_STRICT_MODULE_RWX and STRICT_MODULE_RWX #ifdefs") from the modules tree. I have cherry-picked that commit for today. Sorry, I guessed we missed that dependency between the live-patching and module trees. Jessica, are you planning on sending a pull request? Yep, just sent. So hopefully this gets resolved in the next day or two.
Re: linux-next: build failure after merge of the origin tree
On Fri, Jun 05, 2020 at 08:37:15AM +1000, Stephen Rothwell wrote: > Hi all, > > After merging the origin tree, today's linux-next build (powerpc > ppc64_defconfig) failed like this: > > kernel/module.c: In function 'do_init_module': > kernel/module.c:3593:2: error: implicit declaration of function > 'module_enable_ro'; did you mean 'module_enable_x'? > [-Werror=implicit-function-declaration] > 3593 | module_enable_ro(mod, true); > | ^~~~ > | module_enable_x > > Caused by commit > > e6eff4376e28 ("module: Make module_enable_ro() static again") > > This config has neither CONFIG_ARCH_HAS_STRICT_MODULE_RWX or > CONFIG_ARCH_HAS_STRICT_MODULE_RWX set. This failure was hidden in > linux-next due to commit > > db991af02f11 ("module: break nested ARCH_HAS_STRICT_MODULE_RWX and > STRICT_MODULE_RWX #ifdefs") > > from the modules tree. I have cherry-picked that commit for today. Sorry, I guessed we missed that dependency between the live-patching and module trees. Jessica, are you planning on sending a pull request? -- Josh
Re: linux-next: build failure after merge of the origin tree
Hi, On Wed, 10 Oct 2012 08:52:21 +0900 Yasuaki Ishimatsu wrote: > > 2012/10/10 8:45, Andrew Morton wrote: > > On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell > > wrote: > > > >> Hi Linus, > >> > >> In Linus' tree, today's linux-next build (powerpc ppc64_defconfig) failed > >> like this: > >> > >> arch/powerpc/platforms/pseries/hotplug-memory.c: In function > >> 'pseries_remove_memblock': > >> arch/powerpc/platforms/pseries/hotplug-memory.c:103:17: error: unused > >> variable 'pfn' [-Werror=unused-variable] > >> > >> Caused by commit d760afd4d257 ("memory-hotplug: suppress "Trying to free > >> nonexistent resource " warning"). > >> > >> I can't see what the point of the "pfn" variable is > > > > This: > > > > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a > > +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c > > @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig > > sections_to_remove = (memblock_size >> PAGE_SHIFT) / PAGES_PER_SECTION; > > for (i = 0; i < sections_to_remove; i++) { > > unsigned long pfn = start_pfn + i * PAGES_PER_SECTION; > > - ret = __remove_pages(zone, start_pfn, PAGES_PER_SECTION); > > + ret = __remove_pages(zone, pfn, PAGES_PER_SECTION); > > if (ret) > > return ret; > > } > > I believe the error to be fixed with this patch. > Could you try it? The certainly fixes the build problem. I can't comment in the semantics of the patch. Tested-by: Stephen Rothwell (Build only) -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpPd0hiaAeXO.pgp Description: PGP signature
Re: linux-next: build failure after merge of the origin tree
Hi Andrew, On Tue, 9 Oct 2012 16:45:14 -0700 Andrew Morton wrote: > > On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell > wrote: > > > I can't see what the point of the "pfn" variable is > > This: > > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a > +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c > @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig > sections_to_remove = (memblock_size >> PAGE_SHIFT) / PAGES_PER_SECTION; > for (i = 0; i < sections_to_remove; i++) { > unsigned long pfn = start_pfn + i * PAGES_PER_SECTION; > - ret = __remove_pages(zone, start_pfn, PAGES_PER_SECTION); > + ret = __remove_pages(zone, pfn, PAGES_PER_SECTION); > if (ret) > return ret; > } Can we get that fix to Linus ASAP, please? > > and this patch never > > appeared in linux-next before being merged. :-( > > It was first sighted October 3. Yeah, my mistake. But it never made it to linux-next. > > I have reverted that commit for today. > > > > If this patch truly was authored yesterday (according the Author Date in > > git), why was it merged yesterday while still under discussion? And the > > latest update to it still has this build problem ... did anyone even try > > to build this for powerpc (since that architecture was obviously > > affected)? > > Apparently not - the ppc bit was a best-effort fixup for a patch which > addresses an x86 problem. Right, and that is one of the reasons we have linux-next - to test for cross architecture problems. -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpKGcnDLpu5b.pgp Description: PGP signature
Re: linux-next: build failure after merge of the origin tree
Hi Stephen, 2012/10/10 8:45, Andrew Morton wrote: On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell wrote: Hi Linus, In Linus' tree, today's linux-next build (powerpc ppc64_defconfig) failed like this: arch/powerpc/platforms/pseries/hotplug-memory.c: In function 'pseries_remove_memblock': arch/powerpc/platforms/pseries/hotplug-memory.c:103:17: error: unused variable 'pfn' [-Werror=unused-variable] Caused by commit d760afd4d257 ("memory-hotplug: suppress "Trying to free nonexistent resource " warning"). I can't see what the point of the "pfn" variable is This: --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig sections_to_remove = (memblock_size >> PAGE_SHIFT) / PAGES_PER_SECTION; for (i = 0; i < sections_to_remove; i++) { unsigned long pfn = start_pfn + i * PAGES_PER_SECTION; - ret = __remove_pages(zone, start_pfn, PAGES_PER_SECTION); + ret = __remove_pages(zone, pfn, PAGES_PER_SECTION); if (ret) return ret; } I believe the error to be fixed with this patch. Could you try it? Thanks, Yasuaki Ishimatsu and this patch never appeared in linux-next before being merged. :-( It was first sighted October 3. I have reverted that commit for today. If this patch truly was authored yesterday (according the Author Date in git), why was it merged yesterday while still under discussion? And the latest update to it still has this build problem ... did anyone even try to build this for powerpc (since that architecture was obviously affected)? Apparently not - the ppc bit was a best-effort fixup for a patch which addresses an x86 problem. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the origin tree
On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell wrote: > Hi Linus, > > In Linus' tree, today's linux-next build (powerpc ppc64_defconfig) failed > like this: > > arch/powerpc/platforms/pseries/hotplug-memory.c: In function > 'pseries_remove_memblock': > arch/powerpc/platforms/pseries/hotplug-memory.c:103:17: error: unused > variable 'pfn' [-Werror=unused-variable] > > Caused by commit d760afd4d257 ("memory-hotplug: suppress "Trying to free > nonexistent resource " warning"). > > I can't see what the point of the "pfn" variable is This: --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig sections_to_remove = (memblock_size >> PAGE_SHIFT) / PAGES_PER_SECTION; for (i = 0; i < sections_to_remove; i++) { unsigned long pfn = start_pfn + i * PAGES_PER_SECTION; - ret = __remove_pages(zone, start_pfn, PAGES_PER_SECTION); + ret = __remove_pages(zone, pfn, PAGES_PER_SECTION); if (ret) return ret; } > and this patch never > appeared in linux-next before being merged. :-( It was first sighted October 3. > I have reverted that commit for today. > > If this patch truly was authored yesterday (according the Author Date in > git), why was it merged yesterday while still under discussion? And the > latest update to it still has this build problem ... did anyone even try > to build this for powerpc (since that architecture was obviously > affected)? Apparently not - the ppc bit was a best-effort fixup for a patch which addresses an x86 problem. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the origin tree
On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell s...@canb.auug.org.au wrote: Hi Linus, In Linus' tree, today's linux-next build (powerpc ppc64_defconfig) failed like this: arch/powerpc/platforms/pseries/hotplug-memory.c: In function 'pseries_remove_memblock': arch/powerpc/platforms/pseries/hotplug-memory.c:103:17: error: unused variable 'pfn' [-Werror=unused-variable] Caused by commit d760afd4d257 (memory-hotplug: suppress Trying to free nonexistent resource - warning). I can't see what the point of the pfn variable is This: --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig sections_to_remove = (memblock_size PAGE_SHIFT) / PAGES_PER_SECTION; for (i = 0; i sections_to_remove; i++) { unsigned long pfn = start_pfn + i * PAGES_PER_SECTION; - ret = __remove_pages(zone, start_pfn, PAGES_PER_SECTION); + ret = __remove_pages(zone, pfn, PAGES_PER_SECTION); if (ret) return ret; } and this patch never appeared in linux-next before being merged. :-( It was first sighted October 3. I have reverted that commit for today. If this patch truly was authored yesterday (according the Author Date in git), why was it merged yesterday while still under discussion? And the latest update to it still has this build problem ... did anyone even try to build this for powerpc (since that architecture was obviously affected)? Apparently not - the ppc bit was a best-effort fixup for a patch which addresses an x86 problem. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the origin tree
Hi Stephen, 2012/10/10 8:45, Andrew Morton wrote: On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell s...@canb.auug.org.au wrote: Hi Linus, In Linus' tree, today's linux-next build (powerpc ppc64_defconfig) failed like this: arch/powerpc/platforms/pseries/hotplug-memory.c: In function 'pseries_remove_memblock': arch/powerpc/platforms/pseries/hotplug-memory.c:103:17: error: unused variable 'pfn' [-Werror=unused-variable] Caused by commit d760afd4d257 (memory-hotplug: suppress Trying to free nonexistent resource - warning). I can't see what the point of the pfn variable is This: --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig sections_to_remove = (memblock_size PAGE_SHIFT) / PAGES_PER_SECTION; for (i = 0; i sections_to_remove; i++) { unsigned long pfn = start_pfn + i * PAGES_PER_SECTION; - ret = __remove_pages(zone, start_pfn, PAGES_PER_SECTION); + ret = __remove_pages(zone, pfn, PAGES_PER_SECTION); if (ret) return ret; } I believe the error to be fixed with this patch. Could you try it? Thanks, Yasuaki Ishimatsu and this patch never appeared in linux-next before being merged. :-( It was first sighted October 3. I have reverted that commit for today. If this patch truly was authored yesterday (according the Author Date in git), why was it merged yesterday while still under discussion? And the latest update to it still has this build problem ... did anyone even try to build this for powerpc (since that architecture was obviously affected)? Apparently not - the ppc bit was a best-effort fixup for a patch which addresses an x86 problem. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the origin tree
Hi Andrew, On Tue, 9 Oct 2012 16:45:14 -0700 Andrew Morton a...@linux-foundation.org wrote: On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell s...@canb.auug.org.au wrote: I can't see what the point of the pfn variable is This: --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig sections_to_remove = (memblock_size PAGE_SHIFT) / PAGES_PER_SECTION; for (i = 0; i sections_to_remove; i++) { unsigned long pfn = start_pfn + i * PAGES_PER_SECTION; - ret = __remove_pages(zone, start_pfn, PAGES_PER_SECTION); + ret = __remove_pages(zone, pfn, PAGES_PER_SECTION); if (ret) return ret; } Can we get that fix to Linus ASAP, please? and this patch never appeared in linux-next before being merged. :-( It was first sighted October 3. Yeah, my mistake. But it never made it to linux-next. I have reverted that commit for today. If this patch truly was authored yesterday (according the Author Date in git), why was it merged yesterday while still under discussion? And the latest update to it still has this build problem ... did anyone even try to build this for powerpc (since that architecture was obviously affected)? Apparently not - the ppc bit was a best-effort fixup for a patch which addresses an x86 problem. Right, and that is one of the reasons we have linux-next - to test for cross architecture problems. -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpKGcnDLpu5b.pgp Description: PGP signature
Re: linux-next: build failure after merge of the origin tree
Hi, On Wed, 10 Oct 2012 08:52:21 +0900 Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com wrote: 2012/10/10 8:45, Andrew Morton wrote: On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell s...@canb.auug.org.au wrote: Hi Linus, In Linus' tree, today's linux-next build (powerpc ppc64_defconfig) failed like this: arch/powerpc/platforms/pseries/hotplug-memory.c: In function 'pseries_remove_memblock': arch/powerpc/platforms/pseries/hotplug-memory.c:103:17: error: unused variable 'pfn' [-Werror=unused-variable] Caused by commit d760afd4d257 (memory-hotplug: suppress Trying to free nonexistent resource - warning). I can't see what the point of the pfn variable is This: --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig sections_to_remove = (memblock_size PAGE_SHIFT) / PAGES_PER_SECTION; for (i = 0; i sections_to_remove; i++) { unsigned long pfn = start_pfn + i * PAGES_PER_SECTION; - ret = __remove_pages(zone, start_pfn, PAGES_PER_SECTION); + ret = __remove_pages(zone, pfn, PAGES_PER_SECTION); if (ret) return ret; } I believe the error to be fixed with this patch. Could you try it? The certainly fixes the build problem. I can't comment in the semantics of the patch. Tested-by: Stephen Rothwell s...@canb.auug.org.au (Build only) -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpPd0hiaAeXO.pgp Description: PGP signature