* [PATCH 2/2] Fix coding style errors
From: Brandon Stewart @ 2014-01-28 3:07 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, linux-kernel
I corrected several coding errors.
Signed-off-by: Brandon Stewart <stewartb2@gmail.com>
---
drivers/macintosh/adb.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/macintosh/adb.c b/drivers/macintosh/adb.c
index 53611de..dd3f49a 100644
--- a/drivers/macintosh/adb.c
+++ b/drivers/macintosh/adb.c
@@ -623,7 +623,7 @@ do_adb_query(struct adb_request *req)
{
int ret = -EINVAL;
- switch(req->data[1]) {
+ switch (req->data[1]) {
case ADB_QUERY_GETDEVINFO:
if (req->nbytes < 3)
break;
@@ -792,8 +792,9 @@ static ssize_t adb_write(struct file *file, const char __user *buf,
}
/* Special case for ADB_BUSRESET request, all others are sent to
the controller */
- else if ((req->data[0] == ADB_PACKET) && (count > 1)
- && (req->data[1] == ADB_BUSRESET)) {
+ else if (req->data[0] == ADB_PACKET &&
+ req->data[1] == ADB_BUSRESET &&
+ count > 1) {
ret = do_adb_reset_bus();
up(&adb_probe_mutex);
atomic_dec(&state->n_pending);
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH 1/1] Fixed some coding style problems
From: Joe Perches @ 2014-01-28 1:52 UTC (permalink / raw)
To: Brandon Stewart; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1390873397-4470-1-git-send-email-stewartb2@gmail.com>
On Mon, 2014-01-27 at 19:43 -0600, Brandon Stewart wrote:
[]
> diff --git a/drivers/macintosh/adb.c b/drivers/macintosh/adb.c
[]
> @@ -624,8 +623,7 @@ do_adb_query(struct adb_request *req)
> {
> int ret = -EINVAL;
>
> - switch(req->data[1])
> - {
> + switch(req->data[1]) {
switch (req->data[1]) {
> @@ -794,8 +792,8 @@ static ssize_t adb_write(struct file *file, const char __user *buf,
> }
> /* Special case for ADB_BUSRESET request, all others are sent to
> the controller */
> - else if ((req->data[0] == ADB_PACKET)&&(count > 1)
> - &&(req->data[1] == ADB_BUSRESET)) {
> + else if ((req->data[0] == ADB_PACKET) && (count > 1)
> + && (req->data[1] == ADB_BUSRESET)) {
else if (req->data[0] == ADB_PACKET &&
req->data[1] == ADB_BUSRESET &&
count > 1) {
^ permalink raw reply
* [PATCH 1/1] Fixed some coding style problems
From: Brandon Stewart @ 2014-01-28 1:43 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, linux-kernel
Signed-off-by: Brandon Stewart <stewartb2@gmail.com>
---
drivers/macintosh/adb.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/macintosh/adb.c b/drivers/macintosh/adb.c
index 04a5049..53611de 100644
--- a/drivers/macintosh/adb.c
+++ b/drivers/macintosh/adb.c
@@ -38,7 +38,7 @@
#include <linux/platform_device.h>
#include <linux/mutex.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
#ifdef CONFIG_PPC
#include <asm/prom.h>
#include <asm/machdep.h>
@@ -193,8 +193,7 @@ static int adb_scan_bus(void)
break;
noMovement = 0;
- }
- else {
+ } else {
/*
* No devices left at address i; move the
* one(s) we moved to `highFree' back to i.
@@ -502,7 +501,7 @@ void
adb_input(unsigned char *buf, int nb, int autopoll)
{
int i, id;
- static int dump_adb_input = 0;
+ static int dump_adb_input;
unsigned long flags;
void (*handler)(unsigned char *, int, int);
@@ -624,8 +623,7 @@ do_adb_query(struct adb_request *req)
{
int ret = -EINVAL;
- switch(req->data[1])
- {
+ switch(req->data[1]) {
case ADB_QUERY_GETDEVINFO:
if (req->nbytes < 3)
break;
@@ -697,7 +695,7 @@ static ssize_t adb_read(struct file *file, char __user *buf,
int ret = 0;
struct adbdev_state *state = file->private_data;
struct adb_request *req;
- DECLARE_WAITQUEUE(wait,current);
+ DECLARE_WAITQUEUE(wait, current);
unsigned long flags;
if (count < 2)
@@ -794,8 +792,8 @@ static ssize_t adb_write(struct file *file, const char __user *buf,
}
/* Special case for ADB_BUSRESET request, all others are sent to
the controller */
- else if ((req->data[0] == ADB_PACKET)&&(count > 1)
- &&(req->data[1] == ADB_BUSRESET)) {
+ else if ((req->data[0] == ADB_PACKET) && (count > 1)
+ && (req->data[1] == ADB_BUSRESET)) {
ret = do_adb_reset_bus();
up(&adb_probe_mutex);
atomic_dec(&state->n_pending);
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH 1/2][v6] driver/memory:Move Freescale IFC driver to a common driver
From: Scott Wood @ 2014-01-27 23:04 UTC (permalink / raw)
To: Prabhakar Kushwaha; +Cc: linuxppc-dev
In-Reply-To: <1390651572-24468-1-git-send-email-prabhakar@freescale.com>
On Sat, 2014-01-25 at 17:36 +0530, Prabhakar Kushwaha wrote:
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 29a11db..555d26f 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -50,4 +50,13 @@ config TEGRA30_MC
> analysis, especially for IOMMU/SMMU(System Memory Management
> Unit) module.
>
> +config FSL_IFC
> + bool "Freescale Integrated Flash Controller"
> + default y
Don't use "default y".
-Scott
^ permalink raw reply
* Re: [PATCH 1/2][v6] driver/memory:Move Freescale IFC driver to a common driver
From: Kumar Gala @ 2014-01-27 22:06 UTC (permalink / raw)
To: Prabhakar Kushwaha; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <1390651572-24468-1-git-send-email-prabhakar@freescale.com>
On Jan 25, 2014, at 6:06 AM, Prabhakar Kushwaha =
<prabhakar@freescale.com> wrote:
> Freescale IFC controller has been used for mpc8xxx. It will be used
> for ARM-based SoC as well. This patch moves the driver to =
driver/memory
> and fix the header file includes.
>=20
> Also remove module_platform_driver() and instead call
> platform_driver_register() from subsys_initcall() to make sure this =
module
> has been loaded before MTD partition parsing starts.
>=20
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Based upon =
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git
> Branch next
>=20
> Changes for v2:
> - Move fsl_ifc in driver/memory
>=20
> Changes for v3:
> - move device tree bindings to memory
>=20
> Changes for v4: Rebased to=20
> =
git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git
>=20
> Changes for v5:=20
> - Moved powerpc/Kconfig option to driver/memory
>=20
> Changes for v6:
> - Update Kconfig details
>=20
> .../{powerpc =3D> memory-controllers}/fsl/ifc.txt | 0
> arch/powerpc/Kconfig | 4 ----
> arch/powerpc/sysdev/Makefile | 1 -
> drivers/memory/Kconfig | 9 +++++++++
> drivers/memory/Makefile | 1 +
> {arch/powerpc/sysdev =3D> drivers/memory}/fsl_ifc.c | 8 ++++++--
> drivers/mtd/nand/fsl_ifc_nand.c | 2 +-
> .../include/asm =3D> include/linux}/fsl_ifc.h | 0
> 8 files changed, 17 insertions(+), 8 deletions(-)
> rename Documentation/devicetree/bindings/{powerpc =3D> =
memory-controllers}/fsl/ifc.txt (100%)
> rename {arch/powerpc/sysdev =3D> drivers/memory}/fsl_ifc.c (98%)
> rename {arch/powerpc/include/asm =3D> include/linux}/fsl_ifc.h (100%)
>=20
> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/ifc.txt =
b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
> similarity index 100%
> rename from Documentation/devicetree/bindings/powerpc/fsl/ifc.txt
> rename to =
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index fa39517..91dc43c 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -727,10 +727,6 @@ config FSL_LBC
> controller. Also contains some common code used by
> drivers for specific local bus peripherals.
>=20
> -config FSL_IFC
> - bool
> - depends on FSL_SOC
> -
> config FSL_GTM
> bool
> depends on PPC_83xx || QUICC_ENGINE || CPM2
> diff --git a/arch/powerpc/sysdev/Makefile =
b/arch/powerpc/sysdev/Makefile
> index f67ac90..afbcc37 100644
> --- a/arch/powerpc/sysdev/Makefile
> +++ b/arch/powerpc/sysdev/Makefile
> @@ -21,7 +21,6 @@ obj-$(CONFIG_FSL_SOC) +=3D fsl_soc.o =
fsl_mpic_err.o
> obj-$(CONFIG_FSL_PCI) +=3D fsl_pci.o $(fsl-msi-obj-y)
> obj-$(CONFIG_FSL_PMC) +=3D fsl_pmc.o
> obj-$(CONFIG_FSL_LBC) +=3D fsl_lbc.o
> -obj-$(CONFIG_FSL_IFC) +=3D fsl_ifc.o
> obj-$(CONFIG_FSL_GTM) +=3D fsl_gtm.o
> obj-$(CONFIG_FSL_85XX_CACHE_SRAM) +=3D fsl_85xx_l2ctlr.o =
fsl_85xx_cache_sram.o
> obj-$(CONFIG_SIMPLE_GPIO) +=3D simple_gpio.o
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 29a11db..555d26f 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -50,4 +50,13 @@ config TEGRA30_MC
> analysis, especially for IOMMU/SMMU(System Memory Management
> Unit) module.
>=20
> +config FSL_IFC
> + bool "Freescale Integrated Flash Controller"
> + default y
> + depends on FSL_SOC
minor white space nit (spaces instead of tab)
> + help
> + This driver is for the Integrated Flash Controller =
Controller(IFC)=20
> + module available in Freescale SoCs. This controller allows to =
handle flash
> + devices such as NOR, NAND, FPGA and ASIC etc
> +
> endif
- k
^ permalink raw reply
* Re: [PATCH 1/9] ARM: get rid of arch_cpu_idle_prepare()
From: Nicolas Pitre @ 2014-01-27 17:36 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linaro-kernel, linux-sh, Peter Zijlstra, Catalin Marinas,
linux-pm, Daniel Lezcano, Rafael J. Wysocki, linux-kernel,
Paul Mundt, Thomas Gleixner, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
In-Reply-To: <20140127160654.GO15937@n2100.arm.linux.org.uk>
On Mon, 27 Jan 2014, Russell King - ARM Linux wrote:
> On Mon, Jan 27, 2014 at 10:45:59AM -0500, Nicolas Pitre wrote:
> > On Mon, 27 Jan 2014, Russell King - ARM Linux wrote:
> >
> > > On Mon, Jan 27, 2014 at 01:08:16AM -0500, Nicolas Pitre wrote:
> > > > ARM and ARM64 are the only two architectures implementing
> > > > arch_cpu_idle_prepare() simply to call local_fiq_enable().
> > > >
> > > > We have secondary_start_kernel() already calling local_fiq_enable() and
> > > > this is done a second time in arch_cpu_idle_prepare() in that case. And
> > > > enabling FIQs has nothing to do with idling the CPU to start with.
> > > >
> > > > So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
> > > > CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
> > > > at late_initcall time but this shouldn't make a difference in practice
> > > > i.e. when FIQs are actually used.
> > > >
> > > > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > > > ---
> > > > arch/arm/kernel/process.c | 5 -----
> > > > arch/arm/kernel/setup.c | 7 +++++++
> > > > 2 files changed, 7 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> > > > index 92f7b15dd2..725b8c95e0 100644
> > > > --- a/arch/arm/kernel/process.c
> > > > +++ b/arch/arm/kernel/process.c
> > > > @@ -142,11 +142,6 @@ static void default_idle(void)
> > > > local_irq_enable();
> > > > }
> > > >
> > > > -void arch_cpu_idle_prepare(void)
> > > > -{
> > > > - local_fiq_enable();
> > > > -}
> > > > -
> > > > void arch_cpu_idle_enter(void)
> > > > {
> > > > ledtrig_cpu(CPU_LED_IDLE_START);
> > > > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > > > index 987a7f5bce..d027b1a6fe 100644
> > > > --- a/arch/arm/kernel/setup.c
> > > > +++ b/arch/arm/kernel/setup.c
> > > > @@ -789,6 +789,13 @@ static int __init init_machine_late(void)
> > > > }
> > > > late_initcall(init_machine_late);
> > > >
> > > > +static int __init init_fiq_boot_cpu(void)
> > > > +{
> > > > + local_fiq_enable();
> > > > + return 0;
> > > > +}
> > > > +late_initcall(init_fiq_boot_cpu);
> > >
> > > arch_cpu_idle_prepare() gets called from the swapper thread, and changes
> > > the swapper thread's CPSR. init_fiq_boot_cpu() gets called from PID1, the
> > > init thread, and changes the init thread's CPSR, which will already have
> > > FIQs enabled by way of how kernel threads are created.
> > >
> > > Hence, the above code fragment has no effect what so ever, and those
> > > platforms using FIQs will not have FIQs delivered if they're idle
> > > (because the swapper will have FIQs masked at the CPU.)
> >
> > You're right.
> >
> > What about moving local_fiq_enable() to trap_init() then?
>
> That's potentially unsafe, as we haven't touched any of the IRQ
> controllers at that point - we can't guarantee what state they'd be
> in. Given that the default FIQ is to just return, a FIQ being raised
> at that point will end up with an infinite loop re-entering the FIQ
> handler.
Okay... I don't see any obvious way to work around that besides adding
another explicit hook, which arch_cpu_idle_prepare() incidentally
already is. So, unless you have a better idea, I'll drop this patch and
leave ARM as the only user of arch_cpu_idle_prepare().
Nicolas
^ permalink raw reply
* Re: [PATCH 1/9] ARM: get rid of arch_cpu_idle_prepare()
From: Peter Zijlstra @ 2014-01-27 17:35 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Nicolas Pitre, linaro-kernel, linux-sh, Catalin Marinas, linux-pm,
Daniel Lezcano, Rafael J. Wysocki, linux-kernel, Paul Mundt,
Thomas Gleixner, linuxppc-dev, Ingo Molnar, linux-arm-kernel
In-Reply-To: <20140127172110.GR15937@n2100.arm.linux.org.uk>
On Mon, Jan 27, 2014 at 05:21:10PM +0000, Russell King - ARM Linux wrote:
> A reviewed-by tag on its own doesn't mean much, as it could mean that
> you've just glanced over the code and decided "yea, it looks okay", or
> it could mean that you've spent all day verifying that the code change
> is indeed correct.
One should use Acked-by for the 'yea, it looks okay' thing.
^ permalink raw reply
* Re: [PATCH 1/9] ARM: get rid of arch_cpu_idle_prepare()
From: Daniel Lezcano @ 2014-01-27 17:30 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Nicolas Pitre, linaro-kernel, linux-sh, Peter Zijlstra,
Catalin Marinas, linux-pm, Rafael J. Wysocki, linux-kernel,
Paul Mundt, Thomas Gleixner, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
In-Reply-To: <20140127172110.GR15937@n2100.arm.linux.org.uk>
On 01/27/2014 06:21 PM, Russell King - ARM Linux wrote:
> On Mon, Jan 27, 2014 at 06:12:53PM +0100, Daniel Lezcano wrote:
>> On 01/27/2014 05:07 PM, Russell King - ARM Linux wrote:
>>> On Mon, Jan 27, 2014 at 09:22:55AM +0100, Daniel Lezcano wrote:
>>>> On 01/27/2014 07:08 AM, Nicolas Pitre wrote:
>>>>> ARM and ARM64 are the only two architectures implementing
>>>>> arch_cpu_idle_prepare() simply to call local_fiq_enable().
>>>>>
>>>>> We have secondary_start_kernel() already calling local_fiq_enable() and
>>>>> this is done a second time in arch_cpu_idle_prepare() in that case. And
>>>>> enabling FIQs has nothing to do with idling the CPU to start with.
>>>>>
>>>>> So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
>>>>> CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
>>>>> at late_initcall time but this shouldn't make a difference in practice
>>>>> i.e. when FIQs are actually used.
>>>>>
>>>>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>>>>
>>>> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>
>>> What kind of review did you do when giving that attributation?
>>
>> I did the review to the best of my knowledge and with good will.
>>
>> I read your comment on this patch and I learnt one more thing.
>>
>> Today, I am smarter than yesterday and dumber than tomorrow :)
>
> Just be aware that putting a comment along with the reviewed-by tag
> is always a good idea. I know that's a little more work, but this has
> been raised a number of times by various people over the years.
>
> A reviewed-by tag on its own doesn't mean much, as it could mean that
> you've just glanced over the code and decided "yea, it looks okay", or
> it could mean that you've spent all day verifying that the code change
> is indeed correct.
>
> Consequently, some will ignore emails which just contain a reviewed-by
> attributation.
Thanks for the clarification. I will take care of giving a comment next
time.
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH 1/9] ARM: get rid of arch_cpu_idle_prepare()
From: Russell King - ARM Linux @ 2014-01-27 17:21 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Nicolas Pitre, linaro-kernel, linux-sh, Peter Zijlstra,
Catalin Marinas, linux-pm, Rafael J. Wysocki, linux-kernel,
Paul Mundt, Thomas Gleixner, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
In-Reply-To: <52E69395.9020004@linaro.org>
On Mon, Jan 27, 2014 at 06:12:53PM +0100, Daniel Lezcano wrote:
> On 01/27/2014 05:07 PM, Russell King - ARM Linux wrote:
>> On Mon, Jan 27, 2014 at 09:22:55AM +0100, Daniel Lezcano wrote:
>>> On 01/27/2014 07:08 AM, Nicolas Pitre wrote:
>>>> ARM and ARM64 are the only two architectures implementing
>>>> arch_cpu_idle_prepare() simply to call local_fiq_enable().
>>>>
>>>> We have secondary_start_kernel() already calling local_fiq_enable() and
>>>> this is done a second time in arch_cpu_idle_prepare() in that case. And
>>>> enabling FIQs has nothing to do with idling the CPU to start with.
>>>>
>>>> So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
>>>> CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
>>>> at late_initcall time but this shouldn't make a difference in practice
>>>> i.e. when FIQs are actually used.
>>>>
>>>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>>>
>>> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>
>> What kind of review did you do when giving that attributation?
>
> I did the review to the best of my knowledge and with good will.
>
> I read your comment on this patch and I learnt one more thing.
>
> Today, I am smarter than yesterday and dumber than tomorrow :)
Just be aware that putting a comment along with the reviewed-by tag
is always a good idea. I know that's a little more work, but this has
been raised a number of times by various people over the years.
A reviewed-by tag on its own doesn't mean much, as it could mean that
you've just glanced over the code and decided "yea, it looks okay", or
it could mean that you've spent all day verifying that the code change
is indeed correct.
Consequently, some will ignore emails which just contain a reviewed-by
attributation.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* Re: [PATCH 1/9] ARM: get rid of arch_cpu_idle_prepare()
From: Daniel Lezcano @ 2014-01-27 17:12 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Nicolas Pitre, linaro-kernel, linux-sh, Peter Zijlstra,
Catalin Marinas, linux-pm, Rafael J. Wysocki, linux-kernel,
Paul Mundt, Thomas Gleixner, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
In-Reply-To: <20140127160736.GP15937@n2100.arm.linux.org.uk>
On 01/27/2014 05:07 PM, Russell King - ARM Linux wrote:
> On Mon, Jan 27, 2014 at 09:22:55AM +0100, Daniel Lezcano wrote:
>> On 01/27/2014 07:08 AM, Nicolas Pitre wrote:
>>> ARM and ARM64 are the only two architectures implementing
>>> arch_cpu_idle_prepare() simply to call local_fiq_enable().
>>>
>>> We have secondary_start_kernel() already calling local_fiq_enable() and
>>> this is done a second time in arch_cpu_idle_prepare() in that case. And
>>> enabling FIQs has nothing to do with idling the CPU to start with.
>>>
>>> So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
>>> CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
>>> at late_initcall time but this shouldn't make a difference in practice
>>> i.e. when FIQs are actually used.
>>>
>>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>>
>> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> What kind of review did you do when giving that attributation?
I did the review to the best of my knowledge and with good will.
I read your comment on this patch and I learnt one more thing.
Today, I am smarter than yesterday and dumber than tomorrow :)
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [PATCH] powerpc/pseries Use remove_memory() to remove memory
From: Nathan Fontenot @ 2014-01-27 16:54 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org
The memory remove code for powerpc/pseries should call remove_memory()
so that we are holding the hotplug_memory lock during memory remove
operations.
This patch updates the memory node remove handler to call remove_memory()
and adds a ppc_md.remove_memory() entry to handle pseries specific work
that is called from arch_remove_memory().
During memory remove in pseries_remove_memblock() we have to stay with
removing memory one section at a time. This is needed because of how memory
resources are handled. During memory add for pseries (via the probe file in
sysfs) we add memory one section at a time which gives us a memory resource
for each section. Future patches will aim to address this so will not have
to remove memory one section at a time.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/machdep.h | 4 +
arch/powerpc/mm/mem.c | 7 +
arch/powerpc/platforms/pseries/hotplug-memory.c | 85 +++++++++++-------------
3 files changed, 50 insertions(+), 46 deletions(-)
Index: powerpc/arch/powerpc/platforms/pseries/hotplug-memory.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pseries/hotplug-memory.c 2014-01-27 08:47:19.000000000 -0600
+++ powerpc/arch/powerpc/platforms/pseries/hotplug-memory.c 2014-01-27 09:21:54.000000000 -0600
@@ -14,6 +14,7 @@
#include <linux/memblock.h>
#include <linux/vmalloc.h>
#include <linux/memory.h>
+#include <linux/memory_hotplug.h>
#include <asm/firmware.h>
#include <asm/machdep.h>
@@ -75,13 +76,27 @@
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-static int pseries_remove_memblock(unsigned long base, unsigned int memblock_size)
+static int pseries_remove_memory(u64 start, u64 size)
{
- unsigned long start, start_pfn;
- struct zone *zone;
int ret;
- unsigned long section;
- unsigned long sections_to_remove;
+
+ /* Remove htab bolted mappings for this section of memory */
+ start = (unsigned long)__va(start);
+ ret = remove_section_mapping(start, start + size);
+
+ /* Ensure all vmalloc mappings are flushed in case they also
+ * hit that section of memory
+ */
+ vm_unmap_aliases();
+
+ return ret;
+}
+
+static int pseries_remove_memblock(unsigned long base, unsigned int memblock_size)
+{
+ unsigned long block_sz, start_pfn;
+ int sections_per_block;
+ int i, nid;
start_pfn = base >> PAGE_SHIFT;
@@ -90,45 +105,21 @@
return 0;
}
- zone = page_zone(pfn_to_page(start_pfn));
-
- /*
- * Remove section mappings and sysfs entries for the
- * section of the memory we are removing.
- *
- * NOTE: Ideally, this should be done in generic code like
- * remove_memory(). But remove_memory() gets called by writing
- * to sysfs "state" file and we can't remove sysfs entries
- * while writing to it. So we have to defer it to here.
- */
- sections_to_remove = (memblock_size >> PAGE_SHIFT) / PAGES_PER_SECTION;
- for (section = 0; section < sections_to_remove; section++) {
- unsigned long pfn = start_pfn + section * PAGES_PER_SECTION;
- ret = __remove_pages(zone, pfn, PAGES_PER_SECTION);
- if (ret)
- return ret;
+ block_sz = memory_block_size_bytes();
+ sections_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
+ nid = memory_add_physaddr_to_nid(base);
+
+ for (i = 0; i < sections_per_block; i++) {
+ remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
+ base += MIN_MEMORY_BLOCK_SIZE;
}
- /*
- * Update memory regions for memory remove
- */
+ /* Update memory regions for memory remove */
memblock_remove(base, memblock_size);
-
- /*
- * Remove htab bolted mappings for this section of memory
- */
- start = (unsigned long)__va(base);
- ret = remove_section_mapping(start, start + memblock_size);
-
- /* Ensure all vmalloc mappings are flushed in case they also
- * hit that section of memory
- */
- vm_unmap_aliases();
-
- return ret;
+ return 0;
}
-static int pseries_remove_memory(struct device_node *np)
+static int pseries_remove_mem_node(struct device_node *np)
{
const char *type;
const unsigned int *regs;
@@ -153,8 +144,8 @@
base = *(unsigned long *)regs;
lmb_size = regs[3];
- ret = pseries_remove_memblock(base, lmb_size);
- return ret;
+ pseries_remove_memblock(base, lmb_size);
+ return 0;
}
#else
static inline int pseries_remove_memblock(unsigned long base,
@@ -162,13 +153,13 @@
{
return -EOPNOTSUPP;
}
-static inline int pseries_remove_memory(struct device_node *np)
+static inline int pseries_remove_mem_node(struct device_node *np)
{
return -EOPNOTSUPP;
}
#endif /* CONFIG_MEMORY_HOTREMOVE */
-static int pseries_add_memory(struct device_node *np)
+static int pseries_add_mem_node(struct device_node *np)
{
const char *type;
const unsigned int *regs;
@@ -254,10 +245,10 @@
switch (action) {
case OF_RECONFIG_ATTACH_NODE:
- err = pseries_add_memory(node);
+ err = pseries_add_mem_node(node);
break;
case OF_RECONFIG_DETACH_NODE:
- err = pseries_remove_memory(node);
+ err = pseries_remove_mem_node(node);
break;
case OF_RECONFIG_UPDATE_PROPERTY:
pr = (struct of_prop_reconfig *)node;
@@ -277,6 +268,10 @@
if (firmware_has_feature(FW_FEATURE_LPAR))
of_reconfig_notifier_register(&pseries_mem_nb);
+#ifdef CONFIG_MEMORY_HOTREMOVE
+ ppc_md.remove_memory = pseries_remove_memory;
+#endif
+
return 0;
}
machine_device_initcall(pseries, pseries_memory_hotplug_init);
Index: powerpc/arch/powerpc/include/asm/machdep.h
===================================================================
--- powerpc.orig/arch/powerpc/include/asm/machdep.h 2014-01-27 08:47:18.000000000 -0600
+++ powerpc/arch/powerpc/include/asm/machdep.h 2014-01-27 08:48:31.000000000 -0600
@@ -279,6 +279,10 @@
#ifdef CONFIG_ARCH_RANDOM
int (*get_random_long)(unsigned long *v);
#endif
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+ int (*remove_memory)(u64, u64);
+#endif
};
extern void e500_idle(void);
Index: powerpc/arch/powerpc/mm/mem.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/mem.c 2014-01-27 08:47:18.000000000 -0600
+++ powerpc/arch/powerpc/mm/mem.c 2014-01-27 08:48:31.000000000 -0600
@@ -139,9 +139,14 @@
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
struct zone *zone;
+ int ret;
zone = page_zone(pfn_to_page(start_pfn));
- return __remove_pages(zone, start_pfn, nr_pages);
+ ret = __remove_pages(zone, start_pfn, nr_pages);
+ if (!ret && (ppc_md.remove_memory))
+ ret = ppc_md.remove_memory(start, size);
+
+ return ret;
}
#endif
#endif /* CONFIG_MEMORY_HOTPLUG */
^ permalink raw reply
* Re: [PATCH] slub: Don't throw away partial remote slabs if there is no local memory
From: Christoph Lameter @ 2014-01-27 16:24 UTC (permalink / raw)
To: Nishanth Aravamudan
Cc: Han Pingtian, mpm, penberg, linux-mm, paulus, Anton Blanchard,
David Rientjes, Joonsoo Kim, linuxppc-dev, Wanpeng Li
In-Reply-To: <20140125001643.GA25344@linux.vnet.ibm.com>
On Fri, 24 Jan 2014, Nishanth Aravamudan wrote:
> What I find odd is that there are only 2 nodes on this system, node 0
> (empty) and node 1. So won't numa_mem_id() always be 1? And every page
> should be coming from node 1 (thus node_match() should always be true?)
Well yes that occurs if you specify the node or just always use the
default memory allocation policy.
In order to spread the allocatios over both node you would have to set the
tasks memory allocation policy to MPOL_INTERLEAVE.
^ permalink raw reply
* Re: [PATCH] slub: Don't throw away partial remote slabs if there is no local memory
From: Christoph Lameter @ 2014-01-27 16:16 UTC (permalink / raw)
To: David Rientjes
Cc: Han Pingtian, Nishanth Aravamudan, penberg, linux-mm, paulus,
Anton Blanchard, mpm, Joonsoo Kim, linuxppc-dev, Wanpeng Li
In-Reply-To: <alpine.DEB.2.02.1401241543100.18620@chino.kir.corp.google.com>
On Fri, 24 Jan 2014, David Rientjes wrote:
> kmalloc_node(nid) and kmem_cache_alloc_node(nid) should fallback to nodes
> other than nid when memory can't be allocated, these functions only
> indicate a preference.
The nid passed indicated a preference unless __GFP_THIS_NODE is specified.
Then the allocation must occur on that node.
^ permalink raw reply
* Re: [PATCH] slub: Don't throw away partial remote slabs if there is no local memory
From: Christoph Lameter @ 2014-01-27 16:18 UTC (permalink / raw)
To: Nishanth Aravamudan
Cc: Han Pingtian, mpm, penberg, linux-mm, paulus, Anton Blanchard,
David Rientjes, Joonsoo Kim, linuxppc-dev, Wanpeng Li
In-Reply-To: <20140125011041.GB25344@linux.vnet.ibm.com>
On Fri, 24 Jan 2014, Nishanth Aravamudan wrote:
> As to cpu_to_node() being passed to kmalloc_node(), I think an
> appropriate fix is to change that to cpu_to_mem()?
Yup.
> > Yeah, the default policy should be to fallback to local memory if the node
> > passed is memoryless.
>
> Thanks!
I would suggest to use NUMA_NO_NODE instead. That will fit any slab that
we may be currently allocating from or can get a hold of and is mosty
efficient.
^ permalink raw reply
* Re: [PATCH 1/9] ARM: get rid of arch_cpu_idle_prepare()
From: Russell King - ARM Linux @ 2014-01-27 16:07 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Nicolas Pitre, linaro-kernel, linux-sh, Peter Zijlstra,
Catalin Marinas, linux-pm, Rafael J. Wysocki, linux-kernel,
Paul Mundt, Thomas Gleixner, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
In-Reply-To: <52E6175F.1050401@linaro.org>
On Mon, Jan 27, 2014 at 09:22:55AM +0100, Daniel Lezcano wrote:
> On 01/27/2014 07:08 AM, Nicolas Pitre wrote:
>> ARM and ARM64 are the only two architectures implementing
>> arch_cpu_idle_prepare() simply to call local_fiq_enable().
>>
>> We have secondary_start_kernel() already calling local_fiq_enable() and
>> this is done a second time in arch_cpu_idle_prepare() in that case. And
>> enabling FIQs has nothing to do with idling the CPU to start with.
>>
>> So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
>> CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
>> at late_initcall time but this shouldn't make a difference in practice
>> i.e. when FIQs are actually used.
>>
>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>
> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
What kind of review did you do when giving that attributation?
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* Re: [PATCH 1/9] ARM: get rid of arch_cpu_idle_prepare()
From: Russell King - ARM Linux @ 2014-01-27 16:06 UTC (permalink / raw)
To: Nicolas Pitre
Cc: linaro-kernel, linux-sh, Peter Zijlstra, Catalin Marinas,
linux-pm, Daniel Lezcano, Rafael J. Wysocki, linux-kernel,
Paul Mundt, Thomas Gleixner, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
In-Reply-To: <alpine.LFD.2.11.1401271004060.1652@knanqh.ubzr>
On Mon, Jan 27, 2014 at 10:45:59AM -0500, Nicolas Pitre wrote:
> On Mon, 27 Jan 2014, Russell King - ARM Linux wrote:
>
> > On Mon, Jan 27, 2014 at 01:08:16AM -0500, Nicolas Pitre wrote:
> > > ARM and ARM64 are the only two architectures implementing
> > > arch_cpu_idle_prepare() simply to call local_fiq_enable().
> > >
> > > We have secondary_start_kernel() already calling local_fiq_enable() and
> > > this is done a second time in arch_cpu_idle_prepare() in that case. And
> > > enabling FIQs has nothing to do with idling the CPU to start with.
> > >
> > > So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
> > > CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
> > > at late_initcall time but this shouldn't make a difference in practice
> > > i.e. when FIQs are actually used.
> > >
> > > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > > ---
> > > arch/arm/kernel/process.c | 5 -----
> > > arch/arm/kernel/setup.c | 7 +++++++
> > > 2 files changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> > > index 92f7b15dd2..725b8c95e0 100644
> > > --- a/arch/arm/kernel/process.c
> > > +++ b/arch/arm/kernel/process.c
> > > @@ -142,11 +142,6 @@ static void default_idle(void)
> > > local_irq_enable();
> > > }
> > >
> > > -void arch_cpu_idle_prepare(void)
> > > -{
> > > - local_fiq_enable();
> > > -}
> > > -
> > > void arch_cpu_idle_enter(void)
> > > {
> > > ledtrig_cpu(CPU_LED_IDLE_START);
> > > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > > index 987a7f5bce..d027b1a6fe 100644
> > > --- a/arch/arm/kernel/setup.c
> > > +++ b/arch/arm/kernel/setup.c
> > > @@ -789,6 +789,13 @@ static int __init init_machine_late(void)
> > > }
> > > late_initcall(init_machine_late);
> > >
> > > +static int __init init_fiq_boot_cpu(void)
> > > +{
> > > + local_fiq_enable();
> > > + return 0;
> > > +}
> > > +late_initcall(init_fiq_boot_cpu);
> >
> > arch_cpu_idle_prepare() gets called from the swapper thread, and changes
> > the swapper thread's CPSR. init_fiq_boot_cpu() gets called from PID1, the
> > init thread, and changes the init thread's CPSR, which will already have
> > FIQs enabled by way of how kernel threads are created.
> >
> > Hence, the above code fragment has no effect what so ever, and those
> > platforms using FIQs will not have FIQs delivered if they're idle
> > (because the swapper will have FIQs masked at the CPU.)
>
> You're right.
>
> What about moving local_fiq_enable() to trap_init() then?
That's potentially unsafe, as we haven't touched any of the IRQ
controllers at that point - we can't guarantee what state they'd be
in. Given that the default FIQ is to just return, a FIQ being raised
at that point will end up with an infinite loop re-entering the FIQ
handler.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* Re: [PATCH 2/9] ARM64: get rid of arch_cpu_idle_prepare()
From: Catalin Marinas @ 2014-01-27 15:57 UTC (permalink / raw)
To: Nicolas Pitre
Cc: linaro-kernel@lists.linaro.org, Russell King,
linux-sh@vger.kernel.org, Peter Zijlstra,
linux-pm@vger.kernel.org, Daniel Lezcano, Rafael J. Wysocki,
linux-kernel@vger.kernel.org, Paul Mundt, Thomas Gleixner,
linuxppc-dev@lists.ozlabs.org, Ingo Molnar,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <alpine.LFD.2.11.1401271047240.1652@knanqh.ubzr>
On Mon, Jan 27, 2014 at 03:51:02PM +0000, Nicolas Pitre wrote:
> On Mon, 27 Jan 2014, Catalin Marinas wrote:
>
> > On Mon, Jan 27, 2014 at 06:08:17AM +0000, Nicolas Pitre wrote:
> > > ARM and ARM64 are the only two architectures implementing
> > > arch_cpu_idle_prepare() simply to call local_fiq_enable().
> > >
> > > We have secondary_start_kernel() already calling local_fiq_enable() and
> > > this is done a second time in arch_cpu_idle_prepare() in that case. And
> > > enabling FIQs has nothing to do with idling the CPU to start with.
> > >
> > > So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
> > > CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
> > > at late_initcall time but this shouldn't make a difference in practice
> > > given that FIQs are not currently used on ARM64.
> > >
> > > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> >
> > For arm64, we could simply remove any reference to FIQs. I'm not aware
> > of anyone using them.
>
> OK. What if I sumply remove arch_cpu_idle_prepare() and let you do the
> remove the rest?
>
> IMHO I'd simply remove local_fiq_{enable/disable}() from
> arm64/kernel/smp.c and leave the infrastructure in place in case someone
> needs it eventually. In which case I could include that into my patch
> as well.
Sounds good. We can keep the local_fiq_*() functions but remove about 4
calling sites (process.c and smp.c) until needed.
Thanks.
--
Catalin
^ permalink raw reply
* Re: [PATCH v2] powernv: kvm: make _PAGE_NUMA take effect
From: Aneesh Kumar K.V @ 2014-01-27 15:57 UTC (permalink / raw)
To: Alexander Graf
Cc: kvm-devel, kvm-ppc, Paul Mackerras, Liu Ping Fan, linuxppc-dev
In-Reply-To: <72133741-22DA-4477-B89F-E9C978FA294F@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 27.01.2014, at 11:28, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> Alexander Graf <agraf@suse.de> writes:
>>
>>> On 21.01.2014, at 10:42, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>>
>>>> Liu Ping Fan <kernelfans@gmail.com> writes:
>>>>
>>>>> To make sure that on host, the pages marked with _PAGE_NUMA result in a fault
>>>>> when guest access them, we should force the checking when guest uses hypercall
>>>>> to setup hpte.
>>>>>
>>>>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>>>>
>>>> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>
>>>> When we mark pte with _PAGE_NUMA we already call mmu_notifier_invalidate_range_start and
>>>> mmu_notifier_invalidate_range_end, which will mark existing guest hpte
>>>> entry as HPTE_V_ABSENT. Now we need to do that when we are inserting new
>>>> guest hpte entries. This patch does that.
>>>
>>> So what happens next? We insert a page into the HTAB without
>>> HPTE_V_VALID set, so the guest will fail to use it. If the guest does
>>> an H_READ on it it will suddenly turn to V_VALID though?
>>
>> As per the guest the entry is valid, so yes an hread should return a
>> valid entry. But in real hpte we would mark it not valid.
>
> Ah, yes.
>
>>
>>>
>>> I might need a crash course in the use of HPTE_V_ABSENT.
>>
>> When guest tries to access the address, the host will handle the fault.
>>
>> kvmppc_hpte_hv_fault should give more info
>
> Thanks for the pointer. So we fault it in lazily. Is there any
> particular reason we can't do that on h_enter already? After all this
> just means an additional roundtrip because the guest is pretty likely
> to use the page it just entered, no?
We could get wrong numa fault information if we didn't do h_enter from
the right node from which we faulted.
-aneesh
^ permalink raw reply
* Re: [PATCH 2/9] ARM64: get rid of arch_cpu_idle_prepare()
From: Catalin Marinas @ 2014-01-27 15:43 UTC (permalink / raw)
To: Nicolas Pitre
Cc: linaro-kernel@lists.linaro.org, Russell King,
linux-sh@vger.kernel.org, Peter Zijlstra,
linux-pm@vger.kernel.org, Daniel Lezcano, Rafael J. Wysocki,
linux-kernel@vger.kernel.org, Paul Mundt, Thomas Gleixner,
linuxppc-dev@lists.ozlabs.org, Ingo Molnar,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1390802904-28399-3-git-send-email-nicolas.pitre@linaro.org>
On Mon, Jan 27, 2014 at 06:08:17AM +0000, Nicolas Pitre wrote:
> ARM and ARM64 are the only two architectures implementing
> arch_cpu_idle_prepare() simply to call local_fiq_enable().
>
> We have secondary_start_kernel() already calling local_fiq_enable() and
> this is done a second time in arch_cpu_idle_prepare() in that case. And
> enabling FIQs has nothing to do with idling the CPU to start with.
>
> So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
> CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
> at late_initcall time but this shouldn't make a difference in practice
> given that FIQs are not currently used on ARM64.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
For arm64, we could simply remove any reference to FIQs. I'm not aware
of anyone using them.
--
Catalin
^ permalink raw reply
* Re: [PATCH 2/9] ARM64: get rid of arch_cpu_idle_prepare()
From: Nicolas Pitre @ 2014-01-27 15:51 UTC (permalink / raw)
To: Catalin Marinas
Cc: linaro-kernel@lists.linaro.org, Russell King,
linux-sh@vger.kernel.org, Peter Zijlstra,
linux-pm@vger.kernel.org, Daniel Lezcano, Rafael J. Wysocki,
linux-kernel@vger.kernel.org, Paul Mundt, Thomas Gleixner,
linuxppc-dev@lists.ozlabs.org, Ingo Molnar,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20140127154315.GG32608@arm.com>
On Mon, 27 Jan 2014, Catalin Marinas wrote:
> On Mon, Jan 27, 2014 at 06:08:17AM +0000, Nicolas Pitre wrote:
> > ARM and ARM64 are the only two architectures implementing
> > arch_cpu_idle_prepare() simply to call local_fiq_enable().
> >
> > We have secondary_start_kernel() already calling local_fiq_enable() and
> > this is done a second time in arch_cpu_idle_prepare() in that case. And
> > enabling FIQs has nothing to do with idling the CPU to start with.
> >
> > So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
> > CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
> > at late_initcall time but this shouldn't make a difference in practice
> > given that FIQs are not currently used on ARM64.
> >
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
>
> For arm64, we could simply remove any reference to FIQs. I'm not aware
> of anyone using them.
OK. What if I sumply remove arch_cpu_idle_prepare() and let you do the
remove the rest?
IMHO I'd simply remove local_fiq_{enable/disable}() from
arm64/kernel/smp.c and leave the infrastructure in place in case someone
needs it eventually. In which case I could include that into my patch
as well.
Nicolas
^ permalink raw reply
* Re: [PATCH 1/9] ARM: get rid of arch_cpu_idle_prepare()
From: Nicolas Pitre @ 2014-01-27 15:45 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linaro-kernel, linux-sh, Peter Zijlstra, Catalin Marinas,
linux-pm, Daniel Lezcano, Rafael J. Wysocki, linux-kernel,
Paul Mundt, Thomas Gleixner, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
In-Reply-To: <20140127124511.GK15937@n2100.arm.linux.org.uk>
On Mon, 27 Jan 2014, Russell King - ARM Linux wrote:
> On Mon, Jan 27, 2014 at 01:08:16AM -0500, Nicolas Pitre wrote:
> > ARM and ARM64 are the only two architectures implementing
> > arch_cpu_idle_prepare() simply to call local_fiq_enable().
> >
> > We have secondary_start_kernel() already calling local_fiq_enable() and
> > this is done a second time in arch_cpu_idle_prepare() in that case. And
> > enabling FIQs has nothing to do with idling the CPU to start with.
> >
> > So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
> > CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
> > at late_initcall time but this shouldn't make a difference in practice
> > i.e. when FIQs are actually used.
> >
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > ---
> > arch/arm/kernel/process.c | 5 -----
> > arch/arm/kernel/setup.c | 7 +++++++
> > 2 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> > index 92f7b15dd2..725b8c95e0 100644
> > --- a/arch/arm/kernel/process.c
> > +++ b/arch/arm/kernel/process.c
> > @@ -142,11 +142,6 @@ static void default_idle(void)
> > local_irq_enable();
> > }
> >
> > -void arch_cpu_idle_prepare(void)
> > -{
> > - local_fiq_enable();
> > -}
> > -
> > void arch_cpu_idle_enter(void)
> > {
> > ledtrig_cpu(CPU_LED_IDLE_START);
> > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > index 987a7f5bce..d027b1a6fe 100644
> > --- a/arch/arm/kernel/setup.c
> > +++ b/arch/arm/kernel/setup.c
> > @@ -789,6 +789,13 @@ static int __init init_machine_late(void)
> > }
> > late_initcall(init_machine_late);
> >
> > +static int __init init_fiq_boot_cpu(void)
> > +{
> > + local_fiq_enable();
> > + return 0;
> > +}
> > +late_initcall(init_fiq_boot_cpu);
>
> arch_cpu_idle_prepare() gets called from the swapper thread, and changes
> the swapper thread's CPSR. init_fiq_boot_cpu() gets called from PID1, the
> init thread, and changes the init thread's CPSR, which will already have
> FIQs enabled by way of how kernel threads are created.
>
> Hence, the above code fragment has no effect what so ever, and those
> platforms using FIQs will not have FIQs delivered if they're idle
> (because the swapper will have FIQs masked at the CPU.)
You're right.
What about moving local_fiq_enable() to trap_init() then?
Nicolas
^ permalink raw reply
* Re: [PATCH 0/9] setting the table for integration of cpuidle with the scheduler
From: Peter Zijlstra @ 2014-01-27 12:47 UTC (permalink / raw)
To: Nicolas Pitre
Cc: linaro-kernel, Russell King, linux-sh, Catalin Marinas, linux-pm,
Daniel Lezcano, Rafael J. Wysocki, linux-kernel, Paul Mundt,
Thomas Gleixner, linuxppc-dev, Ingo Molnar, linux-arm-kernel
In-Reply-To: <1390802904-28399-1-git-send-email-nicolas.pitre@linaro.org>
On Mon, Jan 27, 2014 at 01:08:15AM -0500, Nicolas Pitre wrote:
> As everyone should know by now, we want to integrate the cpuidle
> governor with the scheduler for a more efficient idling of CPUs.
> In order to help the transition, this small patch series moves the
> existing interaction with cpuidle from architecture code to generic
> core code. No functional change should have occurred yet.
>
> The ARM, PPC, SH and X86 architectures are concerned. Small cleanups
> to ARM and ARM64 are also included. I don't know yet the best path for
> those patches to get into mainline, but it is probably best if they
> stay together. So ACKs from architecture maintainers would be greatly
> appreciated.
>
>
> arch/arm/kernel/process.c | 21 +++---------
> arch/arm/kernel/setup.c | 7 ++++
> arch/arm64/kernel/process.c | 5 ---
> arch/arm64/kernel/setup.c | 7 ++++
> arch/powerpc/platforms/pseries/processor_idle.c | 5 +++
> arch/powerpc/platforms/pseries/setup.c | 34 ++++++++-----------
> arch/sh/kernel/idle.c | 4 +--
> arch/x86/kernel/process.c | 5 +--
> include/linux/cpu.h | 1 -
> kernel/Makefile | 1 -
> kernel/cpu/Makefile | 1 -
> kernel/sched/Makefile | 2 +-
> kernel/{cpu => sched}/idle.c | 6 ++--
> 13 files changed, 44 insertions(+), 55 deletions(-)
Thomas, any objections to this? It looks like a sensible thing to do.
^ permalink raw reply
* Re: [PATCH 1/9] ARM: get rid of arch_cpu_idle_prepare()
From: Russell King - ARM Linux @ 2014-01-27 12:45 UTC (permalink / raw)
To: Nicolas Pitre
Cc: linaro-kernel, linux-sh, Peter Zijlstra, Catalin Marinas,
linux-pm, Daniel Lezcano, Rafael J. Wysocki, linux-kernel,
Paul Mundt, Thomas Gleixner, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
In-Reply-To: <1390802904-28399-2-git-send-email-nicolas.pitre@linaro.org>
On Mon, Jan 27, 2014 at 01:08:16AM -0500, Nicolas Pitre wrote:
> ARM and ARM64 are the only two architectures implementing
> arch_cpu_idle_prepare() simply to call local_fiq_enable().
>
> We have secondary_start_kernel() already calling local_fiq_enable() and
> this is done a second time in arch_cpu_idle_prepare() in that case. And
> enabling FIQs has nothing to do with idling the CPU to start with.
>
> So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
> CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
> at late_initcall time but this shouldn't make a difference in practice
> i.e. when FIQs are actually used.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/kernel/process.c | 5 -----
> arch/arm/kernel/setup.c | 7 +++++++
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 92f7b15dd2..725b8c95e0 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -142,11 +142,6 @@ static void default_idle(void)
> local_irq_enable();
> }
>
> -void arch_cpu_idle_prepare(void)
> -{
> - local_fiq_enable();
> -}
> -
> void arch_cpu_idle_enter(void)
> {
> ledtrig_cpu(CPU_LED_IDLE_START);
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 987a7f5bce..d027b1a6fe 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -789,6 +789,13 @@ static int __init init_machine_late(void)
> }
> late_initcall(init_machine_late);
>
> +static int __init init_fiq_boot_cpu(void)
> +{
> + local_fiq_enable();
> + return 0;
> +}
> +late_initcall(init_fiq_boot_cpu);
arch_cpu_idle_prepare() gets called from the swapper thread, and changes
the swapper thread's CPSR. init_fiq_boot_cpu() gets called from PID1, the
init thread, and changes the init thread's CPSR, which will already have
FIQs enabled by way of how kernel threads are created.
Hence, the above code fragment has no effect what so ever, and those
platforms using FIQs will not have FIQs delivered if they're idle
(because the swapper will have FIQs masked at the CPU.)
NAK.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* Re: [PATCH 6/9] PPC: remove redundant cpuidle_idle_call()
From: Preeti U Murthy @ 2014-01-27 11:59 UTC (permalink / raw)
To: Nicolas Pitre
Cc: linaro-kernel, Russell King, linux-pm, Peter Zijlstra,
Catalin Marinas, Daniel Lezcano, Rafael J. Wysocki, linux-kernel,
linux-sh, Paul Mundt, Thomas Gleixner, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
In-Reply-To: <1390802904-28399-7-git-send-email-nicolas.pitre@linaro.org>
Hi Nicolas,
On 01/27/2014 11:38 AM, Nicolas Pitre wrote:
> The core idle loop now takes care of it. However a few things need
> checking:
>
> - Invocation of cpuidle_idle_call() in pseries_lpar_idle() happened
> through arch_cpu_idle() and was therefore always preceded by a call
> to ppc64_runlatch_off(). To preserve this property now that
> cpuidle_idle_call() is invoked directly from core code, a call to
> ppc64_runlatch_off() has been added to idle_loop_prolog() in
> platforms/pseries/processor_idle.c.
>
> - Similarly, cpuidle_idle_call() was followed by ppc64_runlatch_off()
> so a call to the later has been added to idle_loop_epilog().
>
> - And since arch_cpu_idle() always made sure to re-enable IRQs if they
> were not enabled, this is now
> done in idle_loop_epilog() as well.
>
> The above was made in order to keep the execution flow close to the
> original. I don't know if that was strictly necessary. Someone well
> aquainted with the platform details might find some room for possible
> optimizations.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/powerpc/platforms/pseries/processor_idle.c | 5 ++++
> arch/powerpc/platforms/pseries/setup.c | 34 ++++++++++---------------
> 2 files changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
> index a166e38bd6..72ddfe3d2f 100644
> --- a/arch/powerpc/platforms/pseries/processor_idle.c
> +++ b/arch/powerpc/platforms/pseries/processor_idle.c
> @@ -33,6 +33,7 @@ static struct cpuidle_state *cpuidle_state_table;
>
> static inline void idle_loop_prolog(unsigned long *in_purr)
> {
> + ppc64_runlatch_off();
> *in_purr = mfspr(SPRN_PURR);
> /*
> * Indicate to the HV that we are idle. Now would be
> @@ -49,6 +50,10 @@ static inline void idle_loop_epilog(unsigned long in_purr)
> wait_cycles += mfspr(SPRN_PURR) - in_purr;
> get_lppaca()->wait_state_cycles = cpu_to_be64(wait_cycles);
> get_lppaca()->idle = 0;
> +
> + if (irqs_disabled())
> + local_irq_enable();
> + ppc64_runlatch_on();
> }
>
> static int snooze_loop(struct cpuidle_device *dev,
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index c1f1908587..7604c19d54 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -39,7 +39,6 @@
> #include <linux/irq.h>
> #include <linux/seq_file.h>
> #include <linux/root_dev.h>
> -#include <linux/cpuidle.h>
> #include <linux/of.h>
> #include <linux/kexec.h>
>
> @@ -356,29 +355,24 @@ early_initcall(alloc_dispatch_log_kmem_cache);
>
> static void pseries_lpar_idle(void)
> {
> - /* This would call on the cpuidle framework, and the back-end pseries
> - * driver to go to idle states
> + /*
> + * Default handler to go into low thread priority and possibly
> + * low power mode by cedeing processor to hypervisor
> */
> - if (cpuidle_idle_call()) {
> - /* On error, execute default handler
> - * to go into low thread priority and possibly
> - * low power mode by cedeing processor to hypervisor
> - */
>
> - /* Indicate to hypervisor that we are idle. */
> - get_lppaca()->idle = 1;
> + /* Indicate to hypervisor that we are idle. */
> + get_lppaca()->idle = 1;
>
> - /*
> - * Yield the processor to the hypervisor. We return if
> - * an external interrupt occurs (which are driven prior
> - * to returning here) or if a prod occurs from another
> - * processor. When returning here, external interrupts
> - * are enabled.
> - */
> - cede_processor();
> + /*
> + * Yield the processor to the hypervisor. We return if
> + * an external interrupt occurs (which are driven prior
> + * to returning here) or if a prod occurs from another
> + * processor. When returning here, external interrupts
> + * are enabled.
> + */
> + cede_processor();
>
> - get_lppaca()->idle = 0;
> - }
> + get_lppaca()->idle = 0;
> }
>
> /*
>
Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
The consequence of this would be for other Power platforms like PowerNV,
we will need to invoke ppc_runlatch_off() and ppc_runlatch_on() in each
of the idle routines since the idle_loop_prologue() and
idle_loop_epilogue() are not invoked by them, but we will take care of this.
Regards
Preeti U Murthy
^ permalink raw reply
* Re: [PATCH v2] powernv: kvm: make _PAGE_NUMA take effect
From: Alexander Graf @ 2014-01-27 10:42 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: kvm-devel, kvm-ppc, Paul Mackerras, Liu Ping Fan, linuxppc-dev
In-Reply-To: <87wqhllnvm.fsf@linux.vnet.ibm.com>
On 27.01.2014, at 11:28, Aneesh Kumar K.V =
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> Alexander Graf <agraf@suse.de> writes:
>=20
>> On 21.01.2014, at 10:42, Aneesh Kumar K.V =
<aneesh.kumar@linux.vnet.ibm.com> wrote:
>>=20
>>> Liu Ping Fan <kernelfans@gmail.com> writes:
>>>=20
>>>> To make sure that on host, the pages marked with _PAGE_NUMA result =
in a fault
>>>> when guest access them, we should force the checking when guest =
uses hypercall
>>>> to setup hpte.
>>>>=20
>>>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>>>=20
>>> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>=20
>>> When we mark pte with _PAGE_NUMA we already call =
mmu_notifier_invalidate_range_start and
>>> mmu_notifier_invalidate_range_end, which will mark existing guest =
hpte
>>> entry as HPTE_V_ABSENT. Now we need to do that when we are inserting =
new
>>> guest hpte entries. This patch does that.=20
>>=20
>> So what happens next? We insert a page into the HTAB without
>> HPTE_V_VALID set, so the guest will fail to use it. If the guest does
>> an H_READ on it it will suddenly turn to V_VALID though?
>=20
> As per the guest the entry is valid, so yes an hread should return a
> valid entry. But in real hpte we would mark it not valid.
Ah, yes.
>=20
>>=20
>> I might need a crash course in the use of HPTE_V_ABSENT.
>=20
> When guest tries to access the address, the host will handle the =
fault.
>=20
> kvmppc_hpte_hv_fault should give more info
Thanks for the pointer. So we fault it in lazily. Is there any =
particular reason we can't do that on h_enter already? After all this =
just means an additional roundtrip because the guest is pretty likely to =
use the page it just entered, no?
Alex
^ 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