* Re: [PATCH 2/2] of: search the best compatible match first in __of_match_node()
From: Kumar Gala @ 2014-02-14 16:19 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree@vger.kernel.org, Kevin Hao, Stephen N Chivers,
Rob Herring, Grant Likely, linuxppc-dev, Sebastian Hesselbarth
In-Reply-To: <CAL_JsqKq4_1K8EF+PZoP=0=H6tiRxbgdzs9UHHVdbHS014n74Q@mail.gmail.com>
On Feb 14, 2014, at 9:53 AM, Rob Herring <robherring2@gmail.com> wrote:
> On Thu, Feb 13, 2014 at 11:22 PM, Kevin Hao <haokexin@gmail.com> =
wrote:
>> Currently, of_match_node compares each given match against all node's
>> compatible strings with of_device_is_compatible.
>>=20
>> To achieve multiple compatible strings per node with ordering from
>> specific to generic, this requires given matches to be ordered from
>> specific to generic. For most of the drivers this is not true and =
also
>> an alphabetical ordering is more sane there.
>>=20
>> Therefore, this patch introduces a function to match each of the =
node's
>> compatible strings against all given compatible matches without type =
and
>> name first, before checking the next compatible string. This implies
>> that node's compatibles are ordered from specific to generic while
>> given matches can be in any order. If we fail to find such a match
>> entry, then fall-back to the old method in order to keep =
compatibility.
>>=20
>> Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>> Signed-off-by: Kevin Hao <haokexin@gmail.com>
>=20
> Looks good to me. I'll put this in next for a few days. I'd really
> like to see some acks and tested-by's before sending to Linus.
>=20
> We could be a bit more strict here and fallback to the old matching if
> the match table has any entries with name or type. I don't think that
> should be necessary though.
>=20
> Rob
Can you push the revert to Linus sooner, since currently a ton of boards =
wouldn=92t be working on the PPC side, so at least -rc3 has the =
possibility of working for them.
- k
>=20
>> ---
>> drivers/of/base.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 42 insertions(+), 1 deletion(-)
>>=20
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index ba195fbce4c6..10b51106c854 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -730,13 +730,49 @@ out:
>> }
>> EXPORT_SYMBOL(of_find_node_with_property);
>>=20
>> +static const struct of_device_id *
>> +of_match_compatible(const struct of_device_id *matches,
>> + const struct device_node *node)
>> +{
>> + const char *cp;
>> + int cplen, l;
>> + const struct of_device_id *m;
>> +
>> + cp =3D __of_get_property(node, "compatible", &cplen);
>> + while (cp && (cplen > 0)) {
>> + m =3D matches;
>> + while (m->name[0] || m->type[0] || m->compatible[0]) =
{
>> + /* Only match for the entries without type =
and name */
>> + if (m->name[0] || m->type[0] ||
>> + of_compat_cmp(m->compatible, cp,
>> + strlen(m->compatible)))
>> + m++;
>> + else
>> + return m;
>> + }
>> +
>> + /* Get node's next compatible string */
>> + l =3D strlen(cp) + 1;
>> + cp +=3D l;
>> + cplen -=3D l;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> static
>> const struct of_device_id *__of_match_node(const struct of_device_id =
*matches,
>> const struct device_node =
*node)
>> {
>> + const struct of_device_id *m;
>> +
>> if (!matches)
>> return NULL;
>>=20
>> + m =3D of_match_compatible(matches, node);
>> + if (m)
>> + return m;
>> +
>> while (matches->name[0] || matches->type[0] || =
matches->compatible[0]) {
>> int match =3D 1;
>> if (matches->name[0])
>> @@ -760,7 +796,12 @@ const struct of_device_id *__of_match_node(const =
struct of_device_id *matches,
>> * @matches: array of of device match structures to search =
in
>> * @node: the of device structure to match against
>> *
>> - * Low level utility function used by device matching.
>> + * Low level utility function used by device matching. We have =
two ways
>> + * of matching:
>> + * - Try to find the best compatible match by comparing each =
compatible
>> + * string of device node with all the given matches =
respectively.
>> + * - If the above method failed, then try to match the =
compatible by using
>> + * __of_device_is_compatible() besides the match in type and =
name.
>> */
>> const struct of_device_id *of_match_node(const struct of_device_id =
*matches,
>> const struct device_node =
*node)
>> --
>> 1.8.5.3
--=20
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, =
hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH 2/2] of: search the best compatible match first in __of_match_node()
From: Rob Herring @ 2014-02-14 15:53 UTC (permalink / raw)
To: Kevin Hao
Cc: devicetree@vger.kernel.org, Stephen N Chivers, Rob Herring,
Kumar Gala, Grant Likely, linuxppc-dev, Sebastian Hesselbarth
In-Reply-To: <1392355366-1445-3-git-send-email-haokexin@gmail.com>
On Thu, Feb 13, 2014 at 11:22 PM, Kevin Hao <haokexin@gmail.com> wrote:
> Currently, of_match_node compares each given match against all node's
> compatible strings with of_device_is_compatible.
>
> To achieve multiple compatible strings per node with ordering from
> specific to generic, this requires given matches to be ordered from
> specific to generic. For most of the drivers this is not true and also
> an alphabetical ordering is more sane there.
>
> Therefore, this patch introduces a function to match each of the node's
> compatible strings against all given compatible matches without type and
> name first, before checking the next compatible string. This implies
> that node's compatibles are ordered from specific to generic while
> given matches can be in any order. If we fail to find such a match
> entry, then fall-back to the old method in order to keep compatibility.
>
> Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Signed-off-by: Kevin Hao <haokexin@gmail.com>
Looks good to me. I'll put this in next for a few days. I'd really
like to see some acks and tested-by's before sending to Linus.
We could be a bit more strict here and fallback to the old matching if
the match table has any entries with name or type. I don't think that
should be necessary though.
Rob
> ---
> drivers/of/base.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ba195fbce4c6..10b51106c854 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -730,13 +730,49 @@ out:
> }
> EXPORT_SYMBOL(of_find_node_with_property);
>
> +static const struct of_device_id *
> +of_match_compatible(const struct of_device_id *matches,
> + const struct device_node *node)
> +{
> + const char *cp;
> + int cplen, l;
> + const struct of_device_id *m;
> +
> + cp = __of_get_property(node, "compatible", &cplen);
> + while (cp && (cplen > 0)) {
> + m = matches;
> + while (m->name[0] || m->type[0] || m->compatible[0]) {
> + /* Only match for the entries without type and name */
> + if (m->name[0] || m->type[0] ||
> + of_compat_cmp(m->compatible, cp,
> + strlen(m->compatible)))
> + m++;
> + else
> + return m;
> + }
> +
> + /* Get node's next compatible string */
> + l = strlen(cp) + 1;
> + cp += l;
> + cplen -= l;
> + }
> +
> + return NULL;
> +}
> +
> static
> const struct of_device_id *__of_match_node(const struct of_device_id *matches,
> const struct device_node *node)
> {
> + const struct of_device_id *m;
> +
> if (!matches)
> return NULL;
>
> + m = of_match_compatible(matches, node);
> + if (m)
> + return m;
> +
> while (matches->name[0] || matches->type[0] || matches->compatible[0]) {
> int match = 1;
> if (matches->name[0])
> @@ -760,7 +796,12 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
> * @matches: array of of device match structures to search in
> * @node: the of device structure to match against
> *
> - * Low level utility function used by device matching.
> + * Low level utility function used by device matching. We have two ways
> + * of matching:
> + * - Try to find the best compatible match by comparing each compatible
> + * string of device node with all the given matches respectively.
> + * - If the above method failed, then try to match the compatible by using
> + * __of_device_is_compatible() besides the match in type and name.
> */
> const struct of_device_id *of_match_node(const struct of_device_id *matches,
> const struct device_node *node)
> --
> 1.8.5.3
>
^ permalink raw reply
* Re: [PATCH 1/6] PCI, acpiphp: Use list_for_each_entry() for bus traversal
From: Rafael J. Wysocki @ 2014-02-14 13:39 UTC (permalink / raw)
To: Yijing Wang
Cc: Russell King, David Airlie, linux-pcmcia, Hanjun Guo, dri-devel,
linux-pci, Bjorn Helgaas, linuxppc-dev
In-Reply-To: <52FD7D3D.4030409@huawei.com>
On Friday, February 14, 2014 10:19:41 AM Yijing Wang wrote:
> On 2014/2/14 7:54, Rafael J. Wysocki wrote:
> > On Thursday, February 13, 2014 09:13:58 PM Yijing Wang wrote:
> >> Replace list_for_each() + pci_bus_b() with the simpler
> >> list_for_each_entry().
> >>
> >> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> >
> > Looks reasonable to me.
> >
> > Does it conflict with anything currently in linux-next (the linux-next branch
> > of linux-pm.git in particular)?
>
> Hi Rafael,
> I applied this to your linux-next branch successfully . No conflicts found.
Good. :-)
Please feel free to add my ACK to it.
Rafael
> >> ---
> >> drivers/pci/hotplug/acpiphp_glue.c | 6 +++---
> >> 1 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> >> index cd929ae..aee6a0a 100644
> >> --- a/drivers/pci/hotplug/acpiphp_glue.c
> >> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> >> @@ -450,7 +450,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
> >> */
> >> static unsigned char acpiphp_max_busnr(struct pci_bus *bus)
> >> {
> >> - struct list_head *tmp;
> >> + struct pci_bus *tmp;
> >> unsigned char max, n;
> >>
> >> /*
> >> @@ -463,8 +463,8 @@ static unsigned char acpiphp_max_busnr(struct pci_bus *bus)
> >> */
> >> max = bus->busn_res.start;
> >>
> >> - list_for_each(tmp, &bus->children) {
> >> - n = pci_bus_max_busnr(pci_bus_b(tmp));
> >> + list_for_each_entry(tmp, &bus->children, node) {
> >> + n = pci_bus_max_busnr(tmp);
> >> if (n > max)
> >> max = n;
> >> }
> >>
> >
>
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()
From: Deepthi Dharwar @ 2014-02-14 11:25 UTC (permalink / raw)
To: Preeti U Murthy
Cc: Nicolas Pitre, linaro-kernel, linux-pm, Peter Zijlstra,
Daniel Lezcano, Rafael J. Wysocki, linux-kernel, Ingo Molnar,
Thomas Gleixner, linuxppc-dev, linux-arm-kernel
In-Reply-To: <52FDFB4C.7010607@linux.vnet.ibm.com>
On 02/14/2014 04:47 PM, Preeti U Murthy wrote:
> Hi Nicolas,
>
> You will have to include the below patch with yours. You
> could squash the two I guess, I have added the changelog
> just for clarity. And you also might want to change the subject to
> cpuidle/powernv. It gives a better picture.
>
> Thanks
>
> Regards
> Preeti U Murthy
>
>
> cpuidle/powernv: Add ppc64_runlatch_off/on() to idle routines
>
> Following moving of cpuidle_idle_call() to the generic idle loop, we need to
> add the runlatch functions to the idle routines on powernv which was earlier
> taken care of by the arch specific idle routine.
>
> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Reviewed-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
> ---
> drivers/cpuidle/cpuidle-powernv.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
> index 78fd174..f48607c 100644
> --- a/drivers/cpuidle/cpuidle-powernv.c
> +++ b/drivers/cpuidle/cpuidle-powernv.c
> @@ -14,6 +14,7 @@
>
> #include <asm/machdep.h>
> #include <asm/firmware.h>
> +#include <asm/runlatch.h>
>
> struct cpuidle_driver powernv_idle_driver = {
> .name = "powernv_idle",
> @@ -30,12 +31,14 @@ static int snooze_loop(struct cpuidle_device *dev,
> local_irq_enable();
> set_thread_flag(TIF_POLLING_NRFLAG);
>
> + ppc64_runlatch_off();
> while (!need_resched()) {
> HMT_low();
> HMT_very_low();
> }
>
> HMT_medium();
> + ppc64_runlatch_on();
> clear_thread_flag(TIF_POLLING_NRFLAG);
> smp_mb();
> return index;
> @@ -45,7 +48,9 @@ static int nap_loop(struct cpuidle_device *dev,
> struct cpuidle_driver *drv,
> int index)
> {
> + ppc64_runlatch_off();
> power7_idle();
> + ppc64_runlatch_on();
> return index;
> }
>
>
> On 02/06/2014 07:46 PM, Nicolas Pitre wrote:
>> The core idle loop now takes care of it.
>>
>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>> ---
>> arch/powerpc/platforms/powernv/setup.c | 13 +------------
>> 1 file changed, 1 insertion(+), 12 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
>> index 21166f65c9..a932feb290 100644
>> --- a/arch/powerpc/platforms/powernv/setup.c
>> +++ b/arch/powerpc/platforms/powernv/setup.c
>> @@ -26,7 +26,6 @@
>> #include <linux/of_fdt.h>
>> #include <linux/interrupt.h>
>> #include <linux/bug.h>
>> -#include <linux/cpuidle.h>
>>
>> #include <asm/machdep.h>
>> #include <asm/firmware.h>
>> @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
>> return 1;
>> }
>>
>> -void powernv_idle(void)
>> -{
>> - /* Hook to cpuidle framework if available, else
>> - * call on default platform idle code
>> - */
>> - if (cpuidle_idle_call()) {
>> - power7_idle();
>> - }
>> -}
>> -
>> define_machine(powernv) {
>> .name = "PowerNV",
>> .probe = pnv_probe,
>> @@ -236,7 +225,7 @@ define_machine(powernv) {
>> .show_cpuinfo = pnv_show_cpuinfo,
>> .progress = pnv_progress,
>> .machine_shutdown = pnv_shutdown,
>> - .power_save = powernv_idle,
>> + .power_save = power7_idle,
>> .calibrate_decr = generic_calibrate_decr,
>> #ifdef CONFIG_KEXEC
>> .kexec_cpu_down = pnv_kexec_cpu_down,
>>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()
From: Preeti U Murthy @ 2014-02-14 11:17 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Deepthi Dharwar, linaro-kernel, linux-pm, Peter Zijlstra,
Daniel Lezcano, Rafael J. Wysocki, linux-kernel, Ingo Molnar,
Thomas Gleixner, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1391696188-14540-1-git-send-email-nicolas.pitre@linaro.org>
Hi Nicolas,
You will have to include the below patch with yours. You
could squash the two I guess, I have added the changelog
just for clarity. And you also might want to change the subject to
cpuidle/powernv. It gives a better picture.
Thanks
Regards
Preeti U Murthy
cpuidle/powernv: Add ppc64_runlatch_off/on() to idle routines
Following moving of cpuidle_idle_call() to the generic idle loop, we need to
add the runlatch functions to the idle routines on powernv which was earlier
taken care of by the arch specific idle routine.
Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
---
drivers/cpuidle/cpuidle-powernv.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 78fd174..f48607c 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -14,6 +14,7 @@
#include <asm/machdep.h>
#include <asm/firmware.h>
+#include <asm/runlatch.h>
struct cpuidle_driver powernv_idle_driver = {
.name = "powernv_idle",
@@ -30,12 +31,14 @@ static int snooze_loop(struct cpuidle_device *dev,
local_irq_enable();
set_thread_flag(TIF_POLLING_NRFLAG);
+ ppc64_runlatch_off();
while (!need_resched()) {
HMT_low();
HMT_very_low();
}
HMT_medium();
+ ppc64_runlatch_on();
clear_thread_flag(TIF_POLLING_NRFLAG);
smp_mb();
return index;
@@ -45,7 +48,9 @@ static int nap_loop(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
{
+ ppc64_runlatch_off();
power7_idle();
+ ppc64_runlatch_on();
return index;
}
On 02/06/2014 07:46 PM, Nicolas Pitre wrote:
> The core idle loop now takes care of it.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/powerpc/platforms/powernv/setup.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
> index 21166f65c9..a932feb290 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -26,7 +26,6 @@
> #include <linux/of_fdt.h>
> #include <linux/interrupt.h>
> #include <linux/bug.h>
> -#include <linux/cpuidle.h>
>
> #include <asm/machdep.h>
> #include <asm/firmware.h>
> @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
> return 1;
> }
>
> -void powernv_idle(void)
> -{
> - /* Hook to cpuidle framework if available, else
> - * call on default platform idle code
> - */
> - if (cpuidle_idle_call()) {
> - power7_idle();
> - }
> -}
> -
> define_machine(powernv) {
> .name = "PowerNV",
> .probe = pnv_probe,
> @@ -236,7 +225,7 @@ define_machine(powernv) {
> .show_cpuinfo = pnv_show_cpuinfo,
> .progress = pnv_progress,
> .machine_shutdown = pnv_shutdown,
> - .power_save = powernv_idle,
> + .power_save = power7_idle,
> .calibrate_decr = generic_calibrate_decr,
> #ifdef CONFIG_KEXEC
> .kexec_cpu_down = pnv_kexec_cpu_down,
>
^ permalink raw reply related
* [PATCH v2 14/52] powerpc, sysfs: Fix CPU hotplug callback registration
From: Srivatsa S. Bhat @ 2014-02-14 7:52 UTC (permalink / raw)
To: paulus, oleg, mingo, rusty, peterz, tglx, akpm
Cc: linux-arch, ego, walken, linux, rjw, linux-kernel, Wang Dongsheng,
Olof Johansson, Madhavan Srinivasan, Paul Mackerras,
Srivatsa S. Bhat, tj, paulmck, linuxppc-dev, Ingo Molnar
In-Reply-To: <20140214074750.22701.47330.stgit@srivatsabhat.in.ibm.com>
Subsystems that want to register CPU hotplug callbacks, as well as perform
initialization for the CPUs that are already online, often do it as shown
below:
get_online_cpus();
for_each_online_cpu(cpu)
init_cpu(cpu);
register_cpu_notifier(&foobar_cpu_notifier);
put_online_cpus();
This is wrong, since it is prone to ABBA deadlocks involving the
cpu_add_remove_lock and the cpu_hotplug.lock (when running concurrently
with CPU hotplug operations).
Instead, the correct and race-free way of performing the callback
registration is:
cpu_notifier_register_begin();
for_each_online_cpu(cpu)
init_cpu(cpu);
/* Note the use of the double underscored version of the API */
__register_cpu_notifier(&foobar_cpu_notifier);
cpu_notifier_register_done();
Fix the sysfs code in powerpc by using this latter form of callback
registration.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Olof Johansson <olof@lixom.net>
Cc: Wang Dongsheng <dongsheng.wang@freescale.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Acked-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
arch/powerpc/kernel/sysfs.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 97e1dc9..d90d4b7 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -975,7 +975,8 @@ static int __init topology_init(void)
int cpu;
register_nodes();
- register_cpu_notifier(&sysfs_cpu_nb);
+
+ cpu_notifier_register_begin();
for_each_possible_cpu(cpu) {
struct cpu *c = &per_cpu(cpu_devices, cpu);
@@ -999,6 +1000,11 @@ static int __init topology_init(void)
if (cpu_online(cpu))
register_cpu_online(cpu);
}
+
+ __register_cpu_notifier(&sysfs_cpu_nb);
+
+ cpu_notifier_register_done();
+
#ifdef CONFIG_PPC64
sysfs_create_dscr_default();
#endif /* CONFIG_PPC64 */
^ permalink raw reply related
* Re: [PATCH 13/51] powerpc, sysfs: Fix CPU hotplug callback registration
From: Madhavan Srinivasan @ 2014-02-14 6:47 UTC (permalink / raw)
To: Srivatsa S. Bhat, paulus, oleg, rusty, peterz, tglx, akpm
Cc: ego, walken, linux, linux-kernel, Wang Dongsheng, Olof Johansson,
tj, paulmck, linuxppc-dev, mingo
In-Reply-To: <20140205220654.19080.11341.stgit@srivatsabhat.in.ibm.com>
On Thursday 06 February 2014 03:36 AM, Srivatsa S. Bhat wrote:
> Subsystems that want to register CPU hotplug callbacks, as well as perform
> initialization for the CPUs that are already online, often do it as shown
> below:
>
> get_online_cpus();
>
> for_each_online_cpu(cpu)
> init_cpu(cpu);
>
> register_cpu_notifier(&foobar_cpu_notifier);
>
> put_online_cpus();
>
> This is wrong, since it is prone to ABBA deadlocks involving the
> cpu_add_remove_lock and the cpu_hotplug.lock (when running concurrently
> with CPU hotplug operations).
>
> Instead, the correct and race-free way of performing the callback
> registration is:
>
> cpu_maps_update_begin();
>
> for_each_online_cpu(cpu)
> init_cpu(cpu);
>
> /* Note the use of the double underscored version of the API */
> __register_cpu_notifier(&foobar_cpu_notifier);
>
> cpu_maps_update_done();
>
>
> Fix the sysfs code in powerpc by using this latter form of callback
> registration.
Acked-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Wang Dongsheng <dongsheng.wang@freescale.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> ---
>
> arch/powerpc/kernel/sysfs.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 97e1dc9..c29ad44 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -975,7 +975,8 @@ static int __init topology_init(void)
> int cpu;
>
> register_nodes();
> - register_cpu_notifier(&sysfs_cpu_nb);
> +
> + cpu_maps_update_begin();
>
> for_each_possible_cpu(cpu) {
> struct cpu *c = &per_cpu(cpu_devices, cpu);
> @@ -999,6 +1000,11 @@ static int __init topology_init(void)
> if (cpu_online(cpu))
> register_cpu_online(cpu);
> }
> +
> + __register_cpu_notifier(&sysfs_cpu_nb);
> +
> + cpu_maps_update_done();
> +
> #ifdef CONFIG_PPC64
> sysfs_create_dscr_default();
> #endif /* CONFIG_PPC64 */
>
^ permalink raw reply
* [PATCH] powerpc/eeh: Prefetch PHB diag-data
From: Gavin Shan @ 2014-02-14 6:15 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
PHB diag-data is useful information to locate the root cause for
frozen PE. Unfortunately, we cleared part of that by wrongly zapping
LEM registers before collecting PHB diag-data. The patch fixes it
by prefetching that with extended eeh_ops->get_log() for PowerNV
platform.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 3 ++-
arch/powerpc/kernel/eeh.c | 2 +-
arch/powerpc/kernel/eeh_driver.c | 4 ++++
arch/powerpc/platforms/powernv/eeh-ioda.c | 11 +++++++----
arch/powerpc/platforms/powernv/eeh-powernv.c | 6 ++++--
arch/powerpc/platforms/powernv/pci.h | 2 +-
arch/powerpc/platforms/pseries/eeh_pseries.c | 10 ++++++++--
7 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index d4dd41f..b0bce0b 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -163,7 +163,8 @@ struct eeh_ops {
int (*get_state)(struct eeh_pe *pe, int *state);
int (*reset)(struct eeh_pe *pe, int option);
int (*wait_state)(struct eeh_pe *pe, int max_wait);
- int (*get_log)(struct eeh_pe *pe, int severity, char *drv_log, unsigned long len);
+ int (*get_log)(struct eeh_pe *pe, int severity,
+ char *drv_log, unsigned long len, bool prefetch);
int (*configure_bridge)(struct eeh_pe *pe);
int (*read_config)(struct device_node *dn, int where, int size, u32 *val);
int (*write_config)(struct device_node *dn, int where, int size, u32 val);
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index e7b76a6..d409d9d 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -257,7 +257,7 @@ void eeh_slot_error_detail(struct eeh_pe *pe, int severity)
}
}
- eeh_ops->get_log(pe, severity, pci_regs_buf, loglen);
+ eeh_ops->get_log(pe, severity, pci_regs_buf, loglen, false);
}
/**
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 7bb30dc..7a9123a 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -502,6 +502,10 @@ static void eeh_handle_normal_event(struct eeh_pe *pe)
pr_warning("EEH: This PCI device has failed %d times in the last hour\n",
pe->freeze_count);
+ /* Prefetch PHB diag-data if applicable */
+ if (eeh_ops->get_log)
+ eeh_ops->get_log(pe, EEH_LOG_TEMP, NULL, 0, true);
+
/* Walk the various device drivers attached to this slot through
* a reset sequence, giving each an opportunity to do what it needs
* to accomplish the reset. Each child gets a report of the
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index f514743..46fc394 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -539,7 +539,7 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option)
* The function is used to retrieve error log from P7IOC.
*/
static int ioda_eeh_get_log(struct eeh_pe *pe, int severity,
- char *drv_log, unsigned long len)
+ char *drv_log, unsigned long len, bool prefetch)
{
s64 ret;
unsigned long flags;
@@ -548,6 +548,12 @@ static int ioda_eeh_get_log(struct eeh_pe *pe, int severity,
spin_lock_irqsave(&phb->lock, flags);
+ if (!prefetch) {
+ pnv_pci_dump_phb_diag_data(hose, phb->diag.blob);
+ spin_unlock_irqrestore(&phb->lock, flags);
+ return 0;
+ }
+
ret = opal_pci_get_phb_diag_data2(phb->opal_id,
phb->diag.blob, PNV_PCI_DIAG_BUF_SIZE);
if (ret) {
@@ -557,9 +563,6 @@ static int ioda_eeh_get_log(struct eeh_pe *pe, int severity,
return -EIO;
}
- /* The PHB diag-data is always indicative */
- pnv_pci_dump_phb_diag_data(hose, phb->diag.blob);
-
spin_unlock_irqrestore(&phb->lock, flags);
return 0;
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index a59788e..df1b73f 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -290,14 +290,16 @@ static int powernv_eeh_wait_state(struct eeh_pe *pe, int max_wait)
* Retrieve the temporary or permanent error from the PE.
*/
static int powernv_eeh_get_log(struct eeh_pe *pe, int severity,
- char *drv_log, unsigned long len)
+ char *drv_log, unsigned long len,
+ bool prefetch)
{
struct pci_controller *hose = pe->phb;
struct pnv_phb *phb = hose->private_data;
int ret = -EEXIST;
if (phb->eeh_ops && phb->eeh_ops->get_log)
- ret = phb->eeh_ops->get_log(pe, severity, drv_log, len);
+ ret = phb->eeh_ops->get_log(pe, severity,
+ drv_log, len, prefetch);
return ret;
}
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 13f1942..f1a9c2a 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -75,7 +75,7 @@ struct pnv_eeh_ops {
int (*get_state)(struct eeh_pe *pe);
int (*reset)(struct eeh_pe *pe, int option);
int (*get_log)(struct eeh_pe *pe, int severity,
- char *drv_log, unsigned long len);
+ char *drv_log, unsigned long len, bool prefetch);
int (*configure_bridge)(struct eeh_pe *pe);
int (*next_error)(struct eeh_pe **pe);
};
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 8a8f047..d38e1ba 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -576,11 +576,17 @@ static int pseries_eeh_wait_state(struct eeh_pe *pe, int max_wait)
* Actually, the error will be retrieved through the dedicated
* RTAS call.
*/
-static int pseries_eeh_get_log(struct eeh_pe *pe, int severity, char *drv_log, unsigned long len)
+static int pseries_eeh_get_log(struct eeh_pe *pe, int severity,
+ char *drv_log, unsigned long len,
+ bool prefetch)
{
int config_addr;
unsigned long flags;
- int ret;
+ int ret = 0;
+
+ /* We needn't do prefetch stuff */
+ if (prefetch)
+ return ret;
spin_lock_irqsave(&slot_errbuf_lock, flags);
memset(slot_errbuf, 0, eeh_error_buf_size);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH V2 0/3] powerpc: Fix random application crashes with NUMA_BALANCING enabled
From: Benjamin Herrenschmidt @ 2014-02-14 5:41 UTC (permalink / raw)
To: Andrew Morton
Cc: riel, linux-mm, paulus, Aneesh Kumar K.V, linuxppc-dev, mgorman
In-Reply-To: <20140213150639.2b9124797ac4975b6119f6f0@linux-foundation.org>
On Thu, 2014-02-13 at 15:06 -0800, Andrew Morton wrote:
> On Wed, 12 Feb 2014 09:13:35 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
> > Hello,
> >
> > This patch series fix random application crashes observed on ppc64 with numa
> > balancing enabled. Without the patch we see crashes like
> >
> > anacron[14551]: unhandled signal 11 at 0000000000000041 nip 000000003cfd54b4 lr 000000003cfd5464 code 30001
> > anacron[14599]: unhandled signal 11 at 0000000000000041 nip 000000003efc54b4 lr 000000003efc5464 code 30001
> >
>
> Random application crashes are bad. Which kernel version(s) do you think
> need fixing here?
>
> I grabbed the patches but would like to hear from Ben (or something
> approximating him) before doing anything with them, please.
Ah good. Did you grab v2 ? v1 had a compile breakage. I was about to
send them to Linus today as well but then got distracted by a sick
child, so I'm happy for you to pick them up and send them to the
boss :-)
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH V2 0/3] powerpc: Fix random application crashes with NUMA_BALANCING enabled
From: Aneesh Kumar K.V @ 2014-02-14 5:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: riel, linux-mm, paulus, mgorman, linuxppc-dev
In-Reply-To: <20140213150639.2b9124797ac4975b6119f6f0@linux-foundation.org>
Andrew Morton <akpm@linux-foundation.org> writes:
> On Wed, 12 Feb 2014 09:13:35 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> Hello,
>>
>> This patch series fix random application crashes observed on ppc64 with numa
>> balancing enabled. Without the patch we see crashes like
>>
>> anacron[14551]: unhandled signal 11 at 0000000000000041 nip 000000003cfd54b4 lr 000000003cfd5464 code 30001
>> anacron[14599]: unhandled signal 11 at 0000000000000041 nip 000000003efc54b4 lr 000000003efc5464 code 30001
>>
>
> Random application crashes are bad. Which kernel version(s) do you think
> need fixing here?
>
> I grabbed the patches but would like to hear from Ben (or something
> approximating him) before doing anything with them, please.
>
Considering this impact only ppc64 and also only when numa balancing is enabled, we
only need to send this upstream. (no need to backport to any other
kernel versions)
We merged numa balancing support for ppc64
(c34a51ce49b40b9667cd7f5cc2e40475af8b4c3d) only in this merge window.
$git describe --contains c34a51ce49b40b9667cd7f5cc2e40475af8b4c3d
v3.14-rc1~80^2~35
-aneesh
^ permalink raw reply
* [PATCH 2/2] of: search the best compatible match first in __of_match_node()
From: Kevin Hao @ 2014-02-14 5:22 UTC (permalink / raw)
To: devicetree, linuxppc-dev
Cc: Grant Likely, Stephen N Chivers, Rob Herring, Kevin Hao,
Sebastian Hesselbarth
In-Reply-To: <1392355366-1445-1-git-send-email-haokexin@gmail.com>
Currently, of_match_node compares each given match against all node's
compatible strings with of_device_is_compatible.
To achieve multiple compatible strings per node with ordering from
specific to generic, this requires given matches to be ordered from
specific to generic. For most of the drivers this is not true and also
an alphabetical ordering is more sane there.
Therefore, this patch introduces a function to match each of the node's
compatible strings against all given compatible matches without type and
name first, before checking the next compatible string. This implies
that node's compatibles are ordered from specific to generic while
given matches can be in any order. If we fail to find such a match
entry, then fall-back to the old method in order to keep compatibility.
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
drivers/of/base.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ba195fbce4c6..10b51106c854 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -730,13 +730,49 @@ out:
}
EXPORT_SYMBOL(of_find_node_with_property);
+static const struct of_device_id *
+of_match_compatible(const struct of_device_id *matches,
+ const struct device_node *node)
+{
+ const char *cp;
+ int cplen, l;
+ const struct of_device_id *m;
+
+ cp = __of_get_property(node, "compatible", &cplen);
+ while (cp && (cplen > 0)) {
+ m = matches;
+ while (m->name[0] || m->type[0] || m->compatible[0]) {
+ /* Only match for the entries without type and name */
+ if (m->name[0] || m->type[0] ||
+ of_compat_cmp(m->compatible, cp,
+ strlen(m->compatible)))
+ m++;
+ else
+ return m;
+ }
+
+ /* Get node's next compatible string */
+ l = strlen(cp) + 1;
+ cp += l;
+ cplen -= l;
+ }
+
+ return NULL;
+}
+
static
const struct of_device_id *__of_match_node(const struct of_device_id *matches,
const struct device_node *node)
{
+ const struct of_device_id *m;
+
if (!matches)
return NULL;
+ m = of_match_compatible(matches, node);
+ if (m)
+ return m;
+
while (matches->name[0] || matches->type[0] || matches->compatible[0]) {
int match = 1;
if (matches->name[0])
@@ -760,7 +796,12 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
* @matches: array of of device match structures to search in
* @node: the of device structure to match against
*
- * Low level utility function used by device matching.
+ * Low level utility function used by device matching. We have two ways
+ * of matching:
+ * - Try to find the best compatible match by comparing each compatible
+ * string of device node with all the given matches respectively.
+ * - If the above method failed, then try to match the compatible by using
+ * __of_device_is_compatible() besides the match in type and name.
*/
const struct of_device_id *of_match_node(const struct of_device_id *matches,
const struct device_node *node)
--
1.8.5.3
^ permalink raw reply related
* [PATCH 1/2] Revert "OF: base: match each node compatible against all given matches first"
From: Kevin Hao @ 2014-02-14 5:22 UTC (permalink / raw)
To: devicetree, linuxppc-dev
Cc: Grant Likely, Stephen N Chivers, Rob Herring, Kevin Hao,
Sebastian Hesselbarth
In-Reply-To: <1392355366-1445-1-git-send-email-haokexin@gmail.com>
This reverts commit 105353145eafb3ea919f5cdeb652a9d8f270228e.
Stephen Chivers reported this is broken as we will get a match
entry '.type = "serial"' instead of the '.compatible = "ns16550"'
in the following scenario:
serial0: serial@4500 {
compatible = "fsl,ns16550", "ns16550";
}
struct of_device_id of_platform_serial_table[] = {
{ .compatible = "ns8250", .data = (void *)PORT_8250, },
{ .compatible = "ns16450", .data = (void *)PORT_16450, },
{ .compatible = "ns16550a", .data = (void *)PORT_16550A, },
{ .compatible = "ns16550", .data = (void *)PORT_16550, },
{ .compatible = "ns16750", .data = (void *)PORT_16750, },
{ .compatible = "ns16850", .data = (void *)PORT_16850, },
...
{ .type = "serial", .data = (void *)PORT_UNKNOWN, },
{ /* end of list */ },
};
So just revert this patch, we will use another implementation to find
the best compatible match in a follow-on patch.
Reported-by: Stephen N Chivers <schivers@csc.com.au>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
drivers/of/base.c | 53 ++++++++++++++++-------------------------------------
1 file changed, 16 insertions(+), 37 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ff85450d5683..ba195fbce4c6 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -734,42 +734,24 @@ static
const struct of_device_id *__of_match_node(const struct of_device_id *matches,
const struct device_node *node)
{
- const char *cp;
- int cplen, l;
-
if (!matches)
return NULL;
- cp = __of_get_property(node, "compatible", &cplen);
- do {
- const struct of_device_id *m = matches;
-
- /* Check against matches with current compatible string */
- while (m->name[0] || m->type[0] || m->compatible[0]) {
- int match = 1;
- if (m->name[0])
- match &= node->name
- && !strcmp(m->name, node->name);
- if (m->type[0])
- match &= node->type
- && !strcmp(m->type, node->type);
- if (m->compatible[0])
- match &= cp
- && !of_compat_cmp(m->compatible, cp,
- strlen(m->compatible));
- if (match)
- return m;
- m++;
- }
-
- /* Get node's next compatible string */
- if (cp) {
- l = strlen(cp) + 1;
- cp += l;
- cplen -= l;
- }
- } while (cp && (cplen > 0));
-
+ while (matches->name[0] || matches->type[0] || matches->compatible[0]) {
+ int match = 1;
+ if (matches->name[0])
+ match &= node->name
+ && !strcmp(matches->name, node->name);
+ if (matches->type[0])
+ match &= node->type
+ && !strcmp(matches->type, node->type);
+ if (matches->compatible[0])
+ match &= __of_device_is_compatible(node,
+ matches->compatible);
+ if (match)
+ return matches;
+ matches++;
+ }
return NULL;
}
@@ -778,10 +760,7 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
* @matches: array of of device match structures to search in
* @node: the of device structure to match against
*
- * Low level utility function used by device matching. Matching order
- * is to compare each of the node's compatibles with all given matches
- * first. This implies node's compatible is sorted from specific to
- * generic while matches can be in any order.
+ * Low level utility function used by device matching.
*/
const struct of_device_id *of_match_node(const struct of_device_id *matches,
const struct device_node *node)
--
1.8.5.3
^ permalink raw reply related
* [PATCH 0/2] of: fix a regression when trying to find the best compatible match
From: Kevin Hao @ 2014-02-14 5:22 UTC (permalink / raw)
To: devicetree, linuxppc-dev
Cc: Grant Likely, Stephen N Chivers, Rob Herring, Kevin Hao,
Sebastian Hesselbarth
Hi,
This fix a regression when trying to find the best compatible match. You can
find the detail of the issue reported by Stephen Chivers at:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-February/115278.html
I have made a patch to fix this previously.
http://patchwork.ozlabs.org/patch/319624/
This is another respin with the implementation suggested by Rob Herring.
Kevin Hao (2):
Revert "OF: base: match each node compatible against all given matches
first"
of: search the best compatible match first in __of_match_node()
drivers/of/base.c | 88 ++++++++++++++++++++++++++++++++++---------------------
1 file changed, 54 insertions(+), 34 deletions(-)
--
1.8.5.3
^ permalink raw reply
* Re: [PATCH 1/6] PCI, acpiphp: Use list_for_each_entry() for bus traversal
From: Yijing Wang @ 2014-02-14 2:19 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Russell King, David Airlie, linux-pcmcia, Hanjun Guo, dri-devel,
linux-pci, Bjorn Helgaas, linuxppc-dev
In-Reply-To: <2601738.3Ek05F9oo5@vostro.rjw.lan>
On 2014/2/14 7:54, Rafael J. Wysocki wrote:
> On Thursday, February 13, 2014 09:13:58 PM Yijing Wang wrote:
>> Replace list_for_each() + pci_bus_b() with the simpler
>> list_for_each_entry().
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>
> Looks reasonable to me.
>
> Does it conflict with anything currently in linux-next (the linux-next branch
> of linux-pm.git in particular)?
Hi Rafael,
I applied this to your linux-next branch successfully . No conflicts found.
Thanks!
Yijing.
>
>> ---
>> drivers/pci/hotplug/acpiphp_glue.c | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
>> index cd929ae..aee6a0a 100644
>> --- a/drivers/pci/hotplug/acpiphp_glue.c
>> +++ b/drivers/pci/hotplug/acpiphp_glue.c
>> @@ -450,7 +450,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
>> */
>> static unsigned char acpiphp_max_busnr(struct pci_bus *bus)
>> {
>> - struct list_head *tmp;
>> + struct pci_bus *tmp;
>> unsigned char max, n;
>>
>> /*
>> @@ -463,8 +463,8 @@ static unsigned char acpiphp_max_busnr(struct pci_bus *bus)
>> */
>> max = bus->busn_res.start;
>>
>> - list_for_each(tmp, &bus->children) {
>> - n = pci_bus_max_busnr(pci_bus_b(tmp));
>> + list_for_each_entry(tmp, &bus->children, node) {
>> + n = pci_bus_max_busnr(tmp);
>> if (n > max)
>> max = n;
>> }
>>
>
--
Thanks!
Yijing
^ permalink raw reply
* Re: [PATCH] of: give priority to the compatible match in __of_match_node()
From: Kevin Hao @ 2014-02-14 1:20 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree@vger.kernel.org, Arnd Bergmann, Chris Proctor,
Stephen N Chivers, Grant Likely, Rob Herring, Scott Wood,
linuxppc-dev, Sebastian Hesselbarth
In-Reply-To: <CAL_JsqKMi2H=vwoxrOt8QRA2xJeiLqBKKfLtt4QRCRoFk6JUHg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]
On Thu, Feb 13, 2014 at 01:01:42PM -0600, Rob Herring wrote:
> On Wed, Feb 12, 2014 at 5:38 AM, Kevin Hao <haokexin@gmail.com> wrote:
> > When the device node do have a compatible property, we definitely
> > prefer the compatible match besides the type and name. Only if
> > there is no such a match, we then consider the candidate which
> > doesn't have compatible entry but do match the type or name with
> > the device node.
> >
> > This is based on a patch from Sebastian Hesselbarth.
> > http://patchwork.ozlabs.org/patch/319434/
> >
> > I did some code refactoring and also fixed a bug in the original patch.
>
> I'm inclined to just revert this once again and avoid possibly
> breaking yet another platform.
>
> However, I think I would like to see this structured differently. We
> basically have 2 ways of matching: the existing pre-3.14 way and the
> desired match on best compatible only. All new bindings should match
> with the new way and the old way needs to be kept for compatibility.
> So lets structure the code that way. Search the match table first for
> best compatible with name and type NULL, then search the table the old
> way. I realize it appears you are doing this, but it is not clear this
> is the intent of the code. I would like to see this written as a patch
> with commit 105353145eafb3ea919 reverted first and you add a new match
> function to call first and then fallback to the existing function.
OK, I will git it a try.
Thanks,
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: enable CONFIG_HAVE_MEMORYLESS_NODES
From: Nishanth Aravamudan @ 2014-02-14 0:11 UTC (permalink / raw)
To: David Rientjes
Cc: Pekka Enberg, linux-mm, Paul Mackerras, Anton Blanchard,
Matt Mackall, Joonsoo Kim, linuxppc-dev, Christoph Lameter,
Wanpeng Li
In-Reply-To: <alpine.DEB.2.02.1402131444440.13899@chino.kir.corp.google.com>
On 13.02.2014 [14:45:49 -0800], David Rientjes wrote:
> On Thu, 13 Feb 2014, Nishanth Aravamudan wrote:
>
> > > Anton Blanchard found an issue with an LPAR that had no memory in Node
> > > 0. Christoph Lameter recommended, as one possible solution, to use
> > > numa_mem_id() for locality of the nearest memory node-wise. However,
> > > numa_mem_id() [and the other related APIs] are only useful if
> > > CONFIG_HAVE_MEMORYLESS_NODES is set. This is only the case for ia64
> > > currently, but clearly we can have memoryless nodes on ppc64. Add the
> > > Kconfig option and define it to be the same value as CONFIG_NUMA.
> > >
> > > On the LPAR in question, which was very inefficiently using slabs, this
> > > took the slab consumption at boot from roughly 7GB to roughly 4GB.
> >
> > Err, this should have been
> >
> > Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> >
> > !
> >
> > Sorry about that Ben!
> >
> > > ---
> > > Ben, the only question I have wrt this change is if it's appropriate to
> > > change it for all powerpc configs (that have NUMA on)?
> > >
>
> I'm suspecting that Ben will request that the proper set_numa_mem() calls
> are done for ppc init to make this actually do anything other than return
> numa_mem_id() == numa_node_id().
You're right, thanks for pointing this out. I could have sworn that in
my previous debugging I saw proper NUMA information, but perhaps it was
just correct based upon the system configuration.
> > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > > index 25493a0..bb2d5fe 100644
> > > --- a/arch/powerpc/Kconfig
> > > +++ b/arch/powerpc/Kconfig
> > > @@ -447,6 +447,9 @@ config NODES_SHIFT
> > > default "4"
> > > depends on NEED_MULTIPLE_NODES
> > >
> > > +config HAVE_MEMORYLESS_NODES
> > > + def_bool NUMA
> > > +
> > > config ARCH_SELECT_MEMORY_MODEL
> > > def_bool y
> > > depends on PPC64
>
^ permalink raw reply
* Re: [PATCH 1/6] PCI, acpiphp: Use list_for_each_entry() for bus traversal
From: Rafael J. Wysocki @ 2014-02-13 23:54 UTC (permalink / raw)
To: Yijing Wang
Cc: Russell King, David Airlie, linux-pcmcia, Hanjun Guo, dri-devel,
linux-pci, Bjorn Helgaas, linuxppc-dev
In-Reply-To: <1392297243-61848-1-git-send-email-wangyijing@huawei.com>
On Thursday, February 13, 2014 09:13:58 PM Yijing Wang wrote:
> Replace list_for_each() + pci_bus_b() with the simpler
> list_for_each_entry().
>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Looks reasonable to me.
Does it conflict with anything currently in linux-next (the linux-next branch
of linux-pm.git in particular)?
> ---
> drivers/pci/hotplug/acpiphp_glue.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index cd929ae..aee6a0a 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -450,7 +450,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
> */
> static unsigned char acpiphp_max_busnr(struct pci_bus *bus)
> {
> - struct list_head *tmp;
> + struct pci_bus *tmp;
> unsigned char max, n;
>
> /*
> @@ -463,8 +463,8 @@ static unsigned char acpiphp_max_busnr(struct pci_bus *bus)
> */
> max = bus->busn_res.start;
>
> - list_for_each(tmp, &bus->children) {
> - n = pci_bus_max_busnr(pci_bus_b(tmp));
> + list_for_each_entry(tmp, &bus->children, node) {
> + n = pci_bus_max_busnr(tmp);
> if (n > max)
> max = n;
> }
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH] mm: numa: bugfix for LAST_CPUPID_NOT_IN_PAGE_FLAGS
From: Andrew Morton @ 2014-02-13 23:20 UTC (permalink / raw)
To: Liu Ping Fan
Cc: Peter Zijlstra, linux-mm, Paul Mackerras, Aneesh Kumar K.V,
linuxppc-dev
In-Reply-To: <1391563546-26052-1-git-send-email-pingfank@linux.vnet.ibm.com>
On Wed, 5 Feb 2014 09:25:46 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:
> When doing some numa tests on powerpc, I triggered an oops bug. I find
> it is caused by using page->_last_cpupid. It should be initialized as
> "-1 & LAST_CPUPID_MASK", but not "-1". Otherwise, in task_numa_fault(),
> we will miss the checking (last_cpupid == (-1 & LAST_CPUPID_MASK)).
> And finally cause an oops bug in task_numa_group(), since the online cpu is
> less than possible cpu.
I grabbed this. I added this to the changelog:
: PPC needs the LAST_CPUPID_NOT_IN_PAGE_FLAGS case because ppc needs to
: support a large physical address region, up to 2^46 but small section size
: (2^24). So when NR_CPUS grows up, it is easily to cause
: not-in-page-flags.
to hopefully address Peter's observation.
How should we proceed with this? I'm getting the impression that numa
balancing on ppc is a dead duck in 3.14, so perhaps this and
powerpc-mm-add-new-set-flag-argument-to-pte-pmd-update-function.patch
mm-dirty-accountable-change-only-apply-to-non-prot-numa-case.patch
mm-use-ptep-pmdp_set_numa-for-updating-_page_numa-bit.patch
are 3.15-rc1 material?
^ permalink raw reply
* Re: [PATCH V2 0/3] powerpc: Fix random application crashes with NUMA_BALANCING enabled
From: Andrew Morton @ 2014-02-13 23:06 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: riel, linux-mm, paulus, mgorman, linuxppc-dev
In-Reply-To: <1392176618-23667-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On Wed, 12 Feb 2014 09:13:35 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> Hello,
>
> This patch series fix random application crashes observed on ppc64 with numa
> balancing enabled. Without the patch we see crashes like
>
> anacron[14551]: unhandled signal 11 at 0000000000000041 nip 000000003cfd54b4 lr 000000003cfd5464 code 30001
> anacron[14599]: unhandled signal 11 at 0000000000000041 nip 000000003efc54b4 lr 000000003efc5464 code 30001
>
Random application crashes are bad. Which kernel version(s) do you think
need fixing here?
I grabbed the patches but would like to hear from Ben (or something
approximating him) before doing anything with them, please.
^ permalink raw reply
* Re: [PATCH] powerpc: enable CONFIG_HAVE_MEMORYLESS_NODES
From: David Rientjes @ 2014-02-13 22:45 UTC (permalink / raw)
To: Nishanth Aravamudan
Cc: Pekka Enberg, linux-mm, Paul Mackerras, Anton Blanchard,
Matt Mackall, Joonsoo Kim, linuxppc-dev, Christoph Lameter,
Wanpeng Li
In-Reply-To: <20140213214131.GB12409@linux.vnet.ibm.com>
On Thu, 13 Feb 2014, Nishanth Aravamudan wrote:
> > Anton Blanchard found an issue with an LPAR that had no memory in Node
> > 0. Christoph Lameter recommended, as one possible solution, to use
> > numa_mem_id() for locality of the nearest memory node-wise. However,
> > numa_mem_id() [and the other related APIs] are only useful if
> > CONFIG_HAVE_MEMORYLESS_NODES is set. This is only the case for ia64
> > currently, but clearly we can have memoryless nodes on ppc64. Add the
> > Kconfig option and define it to be the same value as CONFIG_NUMA.
> >
> > On the LPAR in question, which was very inefficiently using slabs, this
> > took the slab consumption at boot from roughly 7GB to roughly 4GB.
>
> Err, this should have been
>
> Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
>
> !
>
> Sorry about that Ben!
>
> > ---
> > Ben, the only question I have wrt this change is if it's appropriate to
> > change it for all powerpc configs (that have NUMA on)?
> >
I'm suspecting that Ben will request that the proper set_numa_mem() calls
are done for ppc init to make this actually do anything other than return
numa_mem_id() == numa_node_id().
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 25493a0..bb2d5fe 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -447,6 +447,9 @@ config NODES_SHIFT
> > default "4"
> > depends on NEED_MULTIPLE_NODES
> >
> > +config HAVE_MEMORYLESS_NODES
> > + def_bool NUMA
> > +
> > config ARCH_SELECT_MEMORY_MODEL
> > def_bool y
> > depends on PPC64
^ permalink raw reply
* Re: [PATCH] powerpc: enable CONFIG_HAVE_MEMORYLESS_NODES
From: Nishanth Aravamudan @ 2014-02-13 21:41 UTC (permalink / raw)
To: Christoph Lameter
Cc: David Rientjes, Pekka Enberg, linux-mm, Paul Mackerras,
Anton Blanchard, Matt Mackall, Joonsoo Kim, linuxppc-dev,
Wanpeng Li
In-Reply-To: <20140128183457.GA9315@linux.vnet.ibm.com>
On 28.01.2014 [10:34:57 -0800], Nishanth Aravamudan wrote:
> Anton Blanchard found an issue with an LPAR that had no memory in Node
> 0. Christoph Lameter recommended, as one possible solution, to use
> numa_mem_id() for locality of the nearest memory node-wise. However,
> numa_mem_id() [and the other related APIs] are only useful if
> CONFIG_HAVE_MEMORYLESS_NODES is set. This is only the case for ia64
> currently, but clearly we can have memoryless nodes on ppc64. Add the
> Kconfig option and define it to be the same value as CONFIG_NUMA.
>
> On the LPAR in question, which was very inefficiently using slabs, this
> took the slab consumption at boot from roughly 7GB to roughly 4GB.
Err, this should have been
Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
!
Sorry about that Ben!
> ---
> Ben, the only question I have wrt this change is if it's appropriate to
> change it for all powerpc configs (that have NUMA on)?
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 25493a0..bb2d5fe 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -447,6 +447,9 @@ config NODES_SHIFT
> default "4"
> depends on NEED_MULTIPLE_NODES
>
> +config HAVE_MEMORYLESS_NODES
> + def_bool NUMA
> +
> config ARCH_SELECT_MEMORY_MODEL
> def_bool y
> depends on PPC64
^ permalink raw reply
* Re: [PATCH] of: give priority to the compatible match in __of_match_node()
From: Rob Herring @ 2014-02-13 19:01 UTC (permalink / raw)
To: Kevin Hao
Cc: devicetree@vger.kernel.org, Arnd Bergmann, Chris Proctor,
Stephen N Chivers, Grant Likely, Rob Herring, Scott Wood,
linuxppc-dev, Sebastian Hesselbarth
In-Reply-To: <1392205084-2351-1-git-send-email-haokexin@gmail.com>
On Wed, Feb 12, 2014 at 5:38 AM, Kevin Hao <haokexin@gmail.com> wrote:
> When the device node do have a compatible property, we definitely
> prefer the compatible match besides the type and name. Only if
> there is no such a match, we then consider the candidate which
> doesn't have compatible entry but do match the type or name with
> the device node.
>
> This is based on a patch from Sebastian Hesselbarth.
> http://patchwork.ozlabs.org/patch/319434/
>
> I did some code refactoring and also fixed a bug in the original patch.
I'm inclined to just revert this once again and avoid possibly
breaking yet another platform.
However, I think I would like to see this structured differently. We
basically have 2 ways of matching: the existing pre-3.14 way and the
desired match on best compatible only. All new bindings should match
with the new way and the old way needs to be kept for compatibility.
So lets structure the code that way. Search the match table first for
best compatible with name and type NULL, then search the table the old
way. I realize it appears you are doing this, but it is not clear this
is the intent of the code. I would like to see this written as a patch
with commit 105353145eafb3ea919 reverted first and you add a new match
function to call first and then fallback to the existing function.
Rob
>
> Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Signed-off-by: Kevin Hao <haokexin@gmail.com>
> ---
> drivers/of/base.c | 55 +++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 37 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ff85450d5683..9d655df458bd 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -730,32 +730,45 @@ out:
> }
> EXPORT_SYMBOL(of_find_node_with_property);
>
> +static int of_match_type_or_name(const struct device_node *node,
> + const struct of_device_id *m)
> +{
> + int match = 1;
> +
> + if (m->name[0])
> + match &= node->name && !strcmp(m->name, node->name);
> +
> + if (m->type[0])
> + match &= node->type && !strcmp(m->type, node->type);
> +
> + return match;
> +}
> +
> static
> const struct of_device_id *__of_match_node(const struct of_device_id *matches,
> const struct device_node *node)
> {
> const char *cp;
> int cplen, l;
> + const struct of_device_id *m;
> + int match;
>
> if (!matches)
> return NULL;
>
> cp = __of_get_property(node, "compatible", &cplen);
> - do {
> - const struct of_device_id *m = matches;
> + while (cp && (cplen > 0)) {
> + m = matches;
>
> /* Check against matches with current compatible string */
> while (m->name[0] || m->type[0] || m->compatible[0]) {
> - int match = 1;
> - if (m->name[0])
> - match &= node->name
> - && !strcmp(m->name, node->name);
> - if (m->type[0])
> - match &= node->type
> - && !strcmp(m->type, node->type);
> - if (m->compatible[0])
> - match &= cp
> - && !of_compat_cmp(m->compatible, cp,
> + if (!m->compatible[0]) {
> + m++;
> + continue;
> + }
> +
> + match = of_match_type_or_name(node, m);
> + match &= cp && !of_compat_cmp(m->compatible, cp,
> strlen(m->compatible));
> if (match)
> return m;
> @@ -763,12 +776,18 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
> }
>
> /* Get node's next compatible string */
> - if (cp) {
> - l = strlen(cp) + 1;
> - cp += l;
> - cplen -= l;
> - }
> - } while (cp && (cplen > 0));
> + l = strlen(cp) + 1;
> + cp += l;
> + cplen -= l;
> + }
> +
> + m = matches;
> + /* Check against matches without compatible string */
> + while (m->name[0] || m->type[0] || m->compatible[0]) {
> + if (!m->compatible[0] && of_match_type_or_name(node, m))
> + return m;
> + m++;
> + }
>
> return NULL;
> }
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/6] PCI/arm: Use list_for_each_entry() for bus traversal
From: Russell King - ARM Linux @ 2014-02-13 14:46 UTC (permalink / raw)
To: Yijing Wang
Cc: David Airlie, linux-pcmcia, Hanjun Guo, dri-devel, linux-pci,
Bjorn Helgaas, linuxppc-dev
In-Reply-To: <1392297243-61848-2-git-send-email-wangyijing@huawei.com>
On Thu, Feb 13, 2014 at 09:13:59PM +0800, Yijing Wang wrote:
> Replace list_for_each() + pci_bus_b() with the simpler
> list_for_each_entry().
>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
--
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
* [PATCH 2/6] PCI/arm: Use list_for_each_entry() for bus traversal
From: Yijing Wang @ 2014-02-13 13:13 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Russell King, David Airlie, linux-pcmcia, Hanjun Guo, dri-devel,
linux-pci, Yijing Wang, linuxppc-dev
In-Reply-To: <1392297243-61848-1-git-send-email-wangyijing@huawei.com>
Replace list_for_each() + pci_bus_b() with the simpler
list_for_each_entry().
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
arch/arm/kernel/bios32.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 317da88..0a77858 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -57,13 +57,10 @@ static void pcibios_bus_report_status(struct pci_bus *bus, u_int status_mask, in
void pcibios_report_status(u_int status_mask, int warn)
{
- struct list_head *l;
-
- list_for_each(l, &pci_root_buses) {
- struct pci_bus *bus = pci_bus_b(l);
+ struct pci_bus *bus;
+ list_for_each_entry(bus, &pci_root_buses, node)
pcibios_bus_report_status(bus, status_mask, warn);
- }
}
/*
--
1.7.1
^ permalink raw reply related
* [PATCH 1/6] PCI,acpiphp: Use list_for_each_entry() for bus traversal
From: Yijing Wang @ 2014-02-13 13:13 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Russell King, David Airlie, linux-pcmcia, Hanjun Guo, dri-devel,
linux-pci, Yijing Wang, linuxppc-dev
Replace list_for_each() + pci_bus_b() with the simpler
list_for_each_entry().
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
drivers/pci/hotplug/acpiphp_glue.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index cd929ae..aee6a0a 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -450,7 +450,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
*/
static unsigned char acpiphp_max_busnr(struct pci_bus *bus)
{
- struct list_head *tmp;
+ struct pci_bus *tmp;
unsigned char max, n;
/*
@@ -463,8 +463,8 @@ static unsigned char acpiphp_max_busnr(struct pci_bus *bus)
*/
max = bus->busn_res.start;
- list_for_each(tmp, &bus->children) {
- n = pci_bus_max_busnr(pci_bus_b(tmp));
+ list_for_each_entry(tmp, &bus->children, node) {
+ n = pci_bus_max_busnr(tmp);
if (n > max)
max = n;
}
--
1.7.1
^ permalink raw reply related
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