* Re: [PATCH v3 3/3] powerpc/mpc85xx: add support for Keymile's kmcoge4 board
From: Scott Wood @ 2014-04-09 0:47 UTC (permalink / raw)
To: Valentin Longchamp; +Cc: devicetree, linuxppc-dev
In-Reply-To: <1395754915-14534-4-git-send-email-valentin.longchamp@keymile.com>
On Tue, 2014-03-25 at 14:41 +0100, Valentin Longchamp wrote:
> + lbc: localbus@ffe124000 {
> + reg = <0xf 0xfe124000 0 0x1000>;
> + ranges = <0 0 0xf 0xffa00000 0x00040000 /* LB 0 */
> + 1 0 0xf 0xfb000000 0x00010000 /* LB 1 */
> + 2 0 0xf 0xd0000000 0x10000000 /* LB 2 */
> + 3 0 0xf 0xe0000000 0x10000000>; /* LB 3 */
> +
> + nand@0,0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "fsl,elbc-fcm-nand";
> + reg = <0 0 0x40000>;
> +
> + partition@0 {
> + label = "ubi0";
> + reg = <0x0 0x10000000>;
> + };
Putting partition info in the dts file is a bad habit and (as I've told
others) I don't think we should continue doing so in new dts files.
In this case it looks like you're just making the entire chip into one
partition, so I'm not sure what the point is of partitioning at all.
-Scott
^ permalink raw reply
* Re: [PATCH v3 2/3] devcietree: bindings: add some MFD Keymile FPGAs
From: Scott Wood @ 2014-04-09 0:44 UTC (permalink / raw)
To: Valentin Longchamp; +Cc: devicetree, linuxppc-dev
In-Reply-To: <1395754915-14534-3-git-send-email-valentin.longchamp@keymile.com>
On Tue, 2014-03-25 at 14:41 +0100, Valentin Longchamp wrote:
> These are the bindings for 2 MFD devices used on some of the Keymile boards.
> The first one is the chassis managmenet bfticu FPGA.
> The second one is the board controller (reset, LEDs, GPIOs) QRIO CPDL.
> These FPGAs are used in the kmcoge4 board.
>
> This patch also add KEYMILE to the vendor-prefixes.
>
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
>
> ---
> Changes in v3:
> - add a patch with the bindings for the KEYMILE FPGAs
>
> Changes in v2: None
>
> Documentation/devicetree/bindings/mfd/bfticu.txt | 26 ++++++++++++++++++++++
> Documentation/devicetree/bindings/mfd/qriox.txt | 17 ++++++++++++++
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> 3 files changed, 44 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/bfticu.txt
> create mode 100644 Documentation/devicetree/bindings/mfd/qriox.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/bfticu.txt b/Documentation/devicetree/bindings/mfd/bfticu.txt
> new file mode 100644
> index 0000000..92de32e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/bfticu.txt
> @@ -0,0 +1,26 @@
> +KEYMILE bfticu Chassis Management FPGA
> +
> +The bfticu is a multifunction device that manages the whole chassis.
> +Its main functionality is to collect IRQs from the whole chassis and signals
> +them to a single controller.
> +
> +Required properties:
> +- compatible: "keymile,bfticu"
> +- interrupt-controller: the bfticu FPGA is an interrupt controller
> +- interrupts: the main IRQ line to signal the collected IRQs
> +- #interrupt-cells : is 2
> + - The 1st cell is the local IRQ number on the bfticu
> + - The 2nd cell is the type of the IRQ (IRQ_TYPE_xxx)
Device tree bindings should not depend on the content of Linux headers.
One is stable ABI, and the other isn't.
If you want you can make the values the same for convenience, as is done
by IPIC, CPM PIC, etc -- but the values need to be explicitly stated in
the binding. It'll still break if the Linux values change (so it may
not be a good idea to try to keep the values the same), but at least the
fix would be in Linux code rather than an ABI change.
> diff --git a/Documentation/devicetree/bindings/mfd/qriox.txt b/Documentation/devicetree/bindings/mfd/qriox.txt
> new file mode 100644
> index 0000000..f301e2d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/qriox.txt
> @@ -0,0 +1,17 @@
> +KEYMILE qrio Board Control CPLD
> +
> +The qrio is a multifunction device that controls the KEYMILE boards based on
> +the kmp204x design.
> +It is consists of a reset controller, watchdog timer, LEDs, and 2 IRQ capable
> +GPIO blocks.
> +
> +Required properties:
> +- compatible: "keymile,qriox"
> +- reg: access on the parent local bus (chip select, offset in chip select, size)
> +
> +Example:
> +
> + board-control@1,0 {
> + compatible = "keymile,qriox";
> + reg = <1 0 0x80>;
> + };
If it has GPIO blocks, shouldn't it be using the GPIO binding?
-Scott
^ permalink raw reply
* Re: [1/2,v9] powerpc/mpc85xx:Add initial device tree support of T104x
From: Scott Wood @ 2014-04-09 0:08 UTC (permalink / raw)
To: Prabhakar Kushwaha
Cc: Varun Sethi, Poonam Aggrwal, linuxppc-dev, Priyanka Jain
In-Reply-To: <53415417.3010802@freescale.com>
On Sun, 2014-04-06 at 18:48 +0530, Prabhakar Kushwaha wrote:
> On 3/20/2014 4:03 AM, Scott Wood wrote:
> > On Sat, Jan 25, 2014 at 05:10:59PM +0530, Prabhakar Kushwaha wrote:
> >> + clockgen: global-utilities@e1000 {
> >> + compatible = "fsl,t1040-clockgen", "fsl,qoriq-clockgen-2.0",
> >> + "fixed-clock";
> >> + ranges = <0x0 0xe1000 0x1000>;
> >> + clock-frequency = <100000000>;
> > Why is clock-frequency hardcoded here rather than supplied by U-Boot?
> > Especially since this is an SoC file, not a board file.
>
> Your are correct.
> Means, clock-frequency should be added to clockgen in board device tree ??
Usually it gets patched in by U-Boot.
-Scott
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Fix Bit Clock Polarity configurations
From: Mark Brown @ 2014-04-08 11:50 UTC (permalink / raw)
To: Nicolin Chen; +Cc: alsa-devel, linuxppc-dev, linux-kernel
In-Reply-To: <20140408110738.GA10745@MrMyself>
[-- Attachment #1: Type: text/plain, Size: 216 bytes --]
On Tue, Apr 08, 2014 at 07:07:40PM +0800, Nicolin Chen wrote:
> Sir, I can't find this patch on any of the remote branches: for-next,
> topic/fsl-sai and fix/fsl-sai. Where could I find it?
It's in the fix branch.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Fix Bit Clock Polarity configurations
From: Nicolin Chen @ 2014-04-08 11:07 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel, linuxppc-dev, linux-kernel
In-Reply-To: <20140404100532.GN14763@sirena.org.uk>
On Fri, Apr 04, 2014 at 11:05:32AM +0100, Mark Brown wrote:
> On Fri, Apr 04, 2014 at 03:09:47PM +0800, Nicolin Chen wrote:
> > The BCP bit in TCR4/RCR4 register rules as followings:
> > 0 Bit clock is active high with drive outputs on rising edge
> > and sample inputs on falling edge.
> > 1 Bit clock is active low with drive outputs on falling edge
> > and sample inputs on rising edge.
>
> Applied, thanks.
Sir, I can't find this patch on any of the remote branches: for-next,
topic/fsl-sai and fix/fsl-sai. Where could I find it?
Thank you,
Nicolin
^ permalink raw reply
* Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
From: Neil Horman @ 2014-04-08 10:49 UTC (permalink / raw)
To: Gerhard Sittig
Cc: Yann Droneaud, linux-kernel, Neil Horman, Anton Blanchard,
Hans-Bernhard Broeker, linuxppc-dev, Hans-Bernhard Bröker
In-Reply-To: <20140408075610.GJ11339@book.gsilab.sittig.org>
On Tue, Apr 08, 2014 at 09:56:10AM +0200, Gerhard Sittig wrote:
> [ removed cscope-devel from Cc:, non-subscriber mails get blocked anyway ]
>
> On Mon, 2014-04-07 at 14:42 +0200, Gerhard Sittig wrote:
> >
> > On Mon, 2014-04-07 at 06:42 -0400, Neil Horman wrote:
> > >
> > > On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote:
> > > >
> > > > [ ... ]
> > > >
> > > > cscope reports error when generating the cross-reference database:
> > > >
> > > > $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope
> > > > GEN cscope
> > > > cscope: cannot find
> > > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
> > > > cscope: cannot find
> > > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S
> > > > cscope: cannot find
> > > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
> > > > cscope: cannot find
> > > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S
> > > >
> > > > And when calling cscope from ./obj-cscope/ directory, it reports errors
> > > > too.
> > > >
> > > > Hopefully it doesn't stop it from working, so I'm still able to use
> > > > cscope to browse kernel sources.
> > > >
> > > No, it won't stop it from working, it just won't search those files. I don't
> > > recall exactly the reason, but IIRC there was a big discussion long ago about
> > > symlinks and our ability to support them (around version 1.94 I think). We
> > > decided to not handle symlinks, as they would either point outside our search
> > > tree, which we didn't want to include, or would point to another file in the
> > > search tree, which made loading them pointless (as we would cover the search in
> > > the pointed file).
> >
> > So there are valid reasons to not process those filesystem
> > entries. Would it be useful to not emit the warnings then? Or
> > to silent those warnings when the user knows it's perfectly legal
> > to skip those filesytem entries? Like what you can do with the
> > ctags(1) command and its --links option.
>
> For the record: I got a response "off list" (actually to the
> cscope list only which is closed for non-subscribers, while the
> Linux related recipients were removed despite the fact that the
> issue appears to be in Linux), see
> http://article.gmane.org/gmane.comp.programming.tools.cscope.devel/105
>
I don't agree with Hans here. He's right in that the Linux makefiles could
exclude the symlinks from the index file that it builds, but if cscope were left
to its own devices it would also exclude the assembly files and other code that
it doesn't officially parse, but does more or less ok with. We can argue all
day weather its ok for Linux to do that, but the facts of the matter is that
people use cscope to search the linux source tree asm files and all, and while
we could propose that the cscope makefile target filter out symlinks, it seems
IMHO to be more difficult than its worth. Adding code to silence db build
warnings would be good.
> This reponse suggests that the issue is not in cscope(1) itself,
> but instead is in how the cscope(1) command got invoked. Which
> translates into "the Linux infrastructure does something wrong".
>
> A quick search identifies ./scripts/tags.sh:all_target_sources()
> as the spot where symlinks should get filtered out. Where both
> paths of all_target_sources() end up calling all_sources().
>
>
> virtually yours
> Gerhard Sittig
> --
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
>
^ permalink raw reply
* Re: [PATCH v2] power, sched: stop updating inside arch_update_cpu_topology() when nothing to be update
From: Srivatsa S. Bhat @ 2014-04-08 8:24 UTC (permalink / raw)
To: Michael wang
Cc: sfr, LKML, paulus, alistair, nfont, Andrew Morton, rcj,
linuxppc-dev
In-Reply-To: <53436AC8.5020705@linux.vnet.ibm.com>
On 04/08/2014 08:49 AM, Michael wang wrote:
> Since v1:
> Edited the comment according to Srivatsa's suggestion.
>
> During the testing, we encounter below WARN followed by Oops:
>
> WARNING: at kernel/sched/core.c:6218
> ...
> NIP [c000000000101660] .build_sched_domains+0x11d0/0x1200
> LR [c000000000101358] .build_sched_domains+0xec8/0x1200
> PACATMSCRATCH [800000000000f032]
> Call Trace:
> [c00000001b103850] [c000000000101358] .build_sched_domains+0xec8/0x1200
> [c00000001b1039a0] [c00000000010aad4] .partition_sched_domains+0x484/0x510
> [c00000001b103aa0] [c00000000016d0a8] .rebuild_sched_domains+0x68/0xa0
> [c00000001b103b30] [c00000000005cbf0] .topology_work_fn+0x10/0x30
> ...
> Oops: Kernel access of bad area, sig: 11 [#1]
> ...
> NIP [c00000000045c000] .__bitmap_weight+0x60/0xf0
> LR [c00000000010132c] .build_sched_domains+0xe9c/0x1200
> PACATMSCRATCH [8000000000029032]
> Call Trace:
> [c00000001b1037a0] [c000000000288ff4] .kmem_cache_alloc_node_trace+0x184/0x3a0
> [c00000001b103850] [c00000000010132c] .build_sched_domains+0xe9c/0x1200
> [c00000001b1039a0] [c00000000010aad4] .partition_sched_domains+0x484/0x510
> [c00000001b103aa0] [c00000000016d0a8] .rebuild_sched_domains+0x68/0xa0
> [c00000001b103b30] [c00000000005cbf0] .topology_work_fn+0x10/0x30
> ...
>
> This was caused by that 'sd->groups == NULL' after building groups, which
> was caused by the empty 'sd->span'.
>
> The cpu's domain contained nothing because the cpu was assigned to a wrong
> node, due to the following unfortunate sequence of events:
>
> 1. The hypervisor sent a topology update to the guest OS, to notify changes
> to the cpu-node mapping. However, the update was actually redundant - i.e.,
> the "new" mapping was exactly the same as the old one.
>
> 2. Due to this, the 'updated_cpus' mask turned out to be empty after exiting
> the 'for-loop' in arch_update_cpu_topology().
>
> 3. So we ended up calling stop-machine() with an empty cpumask list, which made
> stop-machine internally elect cpumask_first(cpu_online_mask), i.e., CPU0 as
> the cpu to run the payload (the update_cpu_topology() function).
>
> 4. This causes update_cpu_topology() to be run by CPU0. And since 'updates'
> is kzalloc()'ed inside arch_update_cpu_topology(), update_cpu_topology()
> finds update->cpu as well as update->new_nid to be 0. In other words, we
> end up assigning CPU0 (and eventually its siblings) to node 0, incorrectly.
>
> Along with the following wrong updating, it causes the sched-domain rebuild
> code to break and crash the system.
>
> Fix this by skipping the topology update in cases where we find that
> the topology has not actually changed in reality (ie., spurious updates).
>
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: Paul Mackerras <paulus@samba.org>
> CC: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> CC: Stephen Rothwell <sfr@canb.auug.org.au>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Robert Jennings <rcj@linux.vnet.ibm.com>
> CC: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
> CC: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
> CC: Alistair Popple <alistair@popple.id.au>
> Suggested-by: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> ---
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> arch/powerpc/mm/numa.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 30a42e2..4ebbb9e 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1591,6 +1591,20 @@ int arch_update_cpu_topology(void)
> cpu = cpu_last_thread_sibling(cpu);
> }
>
> + /*
> + * In cases where we have nothing to update (because the updates list
> + * is too short or because the new topology is same as the old one),
> + * skip invoking update_cpu_topology() via stop-machine(). This is
> + * necessary (and not just a fast-path optimization) since stop-machine
> + * can end up electing a random CPU to run update_cpu_topology(), and
> + * thus trick us into setting up incorrect cpu-node mappings (since
> + * 'updates' is kzalloc()'ed).
> + *
> + * And for the similar reason, we will skip all the following updating.
> + */
> + if (!cpumask_weight(&updated_cpus))
> + goto out;
> +
> stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
>
> /*
> @@ -1612,6 +1626,7 @@ int arch_update_cpu_topology(void)
> changed = 1;
> }
>
> +out:
> kfree(updates);
> return changed;
> }
>
^ permalink raw reply
* Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
From: Gerhard Sittig @ 2014-04-08 7:56 UTC (permalink / raw)
To: Neil Horman
Cc: Yann Droneaud, linux-kernel, Neil Horman, Anton Blanchard,
Hans-Bernhard Broeker, linuxppc-dev, Hans-Bernhard Bröker
In-Reply-To: <20140407124259.GZ11339@book.gsilab.sittig.org>
[ removed cscope-devel from Cc:, non-subscriber mails get blocked anyway ]
On Mon, 2014-04-07 at 14:42 +0200, Gerhard Sittig wrote:
>
> On Mon, 2014-04-07 at 06:42 -0400, Neil Horman wrote:
> >
> > On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote:
> > >
> > > [ ... ]
> > >
> > > cscope reports error when generating the cross-reference database:
> > >
> > > $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope
> > > GEN cscope
> > > cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
> > > cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S
> > > cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
> > > cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S
> > >
> > > And when calling cscope from ./obj-cscope/ directory, it reports errors
> > > too.
> > >
> > > Hopefully it doesn't stop it from working, so I'm still able to use
> > > cscope to browse kernel sources.
> > >
> > No, it won't stop it from working, it just won't search those files. I don't
> > recall exactly the reason, but IIRC there was a big discussion long ago about
> > symlinks and our ability to support them (around version 1.94 I think). We
> > decided to not handle symlinks, as they would either point outside our search
> > tree, which we didn't want to include, or would point to another file in the
> > search tree, which made loading them pointless (as we would cover the search in
> > the pointed file).
>
> So there are valid reasons to not process those filesystem
> entries. Would it be useful to not emit the warnings then? Or
> to silent those warnings when the user knows it's perfectly legal
> to skip those filesytem entries? Like what you can do with the
> ctags(1) command and its --links option.
For the record: I got a response "off list" (actually to the
cscope list only which is closed for non-subscribers, while the
Linux related recipients were removed despite the fact that the
issue appears to be in Linux), see
http://article.gmane.org/gmane.comp.programming.tools.cscope.devel/105
This reponse suggests that the issue is not in cscope(1) itself,
but instead is in how the cscope(1) command got invoked. Which
translates into "the Linux infrastructure does something wrong".
A quick search identifies ./scripts/tags.sh:all_target_sources()
as the spot where symlinks should get filtered out. Where both
paths of all_target_sources() end up calling all_sources().
virtually yours
Gerhard Sittig
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
^ permalink raw reply
* [PATCH] powerpc/le: Fix module build error
From: Anton Blanchard @ 2014-04-08 7:20 UTC (permalink / raw)
To: benh; +Cc: Stewart Smith, Tony Breeds, linuxppc-dev
I made a cleanup suggestion on 27143b9a0 (powerpc/le: Avoid creatng
R_PPC64_TOCSAVE relocations for modules) that had a stupid typo. Fix
it.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index a9f814a..4c0cedf 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -74,7 +74,7 @@ override CROSS32AS += -mlittle-endian
LDEMULATION := lppc
GNUTARGET := powerpcle
MULTIPLEWORD := -mno-multiple
-KBUILD_CFLAGS_MODULE += $(call cc-option-yn,-mno-save-toc-indirect)
+KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)
else
ifeq ($(call cc-option-yn,-mbig-endian),y)
override CC += -mbig-endian
^ permalink raw reply related
* [PATCH] powerpc/4xx: Fix section mismatch in ppc4xx_pci.c
From: Alistair Popple @ 2014-04-08 4:20 UTC (permalink / raw)
To: benh; +Cc: Alistair Popple, linuxppc-dev
This patch fixes this section mismatch:
WARNING: vmlinux.o(.text+0x1efc4): Section mismatch in reference from
the function apm821xx_pciex_init_port_hw() to the function
.init.text:ppc4xx_pciex_wait_on_sdr.isra.9()
The function apm821xx_pciex_init_port_hw() references the function
__init ppc4xx_pciex_wait_on_sdr.isra.9(). This is often because
apm821xx_pciex_init_port_hw lacks a __init annotation or the
annotation of ppc4xx_pciex_wait_on_sdr.isra.9 is wrong.
apm821xx_pciex_init_port_hw is only referenced by a struct in
__initdata, so it should be safe to add __init to
apm821xx_pciex_init_port_hw.
Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
arch/powerpc/sysdev/ppc4xx_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
index 64603a1..4914fd3 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.c
+++ b/arch/powerpc/sysdev/ppc4xx_pci.c
@@ -1058,7 +1058,7 @@ static int __init apm821xx_pciex_core_init(struct device_node *np)
return 1;
}
-static int apm821xx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
+static int __init apm821xx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
{
u32 val;
--
1.8.3.2
^ permalink raw reply related
* [PATCH v2] power, sched: stop updating inside arch_update_cpu_topology() when nothing to be update
From: Michael wang @ 2014-04-08 3:19 UTC (permalink / raw)
To: linuxppc-dev, LKML, benh, paulus, nfont, sfr, Andrew Morton, rcj,
jlarrew, srivatsa.bhat, alistair
In-Reply-To: <533B8431.8090507@linux.vnet.ibm.com>
Since v1:
Edited the comment according to Srivatsa's suggestion.
During the testing, we encounter below WARN followed by Oops:
WARNING: at kernel/sched/core.c:6218
...
NIP [c000000000101660] .build_sched_domains+0x11d0/0x1200
LR [c000000000101358] .build_sched_domains+0xec8/0x1200
PACATMSCRATCH [800000000000f032]
Call Trace:
[c00000001b103850] [c000000000101358] .build_sched_domains+0xec8/0x1200
[c00000001b1039a0] [c00000000010aad4] .partition_sched_domains+0x484/0x510
[c00000001b103aa0] [c00000000016d0a8] .rebuild_sched_domains+0x68/0xa0
[c00000001b103b30] [c00000000005cbf0] .topology_work_fn+0x10/0x30
...
Oops: Kernel access of bad area, sig: 11 [#1]
...
NIP [c00000000045c000] .__bitmap_weight+0x60/0xf0
LR [c00000000010132c] .build_sched_domains+0xe9c/0x1200
PACATMSCRATCH [8000000000029032]
Call Trace:
[c00000001b1037a0] [c000000000288ff4] .kmem_cache_alloc_node_trace+0x184/0x3a0
[c00000001b103850] [c00000000010132c] .build_sched_domains+0xe9c/0x1200
[c00000001b1039a0] [c00000000010aad4] .partition_sched_domains+0x484/0x510
[c00000001b103aa0] [c00000000016d0a8] .rebuild_sched_domains+0x68/0xa0
[c00000001b103b30] [c00000000005cbf0] .topology_work_fn+0x10/0x30
...
This was caused by that 'sd->groups == NULL' after building groups, which
was caused by the empty 'sd->span'.
The cpu's domain contained nothing because the cpu was assigned to a wrong
node, due to the following unfortunate sequence of events:
1. The hypervisor sent a topology update to the guest OS, to notify changes
to the cpu-node mapping. However, the update was actually redundant - i.e.,
the "new" mapping was exactly the same as the old one.
2. Due to this, the 'updated_cpus' mask turned out to be empty after exiting
the 'for-loop' in arch_update_cpu_topology().
3. So we ended up calling stop-machine() with an empty cpumask list, which made
stop-machine internally elect cpumask_first(cpu_online_mask), i.e., CPU0 as
the cpu to run the payload (the update_cpu_topology() function).
4. This causes update_cpu_topology() to be run by CPU0. And since 'updates'
is kzalloc()'ed inside arch_update_cpu_topology(), update_cpu_topology()
finds update->cpu as well as update->new_nid to be 0. In other words, we
end up assigning CPU0 (and eventually its siblings) to node 0, incorrectly.
Along with the following wrong updating, it causes the sched-domain rebuild
code to break and crash the system.
Fix this by skipping the topology update in cases where we find that
the topology has not actually changed in reality (ie., spurious updates).
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Nathan Fontenot <nfont@linux.vnet.ibm.com>
CC: Stephen Rothwell <sfr@canb.auug.org.au>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Robert Jennings <rcj@linux.vnet.ibm.com>
CC: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
CC: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
CC: Alistair Popple <alistair@popple.id.au>
Suggested-by: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
---
arch/powerpc/mm/numa.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 30a42e2..4ebbb9e 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1591,6 +1591,20 @@ int arch_update_cpu_topology(void)
cpu = cpu_last_thread_sibling(cpu);
}
+ /*
+ * In cases where we have nothing to update (because the updates list
+ * is too short or because the new topology is same as the old one),
+ * skip invoking update_cpu_topology() via stop-machine(). This is
+ * necessary (and not just a fast-path optimization) since stop-machine
+ * can end up electing a random CPU to run update_cpu_topology(), and
+ * thus trick us into setting up incorrect cpu-node mappings (since
+ * 'updates' is kzalloc()'ed).
+ *
+ * And for the similar reason, we will skip all the following updating.
+ */
+ if (!cpumask_weight(&updated_cpus))
+ goto out;
+
stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
/*
@@ -1612,6 +1626,7 @@ int arch_update_cpu_topology(void)
changed = 1;
}
+out:
kfree(updates);
return changed;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2] powerpc/le: enable RTAS events support
From: Nathan Fontenot @ 2014-04-08 2:47 UTC (permalink / raw)
To: Greg Kurz, benh; +Cc: linux-kernel, paulus, anton, geert, linuxppc-dev
In-Reply-To: <20140404072750.20016.18969.stgit@bahia.local>
On 04/04/2014 02:35 AM, Greg Kurz wrote:
> The current kernel code assumes big endian and parses RTAS events all
> wrong. The most visible effect is that we cannot honor EPOW events,
> meaning, for example, we cannot shut down a guest properly from the
> hypervisor.
>
> This new patch is largely inspired by Nathan's work: we get rid of all
> the bit fields in the RTAS event structures (even the unused ones, for
> consistency). We also introduce endian safe accessors for the fields used
> by the kernel (trivial rtas_error_type() accessor added for consistency).
>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Looks good, thanks for getting this done Greg.
-Nathan
^ permalink raw reply
* Re: [PATCH] power, sched: stop updating inside arch_update_cpu_topology() when nothing to be update
From: Michael wang @ 2014-04-08 2:39 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: sfr, LKML, paulus, alistair, nfont, Andrew Morton, rcj,
linuxppc-dev
In-Reply-To: <53427AC1.9010801@linux.vnet.ibm.com>
Hi, Srivatsa
It's nice to have you confirmed the fix, and thanks for the well-writing
comments, will apply them and send out the new patch later :)
Regards,
Michael Wang
On 04/07/2014 06:15 PM, Srivatsa S. Bhat wrote:
> Hi Michael,
>
> On 04/02/2014 08:59 AM, Michael wang wrote:
>> During the testing, we encounter below WARN followed by Oops:
>>
>> WARNING: at kernel/sched/core.c:6218
>> ...
>> NIP [c000000000101660] .build_sched_domains+0x11d0/0x1200
>> LR [c000000000101358] .build_sched_domains+0xec8/0x1200
>> PACATMSCRATCH [800000000000f032]
>> Call Trace:
>> [c00000001b103850] [c000000000101358] .build_sched_domains+0xec8/0x1200
>> [c00000001b1039a0] [c00000000010aad4] .partition_sched_domains+0x484/0x510
>> [c00000001b103aa0] [c00000000016d0a8] .rebuild_sched_domains+0x68/0xa0
>> [c00000001b103b30] [c00000000005cbf0] .topology_work_fn+0x10/0x30
>> ...
>> Oops: Kernel access of bad area, sig: 11 [#1]
>> ...
>> NIP [c00000000045c000] .__bitmap_weight+0x60/0xf0
>> LR [c00000000010132c] .build_sched_domains+0xe9c/0x1200
>> PACATMSCRATCH [8000000000029032]
>> Call Trace:
>> [c00000001b1037a0] [c000000000288ff4] .kmem_cache_alloc_node_trace+0x184/0x3a0
>> [c00000001b103850] [c00000000010132c] .build_sched_domains+0xe9c/0x1200
>> [c00000001b1039a0] [c00000000010aad4] .partition_sched_domains+0x484/0x510
>> [c00000001b103aa0] [c00000000016d0a8] .rebuild_sched_domains+0x68/0xa0
>> [c00000001b103b30] [c00000000005cbf0] .topology_work_fn+0x10/0x30
>> ...
>>
>> This was caused by that 'sd->groups == NULL' after building groups, which
>> was caused by the empty 'sd->span'.
>>
>> The cpu's domain contain nothing because the cpu was assigned to wrong
>> node inside arch_update_cpu_topology() by calling update_lookup_table()
>> with the uninitialized param, in the case when there is nothing to be
>> update.
>>
>
> Can you reword the above paragraph to something like this:
>
> The cpu's domain contained nothing because the cpu was assigned to a wrong
> node, due to the following unfortunate sequence of events:
>
> 1. The hypervisor sent a topology update to the guest OS, to notify changes
> to the cpu-node mapping. However, the update was actually redundant - i.e.,
> the "new" mapping was exactly the same as the old one.
>
> 2. Due to this, the 'updated_cpus' mask turned out to be empty after exiting
> the 'for-loop' in arch_update_cpu_topology().
>
> 3. So we ended up calling stop-machine() with an empty cpumask list, which made
> stop-machine internally elect cpumask_first(cpu_online_mask), i.e., CPU0 as
> the cpu to run the payload (the update_cpu_topology() function).
>
> 4. This causes update_cpu_topology() to be run by CPU0. And since 'updates'
> is kzalloc()'ed inside arch_update_cpu_topology(), update_cpu_topology()
> finds update->cpu as well as update->new_nid to be 0. In other words, we
> end up assigning CPU0 (and eventually its siblings) to node 0, incorrectly.
>
> This causes the sched-domain rebuild code to break and crash the system.
>
>
>> Thus we should stop the updating in such cases, this patch will achieve
>> this and fix the issue.
>>
>
> We can reword this part as:
>
> Fix this by skipping the topology update in cases where we find that
> the topology has not actually changed in reality (ie., spurious updates).
>
>> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> CC: Paul Mackerras <paulus@samba.org>
>> CC: Nathan Fontenot <nfont@linux.vnet.ibm.com>
>> CC: Stephen Rothwell <sfr@canb.auug.org.au>
>> CC: Andrew Morton <akpm@linux-foundation.org>
>> CC: Robert Jennings <rcj@linux.vnet.ibm.com>
>> CC: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>> CC: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
>> CC: Alistair Popple <alistair@popple.id.au>
>> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/mm/numa.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> index 30a42e2..6757690 100644
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -1591,6 +1591,14 @@ int arch_update_cpu_topology(void)
>> cpu = cpu_last_thread_sibling(cpu);
>> }
>>
>> + /*
>> + * The 'cpu_associativity_changes_mask' could be cleared if
>> + * all the cpus it indicates won't change their node, in
>> + * which case the 'updated_cpus' will be empty.
>> + */
>
> How about rewording the comment like this:
>
> In cases where we have nothing to update (because the updates list
> is too short or because the new topology is same as the old one),
> skip invoking update_cpu_topology() via stop-machine(). This is
> necessary (and not just a fast-path optimization) because stop-machine
> can end up electing a random CPU to run update_cpu_topology(), and
> thus trick us into setting up incorrect cpu-node mappings (since
> 'updates' is kzalloc()'ed).
>
> Regards,
> Srivatsa S. Bhat
>
>> + if (!cpumask_weight(&updated_cpus))
>> + goto out;
>> +
>> stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
>>
>> /*
>> @@ -1612,6 +1620,7 @@ int arch_update_cpu_topology(void)
>> changed = 1;
>> }
>>
>> +out:
>> kfree(updates);
>> return changed;
>> }
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply
* Re: [PATCH v2] powerpc/le: enable RTAS events support
From: Stewart Smith @ 2014-04-07 23:21 UTC (permalink / raw)
To: Greg Kurz, benh; +Cc: linux-kernel, paulus, anton, nfont, geert, linuxppc-dev
In-Reply-To: <20140404072750.20016.18969.stgit@bahia.local>
Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
> The current kernel code assumes big endian and parses RTAS events all
> wrong. The most visible effect is that we cannot honor EPOW events,
> meaning, for example, we cannot shut down a guest properly from the
> hypervisor.
>
> This new patch is largely inspired by Nathan's work: we get rid of all
> the bit fields in the RTAS event structures (even the unused ones, for
> consistency). We also introduce endian safe accessors for the fields used
> by the kernel (trivial rtas_error_type() accessor added for consistency).
>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Acked-by: Stewart Smith <stewart@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [PATCH] powerpc/opal: Add missing include
From: Benjamin Herrenschmidt @ 2014-04-07 22:07 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Stephen Rothwell, Michael Neuling, Linux-Next, Linux PPC dev
In-Reply-To: <CAMuHMdV7BuSMu5gyhBcxY_8v+u3KnVStDgvC-JSN=hX3VZw3+g@mail.gmail.com>
On Mon, 2014-04-07 at 23:03 +0200, Geert Uytterhoeven wrote:
> On Tue, Mar 25, 2014 at 1:43 AM, Michael Neuling <mikey@neuling.org> wrote:
> > next-20140324 currently fails compiling celleb_defconfig with:
> >
> > arch/powerpc/include/asm/opal.h:894:42: error: 'struct notifier_block' declared inside parameter list [-Werror]
> > arch/powerpc/include/asm/opal.h:894:42: error: its scope is only this definition or declaration, which is probably not what you want [-Werror]
> > arch/powerpc/include/asm/opal.h:896:14: error: 'struct notifier_block' declared inside parameter list [-Werror]
> >
> > This is due to a missing include which is added here.
>
> This build issue has now been in mainline since a few days.
It's in my tree, I will send the patch upstream in the next couple of
days (having a medical procedure today).
Ben.
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> >
> > diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> > index fe2aa0b..f57fb58 100644
> > --- a/arch/powerpc/include/asm/opal.h
> > +++ b/arch/powerpc/include/asm/opal.h
> > @@ -177,6 +177,8 @@ extern int opal_enter_rtas(struct rtas_args *args,
> >
> > #ifndef __ASSEMBLY__
> >
> > +#include <linux/notifier.h>
> > +
> > /* Other enums */
> > enum OpalVendorApiTokens {
> > OPAL_START_VENDOR_API_RANGE = 1000, OPAL_END_VENDOR_API_RANGE = 1999
>
> 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
^ permalink raw reply
* Re: [PATCH] powerpc/opal: Add missing include
From: Geert Uytterhoeven @ 2014-04-07 21:03 UTC (permalink / raw)
To: Michael Neuling; +Cc: Linux PPC dev, Linux-Next, Stephen Rothwell
In-Reply-To: <13762.1395708188@ale.ozlabs.ibm.com>
On Tue, Mar 25, 2014 at 1:43 AM, Michael Neuling <mikey@neuling.org> wrote:
> next-20140324 currently fails compiling celleb_defconfig with:
>
> arch/powerpc/include/asm/opal.h:894:42: error: 'struct notifier_block' declared inside parameter list [-Werror]
> arch/powerpc/include/asm/opal.h:894:42: error: its scope is only this definition or declaration, which is probably not what you want [-Werror]
> arch/powerpc/include/asm/opal.h:896:14: error: 'struct notifier_block' declared inside parameter list [-Werror]
>
> This is due to a missing include which is added here.
This build issue has now been in mainline since a few days.
> Signed-off-by: Michael Neuling <mikey@neuling.org>
>
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index fe2aa0b..f57fb58 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -177,6 +177,8 @@ extern int opal_enter_rtas(struct rtas_args *args,
>
> #ifndef __ASSEMBLY__
>
> +#include <linux/notifier.h>
> +
> /* Other enums */
> enum OpalVendorApiTokens {
> OPAL_START_VENDOR_API_RANGE = 1000, OPAL_END_VENDOR_API_RANGE = 1999
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
^ permalink raw reply
* tools/testing/selftests/ptrace/peeksiginfo.c bug
From: Thierry FAUCK @ 2014-04-07 20:09 UTC (permalink / raw)
To: linuxppc-dev
Hello,
Here is a fix to allow compilation of
tools/testing/selftests/ptrace/peeksiginfo.c on ppc64/ppc64el.
Canonical is waiting for the patch to be upstream.
Thanks
Thierry
>From 48a9a9834377a74b603be12dcc76cda24105e33c Mon Sep 17 00:00:00 2001
From: Thierry Fauck <thierry@linux.vnet.ibm.com>
Date: Fri, 28 Feb 2014 16:17:50 +0100
Subject: [PATCH] power: PAGE_SIZE may not be defined
Some systems have a dynamic PAGE_SIZE value and do not add a definition
for PAGE_SIZE. This value will have to be retrieved using getpagesize()
or sysconf().
Signed-off-by: Thierry Fauck <thierry@linux.vnet.ibm.com>
---
tools/testing/selftests/ptrace/peeksiginfo.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/ptrace/peeksiginfo.c
b/tools/testing/selftests/ptrace/peeksiginfo.c
index d46558b..f2ccbbd 100644
--- a/tools/testing/selftests/ptrace/peeksiginfo.c
+++ b/tools/testing/selftests/ptrace/peeksiginfo.c
@@ -35,7 +35,9 @@ static int sys_ptrace(int request, pid_t pid, void
*addr, void *data)
fprintf(stderr, \
"Error (%s:%d): " fmt, \
__FILE__, __LINE__, ##__VA_ARGS__)
-
+#ifndef PAGE_SIZE
+#define PAGE_SIZE sysconf(_SC_PAGESIZE)
+#endif
static int check_error_paths(pid_t child)
{
struct ptrace_peeksiginfo_args arg;
--
1.9.0
^ permalink raw reply related
* Re: [1/2,v9] powerpc/mpc85xx:Add initial device tree support of T104x
From: Scott Wood @ 2014-04-07 18:22 UTC (permalink / raw)
To: Prabhakar Kushwaha
Cc: Varun Sethi, Poonam Aggrwal, linuxppc-dev, Priyanka Jain
In-Reply-To: <53415417.3010802@freescale.com>
On Sun, 2014-04-06 at 18:48 +0530, Prabhakar Kushwaha wrote:
> On 3/20/2014 4:03 AM, Scott Wood wrote:
> > On Sat, Jan 25, 2014 at 05:10:59PM +0530, Prabhakar Kushwaha wrote:
> >> + display@180000 {
> >> + compatible = "fsl,t1040-diu", "fsl,diu";
> >> + reg = <0x180000 1000>;
> >> + interrupts = <74 2 0 0>;
> >> + };
> >> +
> >> +/include/ "qoriq-sata2-0.dtsi"
> >> +sata@220000 {
> >> + fsl,iommu-parent = <&pamu0>;
> >> + fsl,liodn-reg = <&guts 0x550>; /* SATA1LIODNR */
> >> +};
> >> +/include/ "qoriq-sata2-1.dtsi"
> >> +sata@221000 {
> >> + fsl,iommu-parent = <&pamu0>;
> >> + fsl,liodn-reg = <&guts 0x554>; /* SATA2LIODNR */
> >> +};
> >> +/include/ "qoriq-sec5.0-0.dtsi"
> >> +};
> > Whitespace
> >
> >
> i did not find this whitespace :(
Add a tab to the sata nodes' name and braces, and remove a tab from the
sata nodes' content.
-Scott
^ permalink raw reply
* Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
From: Neil Horman @ 2014-04-07 15:36 UTC (permalink / raw)
To: Gerhard Sittig
Cc: Yann Droneaud, linux-kernel, Neil Horman, cscope-devel,
Anton Blanchard, Hans-Bernhard Broeker, linuxppc-dev,
Hans-Bernhard Bröker
In-Reply-To: <20140407124259.GZ11339@book.gsilab.sittig.org>
On Mon, Apr 07, 2014 at 02:42:59PM +0200, Gerhard Sittig wrote:
> On Mon, 2014-04-07 at 06:42 -0400, Neil Horman wrote:
> >
> > On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote:
> > >
> > > [ ... ]
> > >
> > > cscope reports error when generating the cross-reference database:
> > >
> > > $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope
> > > GEN cscope
> > > cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
> > > cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S
> > > cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
> > > cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S
> > >
> > > And when calling cscope from ./obj-cscope/ directory, it reports errors
> > > too.
> > >
> > > Hopefully it doesn't stop it from working, so I'm still able to use
> > > cscope to browse kernel sources.
> > >
> > No, it won't stop it from working, it just won't search those files. I don't
> > recall exactly the reason, but IIRC there was a big discussion long ago about
> > symlinks and our ability to support them (around version 1.94 I think). We
> > decided to not handle symlinks, as they would either point outside our search
> > tree, which we didn't want to include, or would point to another file in the
> > search tree, which made loading them pointless (as we would cover the search in
> > the pointed file).
>
> So there are valid reasons to not process those filesystem
> entries. Would it be useful to not emit the warnings then? Or
> to silent those warnings when the user knows it's perfectly legal
> to skip those filesytem entries? Like what you can do with the
> ctags(1) command and its --links option.
>
I would see no problem with an option to do that. I'd like to make it opt-in,
so that people who want to know about symlink issues will still see them, but
I'd be supportive of an option to quiet them
Neil
>
> virtually yours
> Gerhard Sittig
> --
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
>
^ permalink raw reply
* Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
From: Gerhard Sittig @ 2014-04-07 12:42 UTC (permalink / raw)
To: Neil Horman
Cc: Yann Droneaud, linux-kernel, Neil Horman, cscope-devel,
Anton Blanchard, Hans-Bernhard Broeker, linuxppc-dev,
Hans-Bernhard Bröker
In-Reply-To: <20140407104216.GB5287@hmsreliant.think-freely.org>
On Mon, 2014-04-07 at 06:42 -0400, Neil Horman wrote:
>
> On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote:
> >
> > [ ... ]
> >
> > cscope reports error when generating the cross-reference database:
> >
> > $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope
> > GEN cscope
> > cscope: cannot find
> > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
> > cscope: cannot find
> > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S
> > cscope: cannot find
> > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
> > cscope: cannot find
> > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S
> >
> > And when calling cscope from ./obj-cscope/ directory, it reports errors
> > too.
> >
> > Hopefully it doesn't stop it from working, so I'm still able to use
> > cscope to browse kernel sources.
> >
> No, it won't stop it from working, it just won't search those files. I don't
> recall exactly the reason, but IIRC there was a big discussion long ago about
> symlinks and our ability to support them (around version 1.94 I think). We
> decided to not handle symlinks, as they would either point outside our search
> tree, which we didn't want to include, or would point to another file in the
> search tree, which made loading them pointless (as we would cover the search in
> the pointed file).
So there are valid reasons to not process those filesystem
entries. Would it be useful to not emit the warnings then? Or
to silent those warnings when the user knows it's perfectly legal
to skip those filesytem entries? Like what you can do with the
ctags(1) command and its --links option.
virtually yours
Gerhard Sittig
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
^ permalink raw reply
* Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
From: Neil Horman @ 2014-04-07 10:42 UTC (permalink / raw)
To: Yann Droneaud
Cc: Hans-Bernhard Bröker, linux-kernel, Neil Horman,
cscope-devel, Anton Blanchard, linuxppc-dev,
Hans-Bernhard Broeker
In-Reply-To: <1396530975.4361.28.camel@localhost.localdomain>
On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote:
> Hi,
>
> I'm using cscope to browse kernel sources, but I'm facing warnings from
> the tool since following commit:
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=22d651dcef536c75f75537290bf3da5038e68b6b
>
> commit 22d651dcef536c75f75537290bf3da5038e68b6b
> Author: Michael Ellerman <mpe@ellerman.id.au>
> Date: Tue Jan 21 15:22:17 2014 +1100
>
> selftests/powerpc: Import Anton's memcpy / copy_tofrom_user tests
>
> Turn Anton's memcpy / copy_tofrom_user test into something that can
> live in tools/testing/selftests.
>
> It requires one turd in arch/powerpc/lib/memcpy_64.S, but it's
> pretty harmless IMHO.
>
> We are sailing very close to the wind with the feature macros. We
> define them to nothing, which currently means we get a few extra
> nops and include the unaligned calls.
>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
>
> cscope reports error when generating the cross-reference database:
>
> $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope
> GEN cscope
> cscope: cannot find
> file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
> cscope: cannot find
> file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S
> cscope: cannot find
> file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
> cscope: cannot find
> file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S
>
> And when calling cscope from ./obj-cscope/ directory, it reports errors
> too.
>
> Hopefully it doesn't stop it from working, so I'm still able to use
> cscope to browse kernel sources.
>
No, it won't stop it from working, it just won't search those files. I don't
recall exactly the reason, but IIRC there was a big discussion long ago about
symlinks and our ability to support them (around version 1.94 I think). We
decided to not handle symlinks, as they would either point outside our search
tree, which we didn't want to include, or would point to another file in the
search tree, which made loading them pointless (as we would cover the search in
the pointed file).
Neil
> It's a rather uncommon side effect of having (for the first time ?)
> sources files as symlinks: looking for symlinks in the kernel sources
> returns only:
>
> $ find . -type l
> ./arch/mips/boot/dts/include/dt-bindings
> ./arch/microblaze/boot/dts/system.dts
> ./arch/powerpc/boot/dts/include/dt-bindings
> ./arch/metag/boot/dts/include/dt-bindings
> ./arch/arm/boot/dts/include/dt-bindings
> ./tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
> ./tools/testing/selftests/powerpc/copyloops/memcpy_64.S
> ./tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
> ./tools/testing/selftests/powerpc/copyloops/copyuser_64.S
> ./obj-cscope/source
> ./Documentation/DocBook/vidioc-g-sliced-vbi-cap.xml
> ./Documentation/DocBook/vidioc-decoder-cmd.xml
> ...
> ./Documentation/DocBook/media-func-ioctl.xml
> ./Documentation/DocBook/vidioc-enumoutput.xml
>
>
> So one can wonder if having symlinked sources files is an expected
> supported feature for kbuild and all the various kernel
> tools/infrastructure ?
>
> Regarding cscope specifically, it does not support symlink, and it's the
> expected behavior according to the bug reports I was able to find:
>
> #214 cscope ignores symlinks to files
> http://sourceforge.net/p/cscope/bugs/214/
>
> #229 -I options doesn't handle symbolic link
> http://sourceforge.net/p/cscope/bugs/229/
>
> #247 cscope: cannot find file
> http://sourceforge.net/p/cscope/bugs/247/
>
> #252 cscope: cannot find file ***
> http://sourceforge.net/p/cscope/bugs/252/
>
> #261 Regression - version 15.7a does not follow symbolic links
> http://sourceforge.net/p/cscope/bugs/261/
>
>
> Regards.
>
> --
> Yann Droneaud
> OPTEYA
>
>
>
^ permalink raw reply
* Re: [PATCH] power, sched: stop updating inside arch_update_cpu_topology() when nothing to be update
From: Srivatsa S. Bhat @ 2014-04-07 10:15 UTC (permalink / raw)
To: Michael wang
Cc: sfr, LKML, paulus, alistair, nfont, Andrew Morton, rcj,
linuxppc-dev
In-Reply-To: <533B8431.8090507@linux.vnet.ibm.com>
Hi Michael,
On 04/02/2014 08:59 AM, Michael wang wrote:
> During the testing, we encounter below WARN followed by Oops:
>
> WARNING: at kernel/sched/core.c:6218
> ...
> NIP [c000000000101660] .build_sched_domains+0x11d0/0x1200
> LR [c000000000101358] .build_sched_domains+0xec8/0x1200
> PACATMSCRATCH [800000000000f032]
> Call Trace:
> [c00000001b103850] [c000000000101358] .build_sched_domains+0xec8/0x1200
> [c00000001b1039a0] [c00000000010aad4] .partition_sched_domains+0x484/0x510
> [c00000001b103aa0] [c00000000016d0a8] .rebuild_sched_domains+0x68/0xa0
> [c00000001b103b30] [c00000000005cbf0] .topology_work_fn+0x10/0x30
> ...
> Oops: Kernel access of bad area, sig: 11 [#1]
> ...
> NIP [c00000000045c000] .__bitmap_weight+0x60/0xf0
> LR [c00000000010132c] .build_sched_domains+0xe9c/0x1200
> PACATMSCRATCH [8000000000029032]
> Call Trace:
> [c00000001b1037a0] [c000000000288ff4] .kmem_cache_alloc_node_trace+0x184/0x3a0
> [c00000001b103850] [c00000000010132c] .build_sched_domains+0xe9c/0x1200
> [c00000001b1039a0] [c00000000010aad4] .partition_sched_domains+0x484/0x510
> [c00000001b103aa0] [c00000000016d0a8] .rebuild_sched_domains+0x68/0xa0
> [c00000001b103b30] [c00000000005cbf0] .topology_work_fn+0x10/0x30
> ...
>
> This was caused by that 'sd->groups == NULL' after building groups, which
> was caused by the empty 'sd->span'.
>
> The cpu's domain contain nothing because the cpu was assigned to wrong
> node inside arch_update_cpu_topology() by calling update_lookup_table()
> with the uninitialized param, in the case when there is nothing to be
> update.
>
Can you reword the above paragraph to something like this:
The cpu's domain contained nothing because the cpu was assigned to a wrong
node, due to the following unfortunate sequence of events:
1. The hypervisor sent a topology update to the guest OS, to notify changes
to the cpu-node mapping. However, the update was actually redundant - i.e.,
the "new" mapping was exactly the same as the old one.
2. Due to this, the 'updated_cpus' mask turned out to be empty after exiting
the 'for-loop' in arch_update_cpu_topology().
3. So we ended up calling stop-machine() with an empty cpumask list, which made
stop-machine internally elect cpumask_first(cpu_online_mask), i.e., CPU0 as
the cpu to run the payload (the update_cpu_topology() function).
4. This causes update_cpu_topology() to be run by CPU0. And since 'updates'
is kzalloc()'ed inside arch_update_cpu_topology(), update_cpu_topology()
finds update->cpu as well as update->new_nid to be 0. In other words, we
end up assigning CPU0 (and eventually its siblings) to node 0, incorrectly.
This causes the sched-domain rebuild code to break and crash the system.
> Thus we should stop the updating in such cases, this patch will achieve
> this and fix the issue.
>
We can reword this part as:
Fix this by skipping the topology update in cases where we find that
the topology has not actually changed in reality (ie., spurious updates).
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: Paul Mackerras <paulus@samba.org>
> CC: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> CC: Stephen Rothwell <sfr@canb.auug.org.au>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Robert Jennings <rcj@linux.vnet.ibm.com>
> CC: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
> CC: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
> CC: Alistair Popple <alistair@popple.id.au>
> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/numa.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 30a42e2..6757690 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1591,6 +1591,14 @@ int arch_update_cpu_topology(void)
> cpu = cpu_last_thread_sibling(cpu);
> }
>
> + /*
> + * The 'cpu_associativity_changes_mask' could be cleared if
> + * all the cpus it indicates won't change their node, in
> + * which case the 'updated_cpus' will be empty.
> + */
How about rewording the comment like this:
In cases where we have nothing to update (because the updates list
is too short or because the new topology is same as the old one),
skip invoking update_cpu_topology() via stop-machine(). This is
necessary (and not just a fast-path optimization) because stop-machine
can end up electing a random CPU to run update_cpu_topology(), and
thus trick us into setting up incorrect cpu-node mappings (since
'updates' is kzalloc()'ed).
Regards,
Srivatsa S. Bhat
> + if (!cpumask_weight(&updated_cpus))
> + goto out;
> +
> stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
>
> /*
> @@ -1612,6 +1620,7 @@ int arch_update_cpu_topology(void)
> changed = 1;
> }
>
> +out:
> kfree(updates);
> return changed;
> }
>
^ permalink raw reply
* Re: [PATCH] power, sched: stop updating inside arch_update_cpu_topology() when nothing to be update
From: Srivatsa S. Bhat @ 2014-04-07 9:51 UTC (permalink / raw)
To: Michael wang
Cc: sfr, Srikar Dronamraju, LKML, jlarrew, paulus, alistair, nfont,
Andrew Morton, rcj, linuxppc-dev
In-Reply-To: <533E2BA2.6000107@linux.vnet.ibm.com>
Hi Michael,
On 04/04/2014 09:18 AM, Michael wang wrote:
> Hi, Srivatsa
>
> Thanks for your reply :)
>
> On 04/03/2014 04:50 PM, Srivatsa S. Bhat wrote:
> [snip]
>>
>> Now, the interesting thing to note here is that, if CPU0's node was already
>> set as node0, *nothing* should go wrong, since its just a redundant update.
>> However, if CPU0's original node mapping was something different, or if
>> node0 doesn't even exist in the machine, then the system can crash.
>
> By printk I confirmed all cpus was belong to node 1 at very beginning,
> and things become magically after the wrong updating...
>
Ok, thanks!
>>
>> Have you verified that CPU0's node mapping is different from node 0?
>> That is, boot the kernel with "numa=debug" in the kernel command line and
>> it will print out the cpu-to-node associativity during boot. That way you
>> can figure out what was the original associativity that was set. This will
>> confirm the theory that the hypervisor sent a redundant update, but because
>> of the weird pre-allocation using kzalloc that we do inside
>> arch_update_cpu_topology(), we wrongly updated CPU0's mapping as CPU0 <-> Node0.
>
> Associativity should changes, otherwise we won't continue the updating,
> and empty updates[] was confirmed to show up inside
> arch_update_cpu_topology().
>
Ah, ok, that makes it very clear. So, I agree that your patch is correct,
but I think the comment in your patch can be enhanced a bit. I'll suggest
something if I manage to come up with a better wording.
> What I can't make sure is whether this is legal, notify changes but no
> changes happen sounds weird...however, even if it's legal, a check in
> here still make sense IMHO.
>
That looks like a bug in the hypervisor/firmware. But the Linux kernel should
be able to handle such NULL updates without crashing. So yes, your patch makes
sense to me.
Thank you!
Regards,
Srivatsa S. Bhat
>>
>>> Thus we should stop the updating in such cases, this patch will achieve
>>> this and fix the issue.
>>>
>>> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> CC: Paul Mackerras <paulus@samba.org>
>>> CC: Nathan Fontenot <nfont@linux.vnet.ibm.com>
>>> CC: Stephen Rothwell <sfr@canb.auug.org.au>
>>> CC: Andrew Morton <akpm@linux-foundation.org>
>>> CC: Robert Jennings <rcj@linux.vnet.ibm.com>
>>> CC: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>>> CC: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
>>> CC: Alistair Popple <alistair@popple.id.au>
>>> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/mm/numa.c | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>>> index 30a42e2..6757690 100644
>>> --- a/arch/powerpc/mm/numa.c
>>> +++ b/arch/powerpc/mm/numa.c
>>> @@ -1591,6 +1591,14 @@ int arch_update_cpu_topology(void)
>>> cpu = cpu_last_thread_sibling(cpu);
>>> }
>>>
>>> + /*
>>> + * The 'cpu_associativity_changes_mask' could be cleared if
>>> + * all the cpus it indicates won't change their node, in
>>> + * which case the 'updated_cpus' will be empty.
>>> + */
>>> + if (!cpumask_weight(&updated_cpus))
>>> + goto out;
>>> +
>>> stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
>>>
>>> /*
>>> @@ -1612,6 +1620,7 @@ int arch_update_cpu_topology(void)
>>> changed = 1;
>>> }
>>>
>>> +out:
>>> kfree(updates);
>>> return changed;
>>> }
>>>
>>
^ permalink raw reply
* Re: [PATCH v2] powernv: kvm: make _PAGE_NUMA take effect
From: Alexander Graf @ 2014-04-07 8:36 UTC (permalink / raw)
To: Aneesh Kumar K.V, Alexander Graf, Liu ping fan
Cc: Paul Mackerras, linuxppc-dev, kvm-devel, kvm-ppc
In-Reply-To: <87bnwdshzu.fsf@linux.vnet.ibm.com>
On 07.04.14 09:42, Aneesh Kumar K.V wrote:
> Alexander Graf <agraf@suse.com> writes:
>
>> On 03.04.14 04:36, Liu ping fan wrote:
>>> Hi Alex, could you help to pick up this patch? since v3.14 kernel can
>>> enable numa fault for powerpc.
>> What bad happens without this patch? We map a page even though it was
>> declared to get NUMA migrated? What happens next?
> Nothing much, we won't be properly accounting the numa access in the
> host. What we want to achieve is to convert a guest access of the page to
> a host fault so that we can do proper numa access accounting in the
> host. This would enable us to migrate the page to the correct numa
> node.
Ok, so no breakages, just less performance. I wouldn't consider it
stable material then :).
Alex
^ permalink raw reply
* Re: [PATCH v2] powernv: kvm: make _PAGE_NUMA take effect
From: Aneesh Kumar K.V @ 2014-04-07 7:42 UTC (permalink / raw)
To: Alexander Graf, Liu ping fan, Alexander Graf
Cc: Paul Mackerras, linuxppc-dev, kvm-devel, kvm-ppc
In-Reply-To: <533D47CD.906@suse.com>
Alexander Graf <agraf@suse.com> writes:
> On 03.04.14 04:36, Liu ping fan wrote:
>> Hi Alex, could you help to pick up this patch? since v3.14 kernel can
>> enable numa fault for powerpc.
>
> What bad happens without this patch? We map a page even though it was
> declared to get NUMA migrated? What happens next?
Nothing much, we won't be properly accounting the numa access in the
host. What we want to achieve is to convert a guest access of the page to
a host fault so that we can do proper numa access accounting in the
host. This would enable us to migrate the page to the correct numa
node.
>
> I'm trying to figure out whether I need to mark this with a stable tag
> for 3.14.
>
>
-aneesh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox