* Re: [PATCH RFC v6 4/5] dma: mpc512x: register for device tree channel lookup
From: Alexander Popov @ 2014-01-13 8:17 UTC (permalink / raw)
To: Vinod Koul
Cc: Lars-Peter Clausen, Arnd Bergmann, Gerhard Sittig,
Alexander Popov, dmaengine, Dan Williams, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <20140109111957.GE16227@intel.com>
Thanks for your replies, Gerhard and Vinod.
2014/1/9 Vinod Koul <vinod.koul@intel.com>:
> On Wed, Jan 08, 2014 at 05:47:19PM +0100, Gerhard Sittig wrote:
>> [ what is the semantics of DMA_PRIVATE capability flag?
>> is documentation available beyond the initial commit message?
>> need individual channels be handled instead of controllers? ]
>
> The DMA_PRIVATE means that your channels are not to be used for global memcpy,
> as one can do in async cases (this is hwere DMAengine came into existence)
>
> If the device has the capablity of doing genric memcpy then it should not set
> this. For slave dma usage the dam channel can transfer data to a specfic
> slave device(s), hence we should use this is geric fashion so setting
> DMA_PRIVATE makes sense in those cases.
Each DMA channel of MPC512x DMA controller can do _both_
mem-to-mem transfers and transfers between mem and some slave peripheral
(only one DMA channel is fully dedicated to DDR).
All DMA channels of MPC512x DMA controller belong to one dma_device.
So we _don't_ need setting DMA_PRIVATE flag for this dma_device at all, do we?
>> On Sat, Jan 04, 2014 at 00:54 +0400, Alexander Popov wrote:
>> > I've involved DMA_PRIVATE flag because new of_dma_xlate_by_chan_id()
>> > uses dma_get_slave_channel() instead of dma_request_channel()
>> > (PATCH RFC v6 3/5). This flag is implicitly set in dma_request_channel(),
>> > but is not set in dma_get_slave_channel().
> Which makes me thing you are targetting slave usages. Do you intend to use for
> mempcy too on all controllers you support. in that case you should set it
> selectively.
Vinod, please correct me if I'm wrong.
As I could understand from your comments and the code,
DMA_PRIVATE flag is needed for dma_devices with DMA channels
which _can_ work with slave peripheral but _can't_ do mem-to-mem transfers.
If DMA_PRIVATE flag is set for some dma_device before
dma_async_device_register()
then its DMA channels are not published in tables for kernel slab allocator
(because these channels are simply useless for memcpy).
>> Still I see a difference in the lookup approaches: Yours applies
>> DMA_PRIVATE globally and in advance, preventing _any_ use of DMA
>> for memory transfers. While the __dma_request_channel() routine
>> only applies it _temporarily_ around a dma_chan_get() operation.
>> Allowing for use of DMA channels by both individual peripherals
>> as well as memory transfers.
>>
> No it doesnt prevent. You can still use it for memcpy once you have the channel.
Excuse me, I don't completely understand why dma_request_channel()
needs to set DMA_PRIVATE flag.
If dma_request_channel() for some dma_device without DMA_PRIVATE
is called before the first dmaengine_get()
then no DMA channels of this dma_device will become available for memcpy
by slab allocator.
Could you give me a clue?
>> > > Consider the fact that this driver
>> > > handles both MPC5121 as well as MPC8308 hardware.
>> >
>> > Ah, yes, sorry. I should certainly fix this, if setting of DMA_PRIVATE flag
>> > is needed at all.
>>
>> What I meant here is that implications for all affected platforms
>> should be considered. There is one driver source, but the driver
>> applies to more than one platform (another issue of the driver is
>> that this is not apparent from the doc nor the compat strings).
I'll add a comment with information about the supported platforms to
mpc512x_dma.c
in RFC PATCH 1/5. Ok?
>> So blocking memory transfers in mpc512x_dma.c is a total breakage
>> for MPC8308 (removes the only previous feature and adds nothing),
>> and is a regression for MPC512x (removes the previously supported
>> memory transfers, while it may add peripheral supports with very
>> few users).
Yes, I see. MPC512x and MPC8308 should be treated differently.
Thanks!
Alexander
^ permalink raw reply
* [PATCH v9] clk: corenet: Adds the clock binding
From: Tang Yuantian @ 2014-01-13 8:16 UTC (permalink / raw)
To: b07421; +Cc: mark.rutland, devicetree, Tang Yuantian, linuxppc-dev
From: Tang Yuantian <yuantian.tang@freescale.com>
Adds the clock bindings for Freescale PowerPC CoreNet platforms
Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
v9:
- refined some properties' description
v8:
- added clock-frequency property description
- fixed whitespace and tab mixing issue
v7:
- refined some properties' definitions
v6:
- splited the previous patch into 2 parts, one is for binding(this one),
the other is for DTS modification(will submit once this gets accepted)
- fixed typo
- refined #clock-cells and clock-output-names properties
- removed fixed-clock compatible string
v5:
- refine the binding document
- update the compatible string
v4:
- add binding document
- update compatible string
- update the reg property
v3:
- fix typo
v2:
- add t4240, b4420, b4860 support
- remove pll/4 clock from p2041, p3041 and p5020 board
.../devicetree/bindings/clock/corenet-clock.txt | 134 +++++++++++++++++++++
1 file changed, 134 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/corenet-clock.txt
diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt b/Documentation/devicetree/bindings/clock/corenet-clock.txt
new file mode 100644
index 0000000..8394bc7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt
@@ -0,0 +1,134 @@
+* Clock Block on Freescale CoreNet Platforms
+
+Freescale CoreNet chips take primary clocking input from the external
+SYSCLK signal. The SYSCLK input (frequency) is multiplied using
+multiple phase locked loops (PLL) to create a variety of frequencies
+which can then be passed to a variety of internal logic, including
+cores and peripheral IP blocks.
+Please refer to the Reference Manual for details.
+
+1. Clock Block Binding
+
+Required properties:
+- compatible: Should contain a specific clock block compatible string
+ and a single chassis clock compatible string.
+ Clock block strings include, but not limited to, one of the:
+ * "fsl,p2041-clockgen"
+ * "fsl,p3041-clockgen"
+ * "fsl,p4080-clockgen"
+ * "fsl,p5020-clockgen"
+ * "fsl,p5040-clockgen"
+ * "fsl,t4240-clockgen"
+ * "fsl,b4420-clockgen"
+ * "fsl,b4860-clockgen"
+ Chassis clock strings include:
+ * "fsl,qoriq-clockgen-1.0": for chassis 1.0 clocks
+ * "fsl,qoriq-clockgen-2.0": for chassis 2.0 clocks
+- reg: Describes the address of the device's resources within the
+ address space defined by its parent bus, and resource zero
+ represents the clock register set
+- clock-frequency: Input system clock frequency
+
+Recommended properties:
+- ranges: Allows valid translation between child's address space and
+ parent's. Must be present if the device has sub-nodes.
+- #address-cells: Specifies the number of cells used to represent
+ physical base addresses. Must be present if the device has
+ sub-nodes and set to 1 if present
+- #size-cells: Specifies the number of cells used to represent
+ the size of an address. Must be present if the device has
+ sub-nodes and set to 1 if present
+
+2. Clock Provider/Consumer Binding
+
+Most of the bindings are from the common clock binding[1].
+ [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : Should include one of the following:
+ * "fsl,qoriq-core-pll-1.0" for core PLL clocks (v1.0)
+ * "fsl,qoriq-core-pll-2.0" for core PLL clocks (v2.0)
+ * "fsl,qoriq-core-mux-1.0" for core mux clocks (v1.0)
+ * "fsl,qoriq-core-mux-2.0" for core mux clocks (v2.0)
+ * "fsl,qoriq-sysclk-1.0": for input system clock (v1.0).
+ It takes parent's clock-frequency as its clock.
+ * "fsl,qoriq-sysclk-2.0": for input system clock (v2.0).
+ It takes parent's clock-frequency as its clock.
+- #clock-cells: From common clock binding. The number of cells in a
+ clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"
+ clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.
+ For "fsl,qoriq-core-pll-[1,2].0" clocks, the single
+ clock-specifier cell may take the following values:
+ * 0 - equal to the PLL frequency
+ * 1 - equal to the PLL frequency divided by 2
+ * 2 - equal to the PLL frequency divided by 4
+
+Recommended properties:
+- clocks: Should be the phandle of input parent clock
+- clock-names: From common clock binding, indicates the clock name
+- clock-output-names: From common clock binding, indicates the names of
+ output clocks
+- reg: Should be the offset and length of clock block base address.
+ The length should be 4.
+
+Example for clock block and clock provider:
+/ {
+ clockgen: global-utilities@e1000 {
+ compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0";
+ ranges = <0x0 0xe1000 0x1000>;
+ clock-frequency = 133333;
+ reg = <0xe1000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ sysclk: sysclk {
+ #clock-cells = <0>;
+ compatible = "fsl,qoriq-sysclk-1.0";
+ clock-output-names = "sysclk";
+ }
+
+ pll0: pll0@800 {
+ #clock-cells = <1>;
+ reg = <0x800 0x4>;
+ compatible = "fsl,qoriq-core-pll-1.0";
+ clocks = <&sysclk>;
+ clock-output-names = "pll0", "pll0-div2";
+ };
+
+ pll1: pll1@820 {
+ #clock-cells = <1>;
+ reg = <0x820 0x4>;
+ compatible = "fsl,qoriq-core-pll-1.0";
+ clocks = <&sysclk>;
+ clock-output-names = "pll1", "pll1-div2";
+ };
+
+ mux0: mux0@0 {
+ #clock-cells = <0>;
+ reg = <0x0 0x4>;
+ compatible = "fsl,qoriq-core-mux-1.0";
+ clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>;
+ clock-names = "pll0", "pll0-div2", "pll1", "pll1-div2";
+ clock-output-names = "cmux0";
+ };
+
+ mux1: mux1@20 {
+ #clock-cells = <0>;
+ reg = <0x20 0x4>;
+ compatible = "fsl,qoriq-core-mux-1.0";
+ clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>;
+ clock-names = "pll0", "pll0-div2", "pll1", "pll1-div2";
+ clock-output-names = "cmux1";
+ };
+ };
+ }
+
+Example for clock consumer:
+
+/ {
+ cpu0: PowerPC,e5500@0 {
+ ...
+ clocks = <&mux0>;
+ ...
+ };
+ }
--
1.8.0
^ permalink raw reply related
* Re: [PATCH 02/13] ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
From: Yann Droneaud @ 2014-01-13 9:30 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: cbe-oss-dev, linuxppc-dev, linux-kernel
In-Reply-To: <1389567961.4672.111.camel@pasglop>
Hi Benjamin,
Le lundi 13 janvier 2014 à 10:06 +1100, Benjamin Herrenschmidt a écrit :
> On Tue, 2013-07-02 at 18:39 +0200, Yann Droneaud wrote:
> > Macro get_unused_fd() is used to allocate a file descriptor with
> > default flags. Those default flags (0) can be "unsafe":
> > O_CLOEXEC must be used by default to not leak file descriptor
> > across exec().
> >
> > Instead of macro get_unused_fd(), functions anon_inode_getfd()
> > or get_unused_fd_flags() should be used with flags given by userspace.
> > If not possible, flags should be set to O_CLOEXEC to provide userspace
> > with a default safe behavor.
> >
> > In a further patch, get_unused_fd() will be removed so that
> > new code start using anon_inode_getfd() or get_unused_fd_flags()
> > with correct flags.
> >
> > This patch replaces calls to get_unused_fd() with equivalent call to
> > get_unused_fd_flags(0) to preserve current behavor for existing code.
> >
> > The hard coded flag value (0) should be reviewed on a per-subsystem basis,
> > and, if possible, set to O_CLOEXEC.
> >
> > Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
>
> Should I merge this (v5 on patchwork) or let Al do it ?
>
Please merge it directly: patches from the previous patchsets were
picked individually by each subsystem maintainer after proper review
regarding setting close on exec flag by default.
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
Thanks a lot.
> > ---
> > arch/powerpc/platforms/cell/spufs/inode.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
> > index f390042..88df441 100644
> > --- a/arch/powerpc/platforms/cell/spufs/inode.c
> > +++ b/arch/powerpc/platforms/cell/spufs/inode.c
> > @@ -301,7 +301,7 @@ static int spufs_context_open(struct path *path)
> > int ret;
> > struct file *filp;
> >
> > - ret = get_unused_fd();
> > + ret = get_unused_fd_flags(0);
> > if (ret < 0)
> > return ret;
> >
> > @@ -518,7 +518,7 @@ static int spufs_gang_open(struct path *path)
> > int ret;
> > struct file *filp;
> >
> > - ret = get_unused_fd();
> > + ret = get_unused_fd_flags(0);
> > if (ret < 0)
> > return ret;
> >
>
>
Note:
latest patch (from v5 patchset) is at
http://lkml.kernel.org/r/fe27abcfab5563d36a3e5e58ff36e5500c39be6a.1388952061.git.ydroneaud@opteya.com
v5 patchset is at
http://lkml.kernel.org/r/cover.1388952061.git.ydroneaud@opteya.com
Regards.
--
Yann Droneaud
OPTEYA
^ permalink raw reply
* Re: [PATCH V4] powerpc: thp: Fix crash on mremap
From: Aneesh Kumar K.V @ 2014-01-13 9:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, aarcange
Cc: aarcange, linuxppc-dev, paulus, kirill.shutemov, linux-mm
In-Reply-To: <1389598587.4672.121.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> On Mon, 2014-01-13 at 11:34 +0530, Aneesh Kumar K.V wrote:
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> This patch fix the below crash
>
> Andrea, can you ack the generic bit please ?
>
> Thanks !
Kirill A. Shutemov did ack an earlier version
http://article.gmane.org/gmane.linux.kernel.mm/111368
-aneesh
^ permalink raw reply
* Re: [PATCH V4] powerpc: thp: Fix crash on mremap
From: Benjamin Herrenschmidt @ 2014-01-13 13:37 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: aarcange, linuxppc-dev, paulus, kirill.shutemov, linux-mm
In-Reply-To: <87wqi42p0f.fsf@linux.vnet.ibm.com>
On Mon, 2014-01-13 at 15:16 +0530, Aneesh Kumar K.V wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
>
> > On Mon, 2014-01-13 at 11:34 +0530, Aneesh Kumar K.V wrote:
> >> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> >>
> >> This patch fix the below crash
> >
> > Andrea, can you ack the generic bit please ?
> >
> > Thanks !
>
> Kirill A. Shutemov did ack an earlier version
>
> http://article.gmane.org/gmane.linux.kernel.mm/111368
Doesn't help. If I'm going to send Linus a patch with a generic change
like that, I need an ack of that exact version of the change by a senior
mm person such as Andrea.
Cheers,
Ben.
^ permalink raw reply
* [PATCH 0/4] remap non-modular uses of module_init properly
From: Paul Gortmaker @ 2014-01-13 16:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: Paul Gortmaker, linuxppc-dev
The goal is to move module_init/module_exit from init.h and into
module.h -- however in doing so, we uncover several instances in
powerpc code where module_init is used somewhat incorrectly by
non modular code, and a file that needs module.h but isn't sourcing
it. We need to make these fixups 1st before changing the headers
so that we don't cause build failures.
The changes are largely inert, however we do cause a largely trivial
change in the initcall ordering -- that happens because module_init
is really device_initcall; and yet we shouldn't be using device_initcall
where clearly arch_initcall or subsys_initcall are more appropriate.
Boot tested on sbc8548 on powerpc next branch of today.
Paul Gortmaker (4):
powerpc: use device_initcall for registering rtc devices
powerpc: book3s kvm can be modular so it should use module.h
powerpc: use subsys_initcall for Freescale Local Bus
powerpc: don't use module_init for non-modular core hugetlb code
arch/powerpc/kernel/time.c | 2 +-
arch/powerpc/kvm/book3s.c | 2 +-
arch/powerpc/mm/hugetlbpage.c | 2 +-
arch/powerpc/platforms/ps3/time.c | 3 +--
arch/powerpc/sysdev/fsl_lbc.c | 2 +-
5 files changed, 5 insertions(+), 6 deletions(-)
--
1.8.5.2
^ permalink raw reply
* [PATCH 1/4] powerpc: use device_initcall for registering rtc devices
From: Paul Gortmaker @ 2014-01-13 16:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: Paul Gortmaker, linuxppc-dev
In-Reply-To: <1389630113-7919-1-git-send-email-paul.gortmaker@windriver.com>
Currently these two RTC devices are in core platform code
where it is not possible for them to be modular. It will
never be modular, so using module_init as an alias for
__initcall can be somewhat misleading.
Fix this up now, so that we can relocate module_init from
init.h into module.h in the future. If we don't do this, we'd
have to add module.h to obviously non-modular code, and that
would be a worse thing.
Note that direct use of __initcall is discouraged, vs. one
of the priority categorized subgroups. As __initcall gets
mapped onto device_initcall, our use of device_initcall
directly in this change means that the runtime impact is
zero -- they will remain at level 6 in initcall ordering.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
arch/powerpc/kernel/time.c | 2 +-
arch/powerpc/platforms/ps3/time.c | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index afb1b56ef4fa..bee2bb2bbc75 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -1053,4 +1053,4 @@ static int __init rtc_init(void)
return PTR_ERR_OR_ZERO(pdev);
}
-module_init(rtc_init);
+device_initcall(rtc_init);
diff --git a/arch/powerpc/platforms/ps3/time.c b/arch/powerpc/platforms/ps3/time.c
index ce73ce865613..791c6142c4a7 100644
--- a/arch/powerpc/platforms/ps3/time.c
+++ b/arch/powerpc/platforms/ps3/time.c
@@ -92,5 +92,4 @@ static int __init ps3_rtc_init(void)
return PTR_ERR_OR_ZERO(pdev);
}
-
-module_init(ps3_rtc_init);
+device_initcall(ps3_rtc_init);
--
1.8.5.2
^ permalink raw reply related
* [PATCH 4/4] powerpc: don't use module_init for non-modular core hugetlb code
From: Paul Gortmaker @ 2014-01-13 16:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: Paul Gortmaker, linuxppc-dev
In-Reply-To: <1389630113-7919-1-git-send-email-paul.gortmaker@windriver.com>
The hugetlbpage.o is obj-y (always built in). It will never
be modular, so using module_init as an alias for __initcall is
somewhat misleading.
Fix this up now, so that we can relocate module_init from
init.h into module.h in the future. If we don't do this, we'd
have to add module.h to obviously non-modular code, and that
would be a worse thing.
Note that direct use of __initcall is discouraged, vs. one
of the priority categorized subgroups. As __initcall gets
mapped onto device_initcall, our use of arch_initcall (which
makes sense for arch code) will thus change this registration
from level 6-device to level 3-arch (i.e. slightly earlier).
However no observable impact of that small difference has
been observed during testing, or is expected.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
arch/powerpc/mm/hugetlbpage.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 90bb6d9409bf..d25c202420da 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -911,7 +911,7 @@ static int __init hugetlbpage_init(void)
return 0;
}
#endif
-module_init(hugetlbpage_init);
+arch_initcall(hugetlbpage_init);
void flush_dcache_icache_hugepage(struct page *page)
{
--
1.8.5.2
^ permalink raw reply related
* [PATCH 3/4] powerpc: use subsys_initcall for Freescale Local Bus
From: Paul Gortmaker @ 2014-01-13 16:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: Paul Gortmaker, linuxppc-dev
In-Reply-To: <1389630113-7919-1-git-send-email-paul.gortmaker@windriver.com>
The FSL_SOC option is bool, and hence this code is either
present or absent. It will never be modular, so using
module_init as an alias for __initcall is rather misleading.
Fix this up now, so that we can relocate module_init from
init.h into module.h in the future. If we don't do this, we'd
have to add module.h to obviously non-modular code, and that
would be a worse thing.
Note that direct use of __initcall is discouraged, vs. one
of the priority categorized subgroups. As __initcall gets
mapped onto device_initcall, our use of subsys_initcall (which
makes sense for bus code) will thus change this registration
from level 6-device to level 4-subsys (i.e. slightly earlier).
However no observable impact of that small difference has
been observed during testing, or is expected.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
arch/powerpc/sysdev/fsl_lbc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c
index 6bc5a546d49f..9f00e5f84abe 100644
--- a/arch/powerpc/sysdev/fsl_lbc.c
+++ b/arch/powerpc/sysdev/fsl_lbc.c
@@ -388,4 +388,4 @@ static int __init fsl_lbc_init(void)
{
return platform_driver_register(&fsl_lbc_ctrl_driver);
}
-module_init(fsl_lbc_init);
+subsys_initcall(fsl_lbc_init);
--
1.8.5.2
^ permalink raw reply related
* [PATCH 2/4] powerpc: book3s kvm can be modular so it should use module.h
From: Paul Gortmaker @ 2014-01-13 16:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: Paul Gortmaker, linuxppc-dev
In-Reply-To: <1389630113-7919-1-git-send-email-paul.gortmaker@windriver.com>
KVM support is tristate, so this file should be including
module.h instead of export.h -- it only works currently because
module_init is currently (mis)placed in init.h -- but we are
intending to clean that up and relocate it to module.h
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
arch/powerpc/kvm/book3s.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 8912608b7e1b..279459e8a072 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -16,7 +16,7 @@
#include <linux/kvm_host.h>
#include <linux/err.h>
-#include <linux/export.h>
+#include <linux/module.h>
#include <linux/slab.h>
#include <asm/reg.h>
--
1.8.5.2
^ permalink raw reply related
* Re: [PATCH V4] powerpc: thp: Fix crash on mremap
From: Kirill A. Shutemov @ 2014-01-13 16:32 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: aarcange, linux-mm, paulus, linuxppc-dev, kirill.shutemov
In-Reply-To: <1389593064-32664-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On Mon, Jan 13, 2014 at 11:34:24AM +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> This patch fix the below crash
>
> NIP [c00000000004cee4] .__hash_page_thp+0x2a4/0x440
> LR [c0000000000439ac] .hash_page+0x18c/0x5e0
> ...
> Call Trace:
> [c000000736103c40] [00001ffffb000000] 0x1ffffb000000(unreliable)
> [437908.479693] [c000000736103d50] [c0000000000439ac] .hash_page+0x18c/0x5e0
> [437908.479699] [c000000736103e30] [c00000000000924c] .do_hash_page+0x4c/0x58
>
> On ppc64 we use the pgtable for storing the hpte slot information and
> store address to the pgtable at a constant offset (PTRS_PER_PMD) from
> pmd. On mremap, when we switch the pmd, we need to withdraw and deposit
> the pgtable again, so that we find the pgtable at PTRS_PER_PMD offset
> from new pmd.
>
> We also want to move the withdraw and deposit before the set_pmd so
> that, when page fault find the pmd as trans huge we can be sure that
> pgtable can be located at the offset.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
--
Kirill A. Shutemov
^ permalink raw reply
* [PATCH] powerpc/relocate fix relocate processing in LE mode
From: Laurent Dufour @ 2014-01-13 16:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt, paulus, linuxppc-dev
Relocation's code is not working in little endian mode because the r_info
field, which is a 64 bits value, should be read from the right offset.
The current code is optimized to read the r_info field as a 32 bits value
starting at the middle of the double word (offset 12). When running in LE
mode, the read value is not correct since only the MSB is read.
This patch removes this optimization which consist to deal with a 32 bits
value instead of a 64 bits one. This way it works in big and little endian
mode.
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
arch/powerpc/kernel/reloc_64.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/reloc_64.S b/arch/powerpc/kernel/reloc_64.S
index b47a0e1..1482327 100644
--- a/arch/powerpc/kernel/reloc_64.S
+++ b/arch/powerpc/kernel/reloc_64.S
@@ -69,8 +69,8 @@ _GLOBAL(relocate)
* R_PPC64_RELATIVE ones.
*/
mtctr r8
-5: lwz r0,12(9) /* ELF64_R_TYPE(reloc->r_info) */
- cmpwi r0,R_PPC64_RELATIVE
+5: ld r0,8(9) /* ELF64_R_TYPE(reloc->r_info) */
+ cmpdi r0,R_PPC64_RELATIVE
bne 6f
ld r6,0(r9) /* reloc->r_offset */
ld r0,16(r9) /* reloc->r_addend */
^ permalink raw reply related
* Re: [PATCH v2 0/9] cpuidle: rework device state count handling
From: Rafael J. Wysocki @ 2014-01-13 21:20 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: linux-samsung-soc, linux-pm, daniel.lezcano, linux-kernel,
kyungmin.park, linuxppc-dev, lenb
In-Reply-To: <2079155.EyEBRDoJjP@vostro.rjw.lan>
On Saturday, January 11, 2014 01:37:29 AM Rafael J. Wysocki wrote:
> On Friday, December 20, 2013 07:47:22 PM Bartlomiej Zolnierkiewicz wrote:
> > Hi,
> >
> > Some cpuidle drivers assume that cpuidle core will handle cases where
> > device->state_count is smaller than driver->state_count, unfortunately
> > currently this is untrue (device->state_count is used only for handling
> > cpuidle state sysfs entries and driver->state_count is used for all
> > other cases) and will not be fixed in the future as device->state_count
> > is planned to be removed [1].
> >
> > This patchset fixes such drivers (ARM EXYNOS cpuidle driver and ACPI
> > cpuidle driver), removes superflous device->state_count initialization
> > from drivers for which device->state_count equals driver->state_count
> > (POWERPC pseries cpuidle driver and intel_idle driver) and finally
> > removes state_count field from struct cpuidle_device.
> >
> > Additionaly (while at it) this patchset fixes C1E promotion disable
> > quirk handling (in intel_idle driver) and converts cpuidle drivers code
> > to use the common cpuidle_[un]register() routines (in POWERPC pseries
> > cpuidle driver and intel_idle driver).
> >
> > [1] http://permalink.gmane.org/gmane.linux.power-management.general/36908
> >
> > Reference to v1:
> > http://comments.gmane.org/gmane.linux.power-management.general/37390
> >
> > Changes since v1:
> > - synced patch series with next-20131220
> > - added ACKs from Daniel Lezcano
>
> This series breaks boot on one of my test machines with intel_idle, so I'm
> not sure how well it has been tested.
>
> I've dropped it entirely for now. If I have the time, I will try to identify
> the root cause of the failure, but that may not happen before the merge window.
> Sorry about that.
The breakage was introduced by patch [8/9], so I've re-applied patches [1-7/9]
from this series. Please refer to Fengguang's report [1] for the breakage
details.
Thanks!
[1] http://marc.info/?l=linux-kernel&m=138964167909907&w=2
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH] powerpc: thp: Fix crash on mremap
From: Andrew Morton @ 2014-01-13 22:17 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: aarcange, linux-mm, paulus, Aneesh Kumar K.V, linuxppc-dev,
kirill.shutemov
In-Reply-To: <20140102021951.GA26369@node.dhcp.inet.fi>
On Thu, 2 Jan 2014 04:19:51 +0200 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> On Wed, Jan 01, 2014 at 09:29:05PM +1100, Benjamin Herrenschmidt wrote:
> > On Wed, 2014-01-01 at 15:23 +0530, Aneesh Kumar K.V wrote:
> > > From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> > >
> > > This patch fix the below crash
> > >
> > > NIP [c00000000004cee4] .__hash_page_thp+0x2a4/0x440
> > > LR [c0000000000439ac] .hash_page+0x18c/0x5e0
> > > ...
> > > Call Trace:
> > > [c000000736103c40] [00001ffffb000000] 0x1ffffb000000(unreliable)
> > > [437908.479693] [c000000736103d50] [c0000000000439ac] .hash_page+0x18c/0x5e0
> > > [437908.479699] [c000000736103e30] [c00000000000924c] .do_hash_page+0x4c/0x58
> > >
> > > On ppc64 we use the pgtable for storing the hpte slot information and
> > > store address to the pgtable at a constant offset (PTRS_PER_PMD) from
> > > pmd. On mremap, when we switch the pmd, we need to withdraw and deposit
> > > the pgtable again, so that we find the pgtable at PTRS_PER_PMD offset
> > > from new pmd.
> > >
> > > We also want to move the withdraw and deposit before the set_pmd so
> > > that, when page fault find the pmd as trans huge we can be sure that
> > > pgtable can be located at the offset.
> > >
Did this get fixed?
^ permalink raw reply
* Re: [PATCH] powerpc: thp: Fix crash on mremap
From: Kirill A. Shutemov @ 2014-01-13 22:30 UTC (permalink / raw)
To: Andrew Morton
Cc: aarcange, linux-mm, paulus, Aneesh Kumar K.V, linuxppc-dev,
kirill.shutemov
In-Reply-To: <20140113141748.0b851e1573e41bf26de7c0ae@linux-foundation.org>
On Mon, Jan 13, 2014 at 02:17:48PM -0800, Andrew Morton wrote:
> On Thu, 2 Jan 2014 04:19:51 +0200 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
>
> > On Wed, Jan 01, 2014 at 09:29:05PM +1100, Benjamin Herrenschmidt wrote:
> > > On Wed, 2014-01-01 at 15:23 +0530, Aneesh Kumar K.V wrote:
> > > > From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> > > >
> > > > This patch fix the below crash
> > > >
> > > > NIP [c00000000004cee4] .__hash_page_thp+0x2a4/0x440
> > > > LR [c0000000000439ac] .hash_page+0x18c/0x5e0
> > > > ...
> > > > Call Trace:
> > > > [c000000736103c40] [00001ffffb000000] 0x1ffffb000000(unreliable)
> > > > [437908.479693] [c000000736103d50] [c0000000000439ac] .hash_page+0x18c/0x5e0
> > > > [437908.479699] [c000000736103e30] [c00000000000924c] .do_hash_page+0x4c/0x58
> > > >
> > > > On ppc64 we use the pgtable for storing the hpte slot information and
> > > > store address to the pgtable at a constant offset (PTRS_PER_PMD) from
> > > > pmd. On mremap, when we switch the pmd, we need to withdraw and deposit
> > > > the pgtable again, so that we find the pgtable at PTRS_PER_PMD offset
> > > > from new pmd.
> > > >
> > > > We also want to move the withdraw and deposit before the set_pmd so
> > > > that, when page fault find the pmd as trans huge we can be sure that
> > > > pgtable can be located at the offset.
> > > >
>
> Did this get fixed?
New version: http://thread.gmane.org/gmane.linux.kernel.mm/111809
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH] powerpc: thp: Fix crash on mremap
From: Benjamin Herrenschmidt @ 2014-01-14 4:13 UTC (permalink / raw)
To: Andrew Morton
Cc: aarcange, linux-mm, paulus, Aneesh Kumar K.V, Kirill A. Shutemov,
linuxppc-dev, kirill.shutemov
In-Reply-To: <20140113141748.0b851e1573e41bf26de7c0ae@linux-foundation.org>
On Mon, 2014-01-13 at 14:17 -0800, Andrew Morton wrote:
> Did this get fixed?
Any chance you can Ack the patch on that thread ?
http://thread.gmane.org/gmane.linux.kernel.mm/111809
So I can put it in powerpc -next with a CC stable ? Or if you tell me
tat Kirill Ack is sufficient then I'll go for it.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH mmotm/next] powerpc: fix powernv boot breakage on G5???
From: Benjamin Herrenschmidt @ 2014-01-14 4:17 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Mahesh Salgaonkar, linuxppc-dev
In-Reply-To: <alpine.LSU.2.11.1401120043210.1092@eggly.anvils>
On Sun, 2014-01-12 at 00:46 -0800, Hugh Dickins wrote:
> My PowerMac G5 cannot boot mmotm these days: different symptoms
> (starting /sbin/init failed? or ATA errors and hang?), with unrelated
> bugs adding to the confusion; but a bisection led to b5ff4211a829
> "powerpc/book3s: Queue up and process delayed MCE events". Since that
> series seems to be mostly about powernv, I tried changing BOOK3S_64
> to POWERNV in entry_64.S, which has got it back to working for me.
>
> Signed-off-by: Hugh Dickins <hughd@google.com>
> just in case this happens to be right, but it's well beyond me!
> ---
Do that help instead ?
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 770d6d6..9820d36 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -187,6 +187,7 @@ syscall_exit:
#ifdef CONFIG_PPC_BOOK3S_64
BEGIN_FTR_SECTION
bl .machine_check_process_queued_event
+ ld r3,RESULT(r1)
END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
#endif
CURRENT_THREAD_INFO(r12, r1)
Cheers,
Ben.
>
> arch/powerpc/kernel/entry_64.S | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- mmotm/arch/powerpc/kernel/entry_64.S 2014-01-10 18:24:56.940448828 -0800
> +++ linux/arch/powerpc/kernel/entry_64.S 2014-01-10 18:29:24.276455182 -0800
> @@ -184,7 +184,7 @@ syscall_exit:
> bl .do_show_syscall_exit
> ld r3,RESULT(r1)
> #endif
> -#ifdef CONFIG_PPC_BOOK3S_64
> +#ifdef CONFIG_PPC_POWERNV
> BEGIN_FTR_SECTION
> bl .machine_check_process_queued_event
> END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
^ permalink raw reply related
* [PATCH] Move precessing of MCE queued event out from syscall exit path.
From: Mahesh J Salgaonkar @ 2014-01-14 4:26 UTC (permalink / raw)
To: linuxppc-dev, Benjamin Herrenschmidt; +Cc: Hugh Dickins
From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Huge Dickins reported an issue that b5ff4211a829
"powerpc/book3s: Queue up and process delayed MCE events" breaks the
PowerMac G5 boot. This patch fixes it by moving the mce even processing
away from syscall exit, which was wrong to do that in first place, and
implements a different mechanism to deal with it using a paca flag and
decrementer interrupt to process the event.
Reported-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/mce.h | 3 +++
arch/powerpc/include/asm/paca.h | 3 +++
arch/powerpc/kernel/entry_64.S | 5 -----
arch/powerpc/kernel/irq.c | 11 ++++++++++-
arch/powerpc/kernel/mce.c | 7 +++++++
arch/powerpc/kernel/time.c | 9 +++++++++
6 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 2257d1e..225e678 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -186,6 +186,9 @@ struct mce_error_info {
#define MCE_EVENT_RELEASE true
#define MCE_EVENT_DONTRELEASE false
+/* MCE bit flags (paca.mce_flags) */
+#define MCE_EVENT_PENDING 0x0001
+
extern void save_mce_event(struct pt_regs *regs, long handled,
struct mce_error_info *mce_err, uint64_t nip,
uint64_t addr);
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index c3523d1..f9aa521 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -141,6 +141,9 @@ struct paca_struct {
u8 io_sync; /* writel() needs spin_unlock sync */
u8 irq_work_pending; /* IRQ_WORK interrupt while soft-disable */
u8 nap_state_lost; /* NV GPR values lost in power7_idle */
+#ifdef CONFIG_PPC_BOOK3S_64
+ u8 mce_flags; /* MCE bit flags. */
+#endif
u64 sprg3; /* Saved user-visible sprg */
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
u64 tm_scratch; /* TM scratch area for reclaim */
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 770d6d6..bbfb029 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -184,11 +184,6 @@ syscall_exit:
bl .do_show_syscall_exit
ld r3,RESULT(r1)
#endif
-#ifdef CONFIG_PPC_BOOK3S_64
-BEGIN_FTR_SECTION
- bl .machine_check_process_queued_event
-END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
-#endif
CURRENT_THREAD_INFO(r12, r1)
ld r8,_MSR(r1)
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index ba01656..e22f591 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -67,6 +67,7 @@
#include <asm/udbg.h>
#include <asm/smp.h>
#include <asm/debug.h>
+#include <asm/mce.h>
#ifdef CONFIG_PPC64
#include <asm/paca.h>
@@ -158,9 +159,17 @@ notrace unsigned int __check_irq_replay(void)
* We may have missed a decrementer interrupt. We check the
* decrementer itself rather than the paca irq_happened field
* in case we also had a rollover while hard disabled
+ * Also check if any MCE event is queued up that requires
+ * processing. Machine check handler would set paca->mce_flags
+ * and then call set_dec(1) to trigger a decrementer interrupt
+ * from NMI.
*/
local_paca->irq_happened &= ~PACA_IRQ_DEC;
- if ((happened & PACA_IRQ_DEC) || decrementer_check_overflow())
+ if ((happened & PACA_IRQ_DEC) || decrementer_check_overflow()
+#ifdef CONFIG_PPC_BOOK3S_64
+ || local_paca->mce_flags & MCE_EVENT_PENDING
+#endif
+ )
return 0x900;
/* Finally check if an external interrupt happened */
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index d6edf2b..7bab827 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -185,6 +185,13 @@ void machine_check_queue_event(void)
return;
}
__get_cpu_var(mce_event_queue[index]) = evt;
+
+ /*
+ * Set the event pending flag and raise an decrementer interrupt
+ * to process the queued event later.
+ */
+ local_paca->mce_flags |= MCE_EVENT_PENDING;
+ set_dec(1);
}
/*
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index b3b1441..87ccf92 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -69,6 +69,7 @@
#include <asm/vdso_datapage.h>
#include <asm/firmware.h>
#include <asm/cputime.h>
+#include <asm/mce.h>
/* powerpc clocksource/clockevent code */
@@ -505,6 +506,14 @@ void timer_interrupt(struct pt_regs * regs)
return;
}
+#ifdef CONFIG_PPC_BOOK3S_64
+ /* Check if we have MCE event pending for processing. */
+ if (local_paca->mce_flags & MCE_EVENT_PENDING) {
+ local_paca->mce_flags &= ~MCE_EVENT_PENDING;
+ machine_check_process_queued_event();
+ }
+#endif
+
/* Conditionally hard-enable interrupts now that the DEC has been
* bumped to its maximum value
*/
^ permalink raw reply related
* Re: [PATCH] powerpc: thp: Fix crash on mremap
From: Andrew Morton @ 2014-01-14 4:32 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aarcange, linux-mm, paulus, Aneesh Kumar K.V, Kirill A. Shutemov,
linuxppc-dev, kirill.shutemov
In-Reply-To: <1389672810.6933.0.camel@pasglop>
On Tue, 14 Jan 2014 15:13:30 +1100 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Mon, 2014-01-13 at 14:17 -0800, Andrew Morton wrote:
>
> > Did this get fixed?
>
> Any chance you can Ack the patch on that thread ?
>
> http://thread.gmane.org/gmane.linux.kernel.mm/111809
>
> So I can put it in powerpc -next with a CC stable ? Or if you tell me
> tat Kirill Ack is sufficient then I'll go for it.
yup, it looks OK to me from a non-ppc perspective. Please proceed as
described.
^ permalink raw reply
* [PATCH] cpuidle/menu: Fail cpuidle_idle_call() if no idle state is acceptable
From: Preeti U Murthy @ 2014-01-14 6:05 UTC (permalink / raw)
To: deepthi, paulmck, linux-pm, benh, daniel.lezcano, rjw,
linux-kernel, srivatsa.bhat, svaidy, linuxppc-dev,
tuukka.tikkanen
On PowerPC, in a particular test scenario, all the cpu idle states were disabled.
Inspite of this it was observed that the idle state count of the shallowest
idle state, snooze, was increasing.
This is because the governor returns the idle state index as 0 even in
scenarios when no idle state can be chosen. These scenarios could be when the
latency requirement is 0 or as mentioned above when the user wants to disable
certain cpu idle states at runtime. In the latter case, its possible that no
cpu idle state is valid because the suitable states were disabled
and the rest did not match the menu governor criteria to be chosen as the
next idle state.
This patch adds the code to indicate that a valid cpu idle state could not be
chosen by the menu governor and reports back to arch so that it can take some
default action.
Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
---
drivers/cpuidle/cpuidle.c | 6 +++++-
| 7 ++++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index a55e68f..5bf06bb 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -131,8 +131,9 @@ int cpuidle_idle_call(void)
/* ask the governor for the next state */
next_state = cpuidle_curr_governor->select(drv, dev);
+
+ dev->last_residency = 0;
if (need_resched()) {
- dev->last_residency = 0;
/* give the governor an opportunity to reflect on the outcome */
if (cpuidle_curr_governor->reflect)
cpuidle_curr_governor->reflect(dev, next_state);
@@ -140,6 +141,9 @@ int cpuidle_idle_call(void)
return 0;
}
+ if (next_state < 0)
+ return -EINVAL;
+
trace_cpu_idle_rcuidle(next_state, dev->cpu);
broadcast = !!(drv->states[next_state].flags & CPUIDLE_FLAG_TIMER_STOP);
--git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index cf7f2f0..6921543 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -283,6 +283,7 @@ again:
* menu_select - selects the next idle state to enter
* @drv: cpuidle driver containing state data
* @dev: the CPU
+ * Returns -1 when no idle state is suitable
*/
static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
{
@@ -292,17 +293,17 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
int multiplier;
struct timespec t;
- if (data->needs_update) {
+ if (data->last_state_idx >= 0 && data->needs_update) {
menu_update(drv, dev);
data->needs_update = 0;
}
- data->last_state_idx = 0;
+ data->last_state_idx = -1;
data->exit_us = 0;
/* Special case when user has set very strict latency requirement */
if (unlikely(latency_req == 0))
- return 0;
+ return data->last_state_idx;
/* determine the expected residency time, round up */
t = ktime_to_timespec(tick_nohz_get_sleep_length());
^ permalink raw reply related
* [PATCH] powerpc: Fix races with irq_work
From: Benjamin Herrenschmidt @ 2014-01-14 6:11 UTC (permalink / raw)
To: linuxppc-dev list
If we set irq_work on a processor and immediately afterward, before the
irq work has a chance to be processed, we change the decrementer value,
we can seriously delay the handling of that irq_work.
Fix it by checking in a few places for pending irq work, first before
changing the decrementer in decrementer_set_next_event() and after
changing it in the same function and in timer_interrupt().
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index afb1b56..b3dab20 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -536,6 +536,9 @@ void timer_interrupt(struct pt_regs * regs)
now = *next_tb - now;
if (now <= DECREMENTER_MAX)
set_dec((int)now);
+ /* We may have raced with new irq work */
+ if (test_irq_work_pending())
+ set_dec(1);
__get_cpu_var(irq_stat).timer_irqs_others++;
}
@@ -802,8 +805,16 @@ static void __init clocksource_init(void)
static int decrementer_set_next_event(unsigned long evt,
struct clock_event_device *dev)
{
+ /* Don't adjust the decrementer if some irq work is pending */
+ if (test_irq_work_pending())
+ return 0;
__get_cpu_var(decrementers_next_tb) = get_tb_or_rtc() + evt;
set_dec(evt);
+
+ /* We may have raced with new irq work */
+ if (test_irq_work_pending())
+ set_dec(1);
+
return 0;
}
^ permalink raw reply related
* Re: [PATCH] cpuidle/menu: Fail cpuidle_idle_call() if no idle state is acceptable
From: Deepthi Dharwar @ 2014-01-14 6:16 UTC (permalink / raw)
To: Preeti U Murthy
Cc: linux-pm, daniel.lezcano, rjw, linux-kernel, srivatsa.bhat,
paulmck, linuxppc-dev, tuukka.tikkanen
In-Reply-To: <20140114060516.6109.14901.stgit@preeti.in.ibm.com>
On 01/14/2014 11:35 AM, Preeti U Murthy wrote:
> On PowerPC, in a particular test scenario, all the cpu idle states were disabled.
> Inspite of this it was observed that the idle state count of the shallowest
> idle state, snooze, was increasing.
>
> This is because the governor returns the idle state index as 0 even in
> scenarios when no idle state can be chosen. These scenarios could be when the
> latency requirement is 0 or as mentioned above when the user wants to disable
> certain cpu idle states at runtime. In the latter case, its possible that no
> cpu idle state is valid because the suitable states were disabled
> and the rest did not match the menu governor criteria to be chosen as the
> next idle state.
>
> This patch adds the code to indicate that a valid cpu idle state could not be
> chosen by the menu governor and reports back to arch so that it can take some
> default action.
>
> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
> ---
Acked-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
>
> drivers/cpuidle/cpuidle.c | 6 +++++-
> drivers/cpuidle/governors/menu.c | 7 ++++---
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index a55e68f..5bf06bb 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -131,8 +131,9 @@ int cpuidle_idle_call(void)
>
> /* ask the governor for the next state */
> next_state = cpuidle_curr_governor->select(drv, dev);
> +
> + dev->last_residency = 0;
> if (need_resched()) {
> - dev->last_residency = 0;
> /* give the governor an opportunity to reflect on the outcome */
> if (cpuidle_curr_governor->reflect)
> cpuidle_curr_governor->reflect(dev, next_state);
> @@ -140,6 +141,9 @@ int cpuidle_idle_call(void)
> return 0;
> }
>
> + if (next_state < 0)
> + return -EINVAL;
> +
> trace_cpu_idle_rcuidle(next_state, dev->cpu);
>
> broadcast = !!(drv->states[next_state].flags & CPUIDLE_FLAG_TIMER_STOP);
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index cf7f2f0..6921543 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -283,6 +283,7 @@ again:
> * menu_select - selects the next idle state to enter
> * @drv: cpuidle driver containing state data
> * @dev: the CPU
> + * Returns -1 when no idle state is suitable
> */
> static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> {
> @@ -292,17 +293,17 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> int multiplier;
> struct timespec t;
>
> - if (data->needs_update) {
> + if (data->last_state_idx >= 0 && data->needs_update) {
> menu_update(drv, dev);
> data->needs_update = 0;
> }
>
> - data->last_state_idx = 0;
> + data->last_state_idx = -1;
> data->exit_us = 0;
>
> /* Special case when user has set very strict latency requirement */
> if (unlikely(latency_req == 0))
> - return 0;
> + return data->last_state_idx;
>
> /* determine the expected residency time, round up */
> t = ktime_to_timespec(tick_nohz_get_sleep_length());
>
^ permalink raw reply
* Re: [PATCH] cpuidle/menu: Fail cpuidle_idle_call() if no idle state is acceptable
From: Srivatsa S. Bhat @ 2014-01-14 7:00 UTC (permalink / raw)
To: Preeti U Murthy
Cc: deepthi, linux-pm, daniel.lezcano, rjw, linux-kernel, paulmck,
linuxppc-dev, tuukka.tikkanen
In-Reply-To: <20140114060516.6109.14901.stgit@preeti.in.ibm.com>
On 01/14/2014 11:35 AM, Preeti U Murthy wrote:
> On PowerPC, in a particular test scenario, all the cpu idle states were disabled.
> Inspite of this it was observed that the idle state count of the shallowest
> idle state, snooze, was increasing.
>
> This is because the governor returns the idle state index as 0 even in
> scenarios when no idle state can be chosen. These scenarios could be when the
> latency requirement is 0 or as mentioned above when the user wants to disable
> certain cpu idle states at runtime. In the latter case, its possible that no
> cpu idle state is valid because the suitable states were disabled
> and the rest did not match the menu governor criteria to be chosen as the
> next idle state.
>
> This patch adds the code to indicate that a valid cpu idle state could not be
> chosen by the menu governor and reports back to arch so that it can take some
> default action.
>
That sounds fair enough. However, the "default" action of pseries idle loop
(pseries_lpar_idle()) surprises me. It enters Cede, which is _deeper_ than doing
a snooze! IOW, a user might "disable" cpuidle or set the PM_QOS_CPU_DMA_LATENCY
to 0 hoping to prevent the CPUs from going to deep idle states, but then the
machine would still end up going to Cede, even though that wont get reflected
in the idle state counts. IMHO that scenario needs some thought as well...
> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
> ---
>
> drivers/cpuidle/cpuidle.c | 6 +++++-
> drivers/cpuidle/governors/menu.c | 7 ++++---
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index a55e68f..5bf06bb 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -131,8 +131,9 @@ int cpuidle_idle_call(void)
>
> /* ask the governor for the next state */
> next_state = cpuidle_curr_governor->select(drv, dev);
> +
> + dev->last_residency = 0;
> if (need_resched()) {
> - dev->last_residency = 0;
> /* give the governor an opportunity to reflect on the outcome */
> if (cpuidle_curr_governor->reflect)
> cpuidle_curr_governor->reflect(dev, next_state);
The comments on top of the .reflect() routines of the governors say that the
second parameter is the index of the actual state entered. But after this patch,
next_state can be negative, indicating an invalid index. So those comments need
to be updated accordingly.
> @@ -140,6 +141,9 @@ int cpuidle_idle_call(void)
> return 0;
> }
>
> + if (next_state < 0)
> + return -EINVAL;
The exit path above (due to need_resched) returns with irqs enabled, but the new
one you are adding (next_state < 0) returns with irqs disabled. This is correct,
because in the latter case, "idle" is still in progress and the arch will choose
a default handler to execute (unlike the former case where "idle" is over and
hence its time to enable interrupts).
IMHO it would be good to add comments around this code to explain this subtle
difference. We can never be too careful with these things... ;-)
> +
> trace_cpu_idle_rcuidle(next_state, dev->cpu);
>
> broadcast = !!(drv->states[next_state].flags & CPUIDLE_FLAG_TIMER_STOP);
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index cf7f2f0..6921543 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -283,6 +283,7 @@ again:
> * menu_select - selects the next idle state to enter
> * @drv: cpuidle driver containing state data
> * @dev: the CPU
> + * Returns -1 when no idle state is suitable
> */
> static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> {
> @@ -292,17 +293,17 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> int multiplier;
> struct timespec t;
>
> - if (data->needs_update) {
> + if (data->last_state_idx >= 0 && data->needs_update) {
^^^^^
Doesn't hurt, but actually unnecessary, since ->needs_update is set to 1
only when index >= 0.
> menu_update(drv, dev);
> data->needs_update = 0;
> }
>
> - data->last_state_idx = 0;
> + data->last_state_idx = -1;
> data->exit_us = 0;
>
> /* Special case when user has set very strict latency requirement */
> if (unlikely(latency_req == 0))
> - return 0;
> + return data->last_state_idx;
>
> /* determine the expected residency time, round up */
> t = ktime_to_timespec(tick_nohz_get_sleep_length());
>
What about the ladder governor? I know its not used that much in practice,
but I think it would be good to update that as well, just to keep it
consistent.
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [PATCH] cpuidle/menu: Fail cpuidle_idle_call() if no idle state is acceptable
From: Srivatsa S. Bhat @ 2014-01-14 7:37 UTC (permalink / raw)
To: Preeti U Murthy
Cc: deepthi, linux-pm, daniel.lezcano, rjw, linux-kernel, paulmck,
linuxppc-dev, tuukka.tikkanen
In-Reply-To: <52D4E07E.204@linux.vnet.ibm.com>
On 01/14/2014 12:30 PM, Srivatsa S. Bhat wrote:
> On 01/14/2014 11:35 AM, Preeti U Murthy wrote:
>> On PowerPC, in a particular test scenario, all the cpu idle states were disabled.
>> Inspite of this it was observed that the idle state count of the shallowest
>> idle state, snooze, was increasing.
>>
>> This is because the governor returns the idle state index as 0 even in
>> scenarios when no idle state can be chosen. These scenarios could be when the
>> latency requirement is 0 or as mentioned above when the user wants to disable
>> certain cpu idle states at runtime. In the latter case, its possible that no
>> cpu idle state is valid because the suitable states were disabled
>> and the rest did not match the menu governor criteria to be chosen as the
>> next idle state.
>>
>> This patch adds the code to indicate that a valid cpu idle state could not be
>> chosen by the menu governor and reports back to arch so that it can take some
>> default action.
>>
>
> That sounds fair enough. However, the "default" action of pseries idle loop
> (pseries_lpar_idle()) surprises me. It enters Cede, which is _deeper_ than doing
> a snooze! IOW, a user might "disable" cpuidle or set the PM_QOS_CPU_DMA_LATENCY
> to 0 hoping to prevent the CPUs from going to deep idle states, but then the
> machine would still end up going to Cede, even though that wont get reflected
> in the idle state counts. IMHO that scenario needs some thought as well...
>
I checked the git history and found that the default idle was changed (on purpose)
to cede the processor, in order to speed up booting.. Hmm..
commit 363edbe2614aa90df706c0f19ccfa2a6c06af0be
Author: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
Date: Fri Sep 6 00:25:06 2013 +0530
powerpc: Default arch idle could cede processor on pseries
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [PATCH] Move precessing of MCE queued event out from syscall exit path.
From: Hugh Dickins @ 2014-01-14 7:47 UTC (permalink / raw)
To: Mahesh J Salgaonkar, Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <20140114042611.13145.6551.stgit@mars.in.ibm.com>
On Tue, 14 Jan 2014, Mahesh J Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>
> Huge Dickins reported an issue that b5ff4211a829
> "powerpc/book3s: Queue up and process delayed MCE events" breaks the
> PowerMac G5 boot. This patch fixes it by moving the mce even processing
> away from syscall exit, which was wrong to do that in first place, and
> implements a different mechanism to deal with it using a paca flag and
> decrementer interrupt to process the event.
>
> Reported-by: Hugh Dickins <hughd@google.com>
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Good work guys: I can happily report that both this rework,
and Ben's one-liner, fix the issue for me on the G5: thank you.
(Irrelevant not-so-happy detail: I very nearly mailed you an hour or
so earlier to report that neither fixed it; but retried my original
CONFIG_PPC_POWERNV hack after, and found that now equally useless.
I did write of changing behaviour and ATA errors: it now appears that's
an independent but intermittent issue on the G5 in 3.13-rc7-mm1, which
coincidentally happened to trigger when I tested rc7-mm1 without fixes,
but not when I tested with my hack, until today.
I've gone back to testing on rc6-mm1, the previous week's mmotm,
which showed failure to run /sbin/init: rc6-mm1 has no trouble mounting
root, and it runs properly with your new patch, and with Ben's patch.
And I may be quite wrong to point a finger at ATA errors: perhaps
they're always shown, and quickly cleared off screen in successful boots,
but left visible when root cannot be mounted for some other reason.
I don't know, and won't have time to investigate further - bisecting
intermittents is not much fun! I'll just have to hope that it's
sorted out before it reaches 3.14-rc, or else bite the bullet and
investigate on that.)
Hugh
> ---
> arch/powerpc/include/asm/mce.h | 3 +++
> arch/powerpc/include/asm/paca.h | 3 +++
> arch/powerpc/kernel/entry_64.S | 5 -----
> arch/powerpc/kernel/irq.c | 11 ++++++++++-
> arch/powerpc/kernel/mce.c | 7 +++++++
> arch/powerpc/kernel/time.c | 9 +++++++++
> 6 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
> index 2257d1e..225e678 100644
> --- a/arch/powerpc/include/asm/mce.h
> +++ b/arch/powerpc/include/asm/mce.h
> @@ -186,6 +186,9 @@ struct mce_error_info {
> #define MCE_EVENT_RELEASE true
> #define MCE_EVENT_DONTRELEASE false
>
> +/* MCE bit flags (paca.mce_flags) */
> +#define MCE_EVENT_PENDING 0x0001
> +
> extern void save_mce_event(struct pt_regs *regs, long handled,
> struct mce_error_info *mce_err, uint64_t nip,
> uint64_t addr);
> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> index c3523d1..f9aa521 100644
> --- a/arch/powerpc/include/asm/paca.h
> +++ b/arch/powerpc/include/asm/paca.h
> @@ -141,6 +141,9 @@ struct paca_struct {
> u8 io_sync; /* writel() needs spin_unlock sync */
> u8 irq_work_pending; /* IRQ_WORK interrupt while soft-disable */
> u8 nap_state_lost; /* NV GPR values lost in power7_idle */
> +#ifdef CONFIG_PPC_BOOK3S_64
> + u8 mce_flags; /* MCE bit flags. */
> +#endif
> u64 sprg3; /* Saved user-visible sprg */
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> u64 tm_scratch; /* TM scratch area for reclaim */
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 770d6d6..bbfb029 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -184,11 +184,6 @@ syscall_exit:
> bl .do_show_syscall_exit
> ld r3,RESULT(r1)
> #endif
> -#ifdef CONFIG_PPC_BOOK3S_64
> -BEGIN_FTR_SECTION
> - bl .machine_check_process_queued_event
> -END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> -#endif
> CURRENT_THREAD_INFO(r12, r1)
>
> ld r8,_MSR(r1)
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index ba01656..e22f591 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -67,6 +67,7 @@
> #include <asm/udbg.h>
> #include <asm/smp.h>
> #include <asm/debug.h>
> +#include <asm/mce.h>
>
> #ifdef CONFIG_PPC64
> #include <asm/paca.h>
> @@ -158,9 +159,17 @@ notrace unsigned int __check_irq_replay(void)
> * We may have missed a decrementer interrupt. We check the
> * decrementer itself rather than the paca irq_happened field
> * in case we also had a rollover while hard disabled
> + * Also check if any MCE event is queued up that requires
> + * processing. Machine check handler would set paca->mce_flags
> + * and then call set_dec(1) to trigger a decrementer interrupt
> + * from NMI.
> */
> local_paca->irq_happened &= ~PACA_IRQ_DEC;
> - if ((happened & PACA_IRQ_DEC) || decrementer_check_overflow())
> + if ((happened & PACA_IRQ_DEC) || decrementer_check_overflow()
> +#ifdef CONFIG_PPC_BOOK3S_64
> + || local_paca->mce_flags & MCE_EVENT_PENDING
> +#endif
> + )
> return 0x900;
>
> /* Finally check if an external interrupt happened */
> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
> index d6edf2b..7bab827 100644
> --- a/arch/powerpc/kernel/mce.c
> +++ b/arch/powerpc/kernel/mce.c
> @@ -185,6 +185,13 @@ void machine_check_queue_event(void)
> return;
> }
> __get_cpu_var(mce_event_queue[index]) = evt;
> +
> + /*
> + * Set the event pending flag and raise an decrementer interrupt
> + * to process the queued event later.
> + */
> + local_paca->mce_flags |= MCE_EVENT_PENDING;
> + set_dec(1);
> }
>
> /*
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index b3b1441..87ccf92 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -69,6 +69,7 @@
> #include <asm/vdso_datapage.h>
> #include <asm/firmware.h>
> #include <asm/cputime.h>
> +#include <asm/mce.h>
>
> /* powerpc clocksource/clockevent code */
>
> @@ -505,6 +506,14 @@ void timer_interrupt(struct pt_regs * regs)
> return;
> }
>
> +#ifdef CONFIG_PPC_BOOK3S_64
> + /* Check if we have MCE event pending for processing. */
> + if (local_paca->mce_flags & MCE_EVENT_PENDING) {
> + local_paca->mce_flags &= ~MCE_EVENT_PENDING;
> + machine_check_process_queued_event();
> + }
> +#endif
> +
> /* Conditionally hard-enable interrupts now that the DEC has been
> * bumped to its maximum value
> */
^ 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