* [PATCH V4 04/10] x86, perf: Add conditional branch filtering support
From: Anshuman Khandual @ 2013-12-04 10:32 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: mikey, ak, eranian, michael, acme, sukadev, mingo
In-Reply-To: <1386153162-11225-1-git-send-email-khandual@linux.vnet.ibm.com>
This patch adds conditional branch filtering support,
enabling it for PERF_SAMPLE_BRANCH_COND in perf branch
stack sampling framework by utilizing an available
software filter X86_BR_JCC.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Reviewed-by: Stephane Eranian <eranian@google.com>
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
index d82d155..9dd2459 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -384,6 +384,9 @@ static void intel_pmu_setup_sw_lbr_filter(struct perf_event *event)
if (br_type & PERF_SAMPLE_BRANCH_NO_TX)
mask |= X86_BR_NO_TX;
+ if (br_type & PERF_SAMPLE_BRANCH_COND)
+ mask |= X86_BR_JCC;
+
/*
* stash actual user request into reg, it may
* be used by fixup code for some CPU
@@ -678,6 +681,7 @@ static const int nhm_lbr_sel_map[PERF_SAMPLE_BRANCH_MAX] = {
* NHM/WSM erratum: must include IND_JMP to capture IND_CALL
*/
[PERF_SAMPLE_BRANCH_IND_CALL] = LBR_IND_CALL | LBR_IND_JMP,
+ [PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
};
static const int snb_lbr_sel_map[PERF_SAMPLE_BRANCH_MAX] = {
@@ -689,6 +693,7 @@ static const int snb_lbr_sel_map[PERF_SAMPLE_BRANCH_MAX] = {
[PERF_SAMPLE_BRANCH_ANY_CALL] = LBR_REL_CALL | LBR_IND_CALL
| LBR_FAR,
[PERF_SAMPLE_BRANCH_IND_CALL] = LBR_IND_CALL,
+ [PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
};
/* core */
--
1.7.11.7
^ permalink raw reply related
* [PATCH V4 09/10] power8, perf: Change BHRB branch filter configuration
From: Anshuman Khandual @ 2013-12-04 10:32 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: mikey, ak, eranian, michael, acme, sukadev, mingo
In-Reply-To: <1386153162-11225-1-git-send-email-khandual@linux.vnet.ibm.com>
Powerpc kernel now supports SW based branch filters for book3s systems with some
specifc requirements while dealing with HW supported branch filters in order to
achieve overall OR semantics prevailing in perf branch stack sampling framework.
This patch adapts the BHRB branch filter configuration to meet those protocols.
POWER8 PMU does support 3 branch filters (out of which two are getting used in
perf branch stack) which are mutually exclussive and cannot be ORed with each
other. This implies that PMU can only handle one HW based branch filter request
at any point of time. For all other combinations PMU will pass it on to the SW.
Also the combination of PERF_SAMPLE_BRANCH_ANY_CALL and PERF_SAMPLE_BRANCH_COND
can now be handled in SW, hence we dont error them out anymore.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
arch/powerpc/perf/power8-pmu.c | 73 +++++++++++++++++++++++++++++++-----------
1 file changed, 54 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index 03c5b8d..6021349 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -561,7 +561,56 @@ static int power8_generic_events[] = {
static u64 power8_bhrb_filter_map(u64 branch_sample_type, u64 *filter_mask)
{
- u64 pmu_bhrb_filter = 0;
+ u64 x, tmp, pmu_bhrb_filter = 0;
+ *filter_mask = 0;
+
+ /* No branch filter requested */
+ if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY) {
+ *filter_mask = PERF_SAMPLE_BRANCH_ANY;
+ return pmu_bhrb_filter;
+ }
+
+ /*
+ * P8 does not support oring of PMU HW branch filters. Hence
+ * if multiple branch filters are requested which includes filters
+ * supported in PMU, still go ahead and clear the PMU based HW branch
+ * filter component as in this case all the filters will be processed
+ * in SW.
+ */
+ tmp = branch_sample_type;
+
+ /* Remove privilege filters before comparison */
+ tmp &= ~PERF_SAMPLE_BRANCH_USER;
+ tmp &= ~PERF_SAMPLE_BRANCH_KERNEL;
+ tmp &= ~PERF_SAMPLE_BRANCH_HV;
+
+ for_each_branch_sample_type(x) {
+ /* Ignore privilege requests */
+ if ((x == PERF_SAMPLE_BRANCH_USER) || (x == PERF_SAMPLE_BRANCH_KERNEL) || (x == PERF_SAMPLE_BRANCH_HV))
+ continue;
+
+ if (!(tmp & x))
+ continue;
+
+ /* Supported HW PMU filters */
+ if (tmp & PERF_SAMPLE_BRANCH_ANY_CALL) {
+ tmp &= ~PERF_SAMPLE_BRANCH_ANY_CALL;
+ if (tmp) {
+ pmu_bhrb_filter = 0;
+ *filter_mask = 0;
+ return pmu_bhrb_filter;
+ }
+ }
+
+ if (tmp & PERF_SAMPLE_BRANCH_COND) {
+ tmp &= ~PERF_SAMPLE_BRANCH_COND;
+ if (tmp) {
+ pmu_bhrb_filter = 0;
+ *filter_mask = 0;
+ return pmu_bhrb_filter;
+ }
+ }
+ }
/* BHRB and regular PMU events share the same privilege state
* filter configuration. BHRB is always recorded along with a
@@ -570,34 +619,20 @@ static u64 power8_bhrb_filter_map(u64 branch_sample_type, u64 *filter_mask)
* PMU event, we ignore any separate BHRB specific request.
*/
- /* No branch filter requested */
- if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY)
- return pmu_bhrb_filter;
-
- /* Invalid branch filter options - HW does not support */
- if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
- return -1;
-
- if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL)
- return -1;
-
+ /* Supported individual branch filters */
if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_CALL) {
pmu_bhrb_filter |= POWER8_MMCRA_IFM1;
+ *filter_mask |= PERF_SAMPLE_BRANCH_ANY_CALL;
return pmu_bhrb_filter;
}
if (branch_sample_type & PERF_SAMPLE_BRANCH_COND) {
pmu_bhrb_filter |= POWER8_MMCRA_IFM3;
+ *filter_mask |= PERF_SAMPLE_BRANCH_COND;
return pmu_bhrb_filter;
}
- /* PMU does not support ANY combination of HW BHRB filters */
- if ((branch_sample_type & PERF_SAMPLE_BRANCH_ANY_CALL) &&
- (branch_sample_type & PERF_SAMPLE_BRANCH_COND))
- return -1;
-
- /* Every thing else is unsupported */
- return -1;
+ return pmu_bhrb_filter;
}
static void power8_config_bhrb(u64 pmu_bhrb_filter)
--
1.7.11.7
^ permalink raw reply related
* [PATCH V4 06/10] powerpc, perf: Change the name of HW PMU branch filter tracking variable
From: Anshuman Khandual @ 2013-12-04 10:32 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: mikey, ak, eranian, michael, acme, sukadev, mingo
In-Reply-To: <1386153162-11225-1-git-send-email-khandual@linux.vnet.ibm.com>
This patch simply changes the name of the variable from "bhrb_filter" to
"bhrb_hw_filter" in order to add one more variable which will track SW
filters in generic powerpc book3s code which will be implemented in the
subsequent patch.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
arch/powerpc/perf/core-book3s.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 29b89e8..2de7d48 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -47,7 +47,7 @@ struct cpu_hw_events {
int n_txn_start;
/* BHRB bits */
- u64 bhrb_filter; /* BHRB HW branch filter */
+ u64 bhrb_hw_filter; /* BHRB HW branch filter */
int bhrb_users;
void *bhrb_context;
struct perf_branch_stack bhrb_stack;
@@ -1159,7 +1159,7 @@ static void power_pmu_enable(struct pmu *pmu)
out:
if (cpuhw->bhrb_users)
- ppmu->config_bhrb(cpuhw->bhrb_filter);
+ ppmu->config_bhrb(cpuhw->bhrb_hw_filter);
local_irq_restore(flags);
}
@@ -1254,7 +1254,7 @@ nocheck:
out:
if (has_branch_stack(event)) {
power_pmu_bhrb_enable(event);
- cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
+ cpuhw->bhrb_hw_filter = ppmu->bhrb_filter_map(
event->attr.branch_sample_type);
}
@@ -1637,10 +1637,10 @@ static int power_pmu_event_init(struct perf_event *event)
err = power_check_constraints(cpuhw, events, cflags, n + 1);
if (has_branch_stack(event)) {
- cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
+ cpuhw->bhrb_hw_filter = ppmu->bhrb_filter_map(
event->attr.branch_sample_type);
- if(cpuhw->bhrb_filter == -1)
+ if(cpuhw->bhrb_hw_filter == -1)
return -EOPNOTSUPP;
}
--
1.7.11.7
^ permalink raw reply related
* Re: [alsa-devel] [PATCH] ASoC: fsl_ssi: Implement symmetric_channels and symmetric_samplebits
From: Mark Brown @ 2013-12-04 10:42 UTC (permalink / raw)
To: Nicolin Chen; +Cc: tiwai, alsa-devel, linuxppc-dev, lgirdwood, timur
In-Reply-To: <20131204072702.GE8609@MrMyself>
[-- Attachment #1: Type: text/plain, Size: 631 bytes --]
On Wed, Dec 04, 2013 at 03:27:03PM +0800, Nicolin Chen wrote:
> Sir, shall we also implement these two to other drivers like codecs?
> I know this symmetry is a new feature and might be still immature.
> By implementing it might bring some side effects. But meanwhile, it
> would also allows us to engage more people to test it, although myself
> have done a branch of tests with different user cases on my platform.
> Or we just wait for people engaging by themselves?
It makes sense to roll it out, since it's in the core if there is a
problem we only need to fix it in one place to fix all the users which
makes life easier.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [alsa-devel] [PATCH] ASoC: fsl_ssi: Implement symmetric_channels and symmetric_samplebits
From: Nicolin Chen @ 2013-12-04 10:33 UTC (permalink / raw)
To: Mark Brown; +Cc: tiwai, alsa-devel, linuxppc-dev, lgirdwood, timur
In-Reply-To: <20131204104230.GD29268@sirena.org.uk>
On Wed, Dec 04, 2013 at 10:42:30AM +0000, Mark Brown wrote:
> On Wed, Dec 04, 2013 at 03:27:03PM +0800, Nicolin Chen wrote:
>
> > Sir, shall we also implement these two to other drivers like codecs?
> > I know this symmetry is a new feature and might be still immature.
> > By implementing it might bring some side effects. But meanwhile, it
> > would also allows us to engage more people to test it, although myself
> > have done a branch of tests with different user cases on my platform.
>
> > Or we just wait for people engaging by themselves?
>
> It makes sense to roll it out, since it's in the core if there is a
> problem we only need to fix it in one place to fix all the users which
> makes life easier.
Fully Agreed. I'll try to generate patches to do the rolling job and send
them later.
Thank you for your reply,
Nicolin Chen
^ permalink raw reply
* Re: [PATCH] powerpc: fix xmon disassembler for little-endian
From: Philippe Bergheaud @ 2013-12-04 13:45 UTC (permalink / raw)
To: Tom Musta; +Cc: linuxppc-dev
In-Reply-To: <529C9397.9050704@gmail.com>
Tom Musta wrote:
> On 12/2/2013 3:10 AM, Philippe Bergheaud wrote:
>
>>This patch fixes the disassembler of the powerpc kernel debugger xmon,
>>for little-endian.
>>
>>Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
>>---
>> arch/powerpc/xmon/xmon.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>>diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>index af9d346..6c27804 100644
>>--- a/arch/powerpc/xmon/xmon.c
>>+++ b/arch/powerpc/xmon/xmon.c
>>@@ -171,7 +171,11 @@ extern void xmon_leave(void);
>> #define REG "%.8lx"
>> #endif
>>
>>+#ifdef __LITTLE_ENDIAN__
>>+#define GETWORD(v) (((v)[3] << 24) + ((v)[2] << 16) + ((v)[1] << 8) + (v)[0])
>>+#else
>> #define GETWORD(v) (((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
>>+#endif
>>
>> #define isxdigit(c) (('0' <= (c) && (c) <= '9') \
>> || ('a' <= (c) && (c) <= 'f') \
>>
>
>
> Philippe: Wouldn't it be better to just do a 32-bit load and let the endianness be worked out
> by the hardware? i.e.
>
> #define GETWORD(v) (*(u32 *)v)
Yes, your alternative is better.
Wouldn't it narrow the scope of the macro to aligned words on POWER7?
I think that all references to GETWORD operate on aligned words anyway.
Philippe
^ permalink raw reply
* Re: [PATCH v2] offb: make the screen properties endian safe
From: Cedric Le Goater @ 2013-12-04 16:47 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1385240643.4882.133.camel@pasglop>
On 11/23/2013 10:04 PM, Benjamin Herrenschmidt wrote:
> On Sat, 2013-11-23 at 18:38 +0100, Cedric Le Goater wrote:
>> So, 32bpp "works" but 16 is broken ... I guess my palette fix is just a lucky
>> hack and I need to dig deeper in fb code to have a better understanding of
>> the color map.
>>
>> I should have provided you two patches in the first place. Do you want the
>> device tree data fixes for the frame buffer screen properties ? It helps to
>> have a display for little endian guests even if the colors are wrong.
>
> Before you pull your hair out, check if it works in BE :-)
I spent some more time on this topic and I have a few more patches to
send you.
The first patch is straight forward. It fixes host endian order issues
in the offb code when accessing the OF device tree properties.
The following patch is more of an attempt to fix the pseudo-palette
entries on little endian. Hopefully this is the good direction. If not,
I still have some hair to pull out :)
Cheers,
C.
^ permalink raw reply
* [PATCH v3 2/2] offb: add palette hack for little endian
From: Cédric Le Goater @ 2013-12-04 16:49 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1385240643.4882.133.camel@pasglop>
The pseudo palette color entries need to be ajusted for little
endian.
This patch byteswaps the values in the pseudo palette depending
on the host endian order and the screen depth.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
Tested on powerpc, little endian and big endian, for 8, 15, 16 and 32
bpp using the qemu std vga device, with and without foreign endian
support. I did pull my hair out :)
The foreign endian support needs some more work to be sure the logic
behind fb_be_math() is not broken.
The palette used for the logo would need a similar fix.
Thanks,
drivers/video/offb.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/video/offb.c b/drivers/video/offb.c
index 43a0a52fc527..7d44d669d5b6 100644
--- a/drivers/video/offb.c
+++ b/drivers/video/offb.c
@@ -91,6 +91,15 @@ extern boot_infos_t *boot_infos;
#define AVIVO_DC_LUTB_WHITE_OFFSET_GREEN 0x6cd4
#define AVIVO_DC_LUTB_WHITE_OFFSET_RED 0x6cd8
+#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
+
+static inline u32 offb_cmap_byteswap(struct fb_info *info, u32 value)
+{
+ u32 bpp = info->var.bits_per_pixel;
+
+ return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
+}
+
/*
* Set a single color register. The values supplied are already
* rounded down to the hardware's capabilities (according to the
@@ -120,7 +129,7 @@ static int offb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
mask <<= info->var.transp.offset;
value |= mask;
}
- pal[regno] = value;
+ pal[regno] = offb_cmap_byteswap(info, value);
return 0;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH v3 1/2] offb: little endian fixes
From: Cédric Le Goater @ 2013-12-04 16:49 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1385240643.4882.133.camel@pasglop>
The "screen" properties : depth, width, height, linebytes need
to be converted to the host endian order when read from the device
tree.
The offb_init_palette_hacks() routine also made assumption on the
host endian order.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
Changes in v3:
- fixed offb_init_palette_hacks()
Changes in v2:
- replaced be32_to_cpu() by be32_to_cpup()
drivers/video/offb.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/video/offb.c b/drivers/video/offb.c
index 9dbea2223401..43a0a52fc527 100644
--- a/drivers/video/offb.c
+++ b/drivers/video/offb.c
@@ -301,7 +301,7 @@ static struct fb_ops offb_ops = {
static void __iomem *offb_map_reg(struct device_node *np, int index,
unsigned long offset, unsigned long size)
{
- const u32 *addrp;
+ const __be32 *addrp;
u64 asize, taddr;
unsigned int flags;
@@ -369,7 +369,11 @@ static void offb_init_palette_hacks(struct fb_info *info, struct device_node *dp
}
of_node_put(pciparent);
} else if (dp && of_device_is_compatible(dp, "qemu,std-vga")) {
- const u32 io_of_addr[3] = { 0x01000000, 0x0, 0x0 };
+#ifdef __BIG_ENDIAN
+ const __be32 io_of_addr[3] = { 0x01000000, 0x0, 0x0 };
+#else
+ const __be32 io_of_addr[3] = { 0x00000001, 0x0, 0x0 };
+#endif
u64 io_addr = of_translate_address(dp, io_of_addr);
if (io_addr != OF_BAD_ADDR) {
par->cmap_adr = ioremap(io_addr + 0x3c8, 2);
@@ -535,7 +539,7 @@ static void __init offb_init_nodriver(struct device_node *dp, int no_real_node)
unsigned int flags, rsize, addr_prop = 0;
unsigned long max_size = 0;
u64 rstart, address = OF_BAD_ADDR;
- const u32 *pp, *addrp, *up;
+ const __be32 *pp, *addrp, *up;
u64 asize;
int foreign_endian = 0;
@@ -551,25 +555,25 @@ static void __init offb_init_nodriver(struct device_node *dp, int no_real_node)
if (pp == NULL)
pp = of_get_property(dp, "depth", &len);
if (pp && len == sizeof(u32))
- depth = *pp;
+ depth = be32_to_cpup(pp);
pp = of_get_property(dp, "linux,bootx-width", &len);
if (pp == NULL)
pp = of_get_property(dp, "width", &len);
if (pp && len == sizeof(u32))
- width = *pp;
+ width = be32_to_cpup(pp);
pp = of_get_property(dp, "linux,bootx-height", &len);
if (pp == NULL)
pp = of_get_property(dp, "height", &len);
if (pp && len == sizeof(u32))
- height = *pp;
+ height = be32_to_cpup(pp);
pp = of_get_property(dp, "linux,bootx-linebytes", &len);
if (pp == NULL)
pp = of_get_property(dp, "linebytes", &len);
if (pp && len == sizeof(u32) && (*pp != 0xffffffffu))
- pitch = *pp;
+ pitch = be32_to_cpup(pp);
else
pitch = width * ((depth + 7) / 8);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v6 10/17] serial: mpc512x: setup the PSC FIFO clock as well
From: Greg Kroah-Hartman @ 2013-12-05 0:55 UTC (permalink / raw)
To: Gerhard Sittig
Cc: Mike Turquette, Detlev Zundel, Jiri Slaby, linux-serial,
Scott Wood, Anatolij Gustschin, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1385851897-23475-11-git-send-email-gsi@denx.de>
On Sat, Nov 30, 2013 at 11:51:30PM +0100, Gerhard Sittig wrote:
> prepare and enable the FIFO clock upon PSC FIFO initialization,
> check for and propagage errors when enabling the PSC FIFO clock,
> disable and unprepare the FIFO clock upon PSC FIFO uninitialization
>
> devm_{get,put}_clk() doesn't apply here, as the SoC provides a
> single FIFO component which is shared among several PSC components,
> thus the FIFO isn't associated with a device (while the PSCs are)
>
> provide a fallback clock lookup approach in case the OF based clock
> lookup for the PSC FIFO fails, this allows for successful operation in
> the presence of an outdated device tree which lacks clock specs
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: linux-serial@vger.kernel.org
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> # for v4
> Signed-off-by: Gerhard Sittig <gsi@denx.de>
> ---
> Greg, the addition since v4 is the clk_get_sys() call for the 'ipg'
> clock item (backwards compat for device trees w/o clock specs)
That's fine, my ACK still stands.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v6 11/17] USB: fsl-mph-dr-of: adjust for OF based clock lookup
From: Greg Kroah-Hartman @ 2013-12-05 0:55 UTC (permalink / raw)
To: Gerhard Sittig
Cc: Mike Turquette, Detlev Zundel, linux-usb, Scott Wood,
Anatolij Gustschin, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1385851897-23475-12-git-send-email-gsi@denx.de>
On Sat, Nov 30, 2013 at 11:51:31PM +0100, Gerhard Sittig wrote:
> after device tree based clock lookup became available, the peripheral
> driver need no longer construct clock names which include the component
> index -- remove the "usb%d_clk" template, always use "ipg" instead
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Gerhard Sittig <gsi@denx.de>
> ---
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [PATCH v6 09/17] serial: mpc512x: adjust for OF based clock lookup
From: Greg Kroah-Hartman @ 2013-12-05 0:55 UTC (permalink / raw)
To: Gerhard Sittig
Cc: Mike Turquette, Detlev Zundel, Jiri Slaby, linux-serial,
Scott Wood, Anatolij Gustschin, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1385851897-23475-10-git-send-email-gsi@denx.de>
On Sat, Nov 30, 2013 at 11:51:29PM +0100, Gerhard Sittig wrote:
> after device tree based clock lookup became available, the peripheral
> driver need no longer construct clock names which include the PSC index,
> remove the "psc%d_mclk" template and unconditionally use 'mclk'
>
> acquire and release the "ipg" clock item for register access as well
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: linux-serial@vger.kernel.org
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> # for v4
> Signed-off-by: Gerhard Sittig <gsi@denx.de>
> ---
> Greg, the difference since v4 of this patch is that v4 took the 'mclk'
> and 'ipg' clock items in reverse order, and thus potentially obfuscated
> the adjusted name for 'mclk' -- the updated version of the patch is
> identical in content but cleaner diff-wise
That's fine:
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* [PATCH 2/2] arch/powerpc: Dynamically allocate slb_shadow from memblock
From: Jeremy Kerr @ 2013-12-05 3:31 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Michael Neuling
In-Reply-To: <1386214268.68092.227432126043.1.gpush@pablo>
Currently, the slb_shadow buffer is our largest symbol:
[jk@pablo linux]$ nm --size-sort -r -S obj/vmlinux | head -1
c000000000da0000 0000000000040000 d slb_shadow
- we allocate 128 bytes per cpu; so 256k with NR_CPUS=2048. As we have
constant initialisers, it's allocated in .text, causing a larger vmlinux
image. We may also allocate unecessary slb_shadow buffers (> no. pacas),
since we use the build-time NR_CPUS rather than the run-time nr_cpu_ids.
We could move this to the bss, but then we still have the NR_CPUS vs
nr_cpu_ids potential for overallocation.
This change dynamically allocates the slb_shadow array, during
initialise_pacas(). At a cost of 104 bytes of text, we save 256k of
data:
[jk@pablo linux]$ size obj/vmlinux{.orig,}
text data bss dec hex filename
9202795 5244676 1169576 15617047 ee4c17 obj/vmlinux.orig
9202899 4982532 1169576 15355007 ea4c7f obj/vmlinux
Tested on pseries.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
arch/powerpc/kernel/paca.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 9095a6f7..ad7a485f 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -99,12 +99,28 @@ static inline void free_lppacas(void) { }
* 3 persistent SLBs are registered here. The buffer will be zero
* initially, hence will all be invaild until we actually write them.
*/
-static struct slb_shadow slb_shadow[] __cacheline_aligned = {
- [0 ... (NR_CPUS-1)] = {
- .persistent = cpu_to_be32(SLB_NUM_BOLTED),
- .buffer_length = cpu_to_be32(sizeof(struct slb_shadow)),
- },
-};
+static struct slb_shadow *slb_shadow;
+
+static void __init allocate_slb_shadows(int nr_cpus, int limit)
+{
+ int size = PAGE_ALIGN(sizeof(struct slb_shadow) * nr_cpus);
+ slb_shadow = __va(memblock_alloc_base(size, PAGE_SIZE, limit));
+ memset(slb_shadow, 0, size);
+}
+
+static struct slb_shadow * __init init_slb_shadow(int cpu)
+{
+ struct slb_shadow *s = &slb_shadow[cpu];
+
+ s->persistent = ARRAY_SIZE(s->save_area);
+ s->buffer_length = sizeof(*s);
+
+ return s;
+}
+
+#else /* CONFIG_PPC_STD_MMU_64 */
+
+static void __init allocate_slb_shadows(int nr_cpus, int limit) { }
#endif /* CONFIG_PPC_STD_MMU_64 */
@@ -142,7 +158,7 @@ void __init initialise_paca(struct paca_struct *new_paca, int cpu)
new_paca->__current = &init_task;
new_paca->data_offset = 0xfeeeeeeeeeeeeeeeULL;
#ifdef CONFIG_PPC_STD_MMU_64
- new_paca->slb_shadow_ptr = &slb_shadow[cpu];
+ new_paca->slb_shadow_ptr = init_slb_shadow(cpu);
#endif /* CONFIG_PPC_STD_MMU_64 */
}
@@ -190,6 +206,8 @@ void __init allocate_pacas(void)
allocate_lppacas(nr_cpu_ids, limit);
+ allocate_slb_shadows(nr_cpu_ids, limit);
+
/* Can't use for_each_*_cpu, as they aren't functional yet */
for (cpu = 0; cpu < nr_cpu_ids; cpu++)
initialise_paca(&paca[cpu], cpu);
^ permalink raw reply related
* [PATCH 1/2] arch/powerpc: Make slb_shadow a local
From: Jeremy Kerr @ 2013-12-05 3:31 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Michael Neuling
The only external user of slb_shadow is the pseries lpar code, and it
can access through the paca array instead.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
arch/powerpc/include/asm/lppaca.h | 2 --
arch/powerpc/kernel/paca.c | 2 +-
arch/powerpc/platforms/pseries/lpar.c | 2 +-
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/lppaca.h b/arch/powerpc/include/asm/lppaca.h
index 844c28de..d0a2a2f9 100644
--- a/arch/powerpc/include/asm/lppaca.h
+++ b/arch/powerpc/include/asm/lppaca.h
@@ -132,8 +132,6 @@ struct slb_shadow {
} save_area[SLB_NUM_BOLTED];
} ____cacheline_aligned;
-extern struct slb_shadow slb_shadow[];
-
/*
* Layout of entries in the hypervisor's dispatch trace log buffer.
*/
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 0620eaaa..9095a6f7 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -99,7 +99,7 @@ static inline void free_lppacas(void) { }
* 3 persistent SLBs are registered here. The buffer will be zero
* initially, hence will all be invaild until we actually write them.
*/
-struct slb_shadow slb_shadow[] __cacheline_aligned = {
+static struct slb_shadow slb_shadow[] __cacheline_aligned = {
[0 ... (NR_CPUS-1)] = {
.persistent = cpu_to_be32(SLB_NUM_BOLTED),
.buffer_length = cpu_to_be32(sizeof(struct slb_shadow)),
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 4fca3def..28cf0f33 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -92,7 +92,7 @@ void vpa_init(int cpu)
* PAPR says this feature is SLB-Buffer but firmware never
* reports that. All SPLPAR support SLB shadow buffer.
*/
- addr = __pa(&slb_shadow[cpu]);
+ addr = __pa(paca[cpu].slb_shadow_ptr);
if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
ret = register_slb_shadow(hwcpu, addr);
if (ret)
^ permalink raw reply related
* [PATCH v2] arch/powerpc: Dynamically allocate slb_shadow from memblock
From: Jeremy Kerr @ 2013-12-05 3:42 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Michael Neuling
In-Reply-To: <1386214268.68246.485420017657.2.gpush@pablo>
Currently, the slb_shadow buffer is our largest symbol:
[jk@pablo linux]$ nm --size-sort -r -S obj/vmlinux | head -1
c000000000da0000 0000000000040000 d slb_shadow
- we allocate 128 bytes per cpu; so 256k with NR_CPUS=2048. As we have
constant initialisers, it's allocated in .text, causing a larger vmlinux
image. We may also allocate unecessary slb_shadow buffers (> no. pacas),
since we use the build-time NR_CPUS rather than the run-time nr_cpu_ids.
We could move this to the bss, but then we still have the NR_CPUS vs
nr_cpu_ids potential for overallocation.
This change dynamically allocates the slb_shadow array, during
initialise_pacas(). At a cost of 104 bytes of text, we save 256k of
data:
[jk@pablo linux]$ size obj/vmlinux{.orig,}
text data bss dec hex filename
9202795 5244676 1169576 15617047 ee4c17 obj/vmlinux.orig
9202899 4982532 1169576 15355007 ea4c7f obj/vmlinux
Tested on pseries.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
v2: don't drop the endian conversion, use SLB_NUM_BOLTED
---
arch/powerpc/kernel/paca.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 9095a6f7..623c356f 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -99,12 +99,28 @@ static inline void free_lppacas(void) { }
* 3 persistent SLBs are registered here. The buffer will be zero
* initially, hence will all be invaild until we actually write them.
*/
-static struct slb_shadow slb_shadow[] __cacheline_aligned = {
- [0 ... (NR_CPUS-1)] = {
- .persistent = cpu_to_be32(SLB_NUM_BOLTED),
- .buffer_length = cpu_to_be32(sizeof(struct slb_shadow)),
- },
-};
+static struct slb_shadow *slb_shadow;
+
+static void __init allocate_slb_shadows(int nr_cpus, int limit)
+{
+ int size = PAGE_ALIGN(sizeof(struct slb_shadow) * nr_cpus);
+ slb_shadow = __va(memblock_alloc_base(size, PAGE_SIZE, limit));
+ memset(slb_shadow, 0, size);
+}
+
+static struct slb_shadow * __init init_slb_shadow(int cpu)
+{
+ struct slb_shadow *s = &slb_shadow[cpu];
+
+ s->persistent = cpu_to_be32(SLB_NUM_BOLTED);
+ s->buffer_length = cpu_to_be32(sizeof(*s));
+
+ return s;
+}
+
+#else /* CONFIG_PPC_STD_MMU_64 */
+
+static void __init allocate_slb_shadows(int nr_cpus, int limit) { }
#endif /* CONFIG_PPC_STD_MMU_64 */
@@ -142,7 +158,7 @@ void __init initialise_paca(struct paca_struct *new_paca, int cpu)
new_paca->__current = &init_task;
new_paca->data_offset = 0xfeeeeeeeeeeeeeeeULL;
#ifdef CONFIG_PPC_STD_MMU_64
- new_paca->slb_shadow_ptr = &slb_shadow[cpu];
+ new_paca->slb_shadow_ptr = init_slb_shadow(cpu);
#endif /* CONFIG_PPC_STD_MMU_64 */
}
@@ -190,6 +206,8 @@ void __init allocate_pacas(void)
allocate_lppacas(nr_cpu_ids, limit);
+ allocate_slb_shadows(nr_cpu_ids, limit);
+
/* Can't use for_each_*_cpu, as they aren't functional yet */
for (cpu = 0; cpu < nr_cpu_ids; cpu++)
initialise_paca(&paca[cpu], cpu);
^ permalink raw reply related
* Re: [PATCH] powerpc: fix xmon disassembler for little-endian
From: Benjamin Herrenschmidt @ 2013-12-05 4:39 UTC (permalink / raw)
To: Philippe Bergheaud; +Cc: Tom Musta, linuxppc-dev
In-Reply-To: <529F31F8.6040302@linux.vnet.ibm.com>
On Wed, 2013-12-04 at 14:45 +0100, Philippe Bergheaud wrote:
> >>+#ifdef __LITTLE_ENDIAN__
> >>+#define GETWORD(v) (((v)[3] << 24) + ((v)[2] << 16) + ((v)[1] << 8) + (v)[0])
> >>+#else
> >> #define GETWORD(v) (((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
> >>+#endif
> >>
> >> #define isxdigit(c) (('0' <= (c) && (c) <= '9') \
> >> || ('a' <= (c) && (c) <= 'f') \
> >>
> >
> >
> > Philippe: Wouldn't it be better to just do a 32-bit load and let the endianness be worked out
> > by the hardware? i.e.
> >
> > #define GETWORD(v) (*(u32 *)v)
> Yes, your alternative is better.
> Wouldn't it narrow the scope of the macro to aligned words on POWER7?
> I think that all references to GETWORD operate on aligned words anyway.
Well, xmon has to be robust ... as long as you are *certain* that even
with crap entry state it won't try to access unaligned boundaries then
go for it but we aren't looking at performance here.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH V4 00/10] perf: New conditional branch filter
From: Michael Ellerman @ 2013-12-05 4:47 UTC (permalink / raw)
To: acme
Cc: mikey, ak, linux-kernel, eranian, linuxppc-dev, sukadev, mingo,
Anshuman Khandual
In-Reply-To: <1386153162-11225-1-git-send-email-khandual@linux.vnet.ibm.com>
On Wed, 2013-12-04 at 16:02 +0530, Anshuman Khandual wrote:
> This patchset is the re-spin of the original branch stack sampling
> patchset which introduced new PERF_SAMPLE_BRANCH_COND branch filter. This patchset
> also enables SW based branch filtering support for book3s powerpc platforms which
> have PMU HW backed branch stack sampling support.
>
> Summary of code changes in this patchset:
>
> (1) Introduces a new PERF_SAMPLE_BRANCH_COND branch filter
> (2) Add the "cond" branch filter options in the "perf record" tool
> (3) Enable PERF_SAMPLE_BRANCH_COND in X86 platforms
> (4) Enable PERF_SAMPLE_BRANCH_COND in POWER8 platform
> (5) Update the documentation regarding "perf record" tool
Hi Arnaldo,
Can you please take just patches 1-5 into the perf tree? And do you mind
putting them in a topic branch so Benh can merge that.
The remaining patches are powerpc specific and still need some more review.
cheers
^ permalink raw reply
* Re: [PATCH 3/8] IBM Akebono: Add support for a new PHY interface to the IBM emac driver
From: Benjamin Herrenschmidt @ 2013-12-05 4:49 UTC (permalink / raw)
To: Alistair Popple; +Cc: linuxppc-dev, David S. Miller, netdev
In-Reply-To: <1385086116-10972-3-git-send-email-alistair@popple.id.au>
On Fri, 2013-11-22 at 13:08 +1100, Alistair Popple wrote:
> The IBM PPC476GTR SoC that is used on the Akebono board uses a
> different ethernet PHY interface that has wake on lan (WOL) support
> with the IBM emac. This patch adds support to the IBM emac driver for
> this new PHY interface.
>
> At this stage the wake on lan functionality has not been implemented.
>
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
^ permalink raw reply
* Re: [PATCH -V2 3/5] mm: Move change_prot_numa outside CONFIG_ARCH_USES_NUMA_PROT_NONE
From: Aneesh Kumar K.V @ 2013-12-05 5:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Mel Gorman, Rik van Riel
Cc: linux-mm, paulus, linuxppc-dev
In-Reply-To: <1386126782.16703.137.camel@pasglop>
Adding Mel and Rik to cc:
Benjamin Herrenschmidt <benh@au1.ibm.com> writes:
> On Mon, 2013-11-18 at 14:58 +0530, Aneesh Kumar K.V wrote:
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> change_prot_numa should work even if _PAGE_NUMA != _PAGE_PROTNONE.
>> On archs like ppc64 that don't use _PAGE_PROTNONE and also have
>> a separate page table outside linux pagetable, we just need to
>> make sure that when calling change_prot_numa we flush the
>> hardware page table entry so that next page access result in a numa
>> fault.
>
> That patch doesn't look right...
>
> You are essentially making change_prot_numa() do whatever it does (which
> I don't completely understand) *for all architectures* now, whether they
> have CONFIG_ARCH_USES_NUMA_PROT_NONE or not ... So because you want that
> behaviour on powerpc book3s64, you change everybody.
>
> Is that correct ?
Yes.
>
> Also what exactly is that doing, can you explain ? From what I can see,
> it calls back into the core of mprotect to change the protection to
> vma->vm_page_prot, which I would have expected is already the protection
> there, with the added "prot_numa" flag passed down.
it set the _PAGE_NUMA bit. Now we also want to make sure that when
we set _PAGE_NUMA, we would get a pagefault on that so that we can track
that fault as a numa fault. To ensure that, we had the below BUILD_BUG
BUILD_BUG_ON(_PAGE_NUMA != _PAGE_PROTNONE);
But other than that the function doesn't really have any dependency on
_PAGE_PROTNONE. The only requirement is when we set _PAGE_NUMA, the
architecture should do enough to ensure that we get a page fault. Now on
ppc64 we does that by clearlying hpte entry and also clearing
_PAGE_PRESENT. Since we have _PAGE_PRESENT cleared hash_page will return
1 and we get to page fault handler.
>
> Your changeset comment says "On archs like ppc64 [...] we just need to
> make sure that when calling change_prot_numa we flush the
> hardware page table entry so that next page access result in a numa
> fault."
>
> But change_prot_numa() does a lot more than that ... it does
> pte_mknuma(), do we need it ? I assume we do or we wouldn't have added
> that PTE bit to begin with...
>
> Now it *might* be allright and it might be that no other architecture
> cares anyway etc... but I need at least some mm folks to ack on that
> patch before I can take it because it *will* change behaviour of other
> architectures.
>
Ok, I can move the changes below #ifdef CONFIG_NUMA_BALANCING ? We call
change_prot_numa from task_numa_work and queue_pages_range(). The later
may be an issue. So doing the below will help ?
-#ifdef CONFIG_ARCH_USES_NUMA_PROT_NONE
+#ifdef CONFIG_NUMA_BALANCING
-aneesh
^ permalink raw reply
* Re: [PATCH -V2 3/5] mm: Move change_prot_numa outside CONFIG_ARCH_USES_NUMA_PROT_NONE
From: Benjamin Herrenschmidt @ 2013-12-05 5:20 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linux-mm, Rik van Riel, linuxppc-dev, paulus, Mel Gorman
In-Reply-To: <87a9gfri3u.fsf@linux.vnet.ibm.com>
On Thu, 2013-12-05 at 10:48 +0530, Aneesh Kumar K.V wrote:
>
> Ok, I can move the changes below #ifdef CONFIG_NUMA_BALANCING ? We call
> change_prot_numa from task_numa_work and queue_pages_range(). The later
> may be an issue. So doing the below will help ?
>
> -#ifdef CONFIG_ARCH_USES_NUMA_PROT_NONE
> +#ifdef CONFIG_NUMA_BALANCING
I will defer to Mel and Rik (should we also CC Andrea ?)
Cheers,
Ben.
^ permalink raw reply
* [PATCH] powerpc/p1010rdb-pa: modify phy interrupt.
From: Zhao Qiang @ 2013-12-05 6:00 UTC (permalink / raw)
To: bcousson, tony, devicetree; +Cc: Zhao Qiang, linuxppc-dev
It is not correct according to p1010rdb-pa user guide.
So modify it.
Signed-off-by: Zhao Qiang <B45475@freescale.com>
---
arch/powerpc/boot/dts/p1010rdb-pa.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/boot/dts/p1010rdb-pa.dtsi b/arch/powerpc/boot/dts/p1010rdb-pa.dtsi
index 3d275e0..0cf6c34 100644
--- a/arch/powerpc/boot/dts/p1010rdb-pa.dtsi
+++ b/arch/powerpc/boot/dts/p1010rdb-pa.dtsi
@@ -39,7 +39,7 @@
};
&phy0 {
- interrupts = <3 1 0 0>;
+ interrupts = <1 1 0 0>;
};
&phy1 {
@@ -47,5 +47,5 @@
};
&phy2 {
- interrupts = <2 1 0 0>;
+ interrupts = <4 1 0 0>;
};
--
1.8.4
^ permalink raw reply related
* Re: [PATCH] powerpc: fix xmon disassembler for little-endian
From: Philippe Bergheaud @ 2013-12-05 8:50 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Tom Musta, linuxppc-dev
In-Reply-To: <1386218366.21910.16.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Wed, 2013-12-04 at 14:45 +0100, Philippe Bergheaud wrote:
>
>
>>>>+#ifdef __LITTLE_ENDIAN__
>>>>+#define GETWORD(v) (((v)[3] << 24) + ((v)[2] << 16) + ((v)[1] << 8) + (v)[0])
>>>>+#else
>>>>#define GETWORD(v) (((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
>>>>+#endif
>>>>
>>>>#define isxdigit(c) (('0' <= (c) && (c) <= '9') \
>>>> || ('a' <= (c) && (c) <= 'f') \
>>>>
>>>
>>>
>>>Philippe: Wouldn't it be better to just do a 32-bit load and let the endianness be worked out
>>>by the hardware? i.e.
>>>
>>>#define GETWORD(v) (*(u32 *)v)
>>
>>Yes, your alternative is better.
>>Wouldn't it narrow the scope of the macro to aligned words on POWER7?
>>I think that all references to GETWORD operate on aligned words anyway.
>
>
> Well, xmon has to be robust ... as long as you are *certain* that even
> with crap entry state it won't try to access unaligned boundaries then
> go for it but we aren't looking at performance here.
Thank you Tom and Ben. We are definitely not looking at performance here.
I prefer to stay on the safe side, and leave the original patch untouched.
Philippe
^ permalink raw reply
* Re: [PATCH 1/3] powerpc: mm: make _PAGE_NUMA take effect
From: Aneesh Kumar K.V @ 2013-12-05 10:53 UTC (permalink / raw)
To: Liu Ping Fan, linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <1386140348-7854-2-git-send-email-pingfank@linux.vnet.ibm.com>
Liu Ping Fan <kernelfans@gmail.com> writes:
> To enable the do_numa_page(), we should not fix _PAGE_NUMA in
> hash_page(), so bail out for the case of pte_numa().
>
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/hash_utils_64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index fb176e9..9bf1195 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -1033,7 +1033,7 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap)
>
> /* Get PTE and page size from page tables */
> ptep = find_linux_pte_or_hugepte(pgdir, ea, &hugeshift);
> - if (ptep == NULL || !pte_present(*ptep)) {
> + if (ptep == NULL || !pte_present(*ptep) || pte_numa(*ptep)) {
> DBG_LOW(" no PTE !\n");
> rc = 1;
> goto bail;
why ? , All the hash routines do check for _PAGE_PRESENT via access
variable.
-aneesh
^ permalink raw reply
* RE: Error in frreing hugepages with preemption enabled
From: Bharat Bhushan @ 2013-12-05 11:14 UTC (permalink / raw)
To: Andrea Arcangeli, Alexander Graf
Cc: Scott Wood, linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
kvm-ppc@vger.kernel.org
In-Reply-To: <20131203222121.GB18764@redhat.com>
> -----Original Message-----
> From: Andrea Arcangeli [mailto:aarcange@redhat.com]
> Sent: Wednesday, December 04, 2013 3:51 AM
> To: Alexander Graf
> Cc: Bhushan Bharat-R65777; linuxppc-dev@lists.ozlabs.org; kvm-
> ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Ben Herrensc=
hmidt
> Subject: Re: Error in frreing hugepages with preemption enabled
>=20
> Hi everyone,
>=20
> On Fri, Nov 29, 2013 at 12:13:03PM +0100, Alexander Graf wrote:
> >
> > On 29.11.2013, at 05:38, Bharat Bhushan <Bharat.Bhushan@freescale.com> =
wrote:
> >
> > > Hi Alex,
> > >
> > > I am running KVM guest with host kernel having CONFIG_PREEMPT enabled=
. With
> allocated pages things seems to work fine but I uses hugepages for guest =
I see
> below prints when "quit" from qemu.
> > >
> > > (qemu) QEMU waiting for connection on: telnet:0.0.0.0:4444,server
> > > qemu-system-ppc64: pci_add_option_rom: failed to find romfile "efi-
> virtio.rom"
> > > q
> > > debug_smp_processor_id: 15 callbacks suppressed
> > > BUG: using smp_processor_id() in preemptible [00000000] code:
> > > qemu-system-ppc/2504 caller is .free_hugepd_range+0xb0/0x21c
> > > CPU: 1 PID: 2504 Comm: qemu-system-ppc Not tainted
> > > 3.12.0-rc3-07733-gabf4907 #175 Call Trace:
> > > [c0000000fb433400] [c000000000007d38] .show_stack+0x7c/0x1cc
> > > (unreliable) [c0000000fb4334d0] [c0000000005e8ce0]
> > > .dump_stack+0x9c/0xf4 [c0000000fb433560] [c0000000002de5ec]
> > > .debug_smp_processor_id+0x108/0x11c
> > > [c0000000fb4335f0] [c000000000025e10] .free_hugepd_range+0xb0/0x21c
> > > [c0000000fb433680] [c0000000000265bc]
> > > .hugetlb_free_pgd_range+0x2c8/0x3b0
> > > [c0000000fb4337a0] [c0000000000e428c] .free_pgtables+0x14c/0x158
> > > [c0000000fb433840] [c0000000000ef320] .exit_mmap+0xec/0x194
> > > [c0000000fb433960] [c00000000004d780] .mmput+0x64/0x124
> > > [c0000000fb4339e0] [c000000000051f40] .do_exit+0x29c/0x9c8
> > > [c0000000fb433ae0] [c0000000000527c8] .do_group_exit+0x50/0xc4
> > > [c0000000fb433b70] [c0000000000606a0]
> > > .get_signal_to_deliver+0x21c/0x5d8
> > > [c0000000fb433c70] [c000000000009b08] .do_signal+0x54/0x278
> > > [c0000000fb433db0] [c000000000009e50] .do_notify_resume+0x64/0x78
> > > [c0000000fb433e30] [c000000000000b44]
> > > .ret_from_except_lite+0x70/0x74
> > >
> > >
> > > This mean that free_hugepd_range() must be called with preemption ena=
bled.
> >
> > with preemption disabled.
> >
> > > I tried below change and this seems to work fine (I am not having
> > > expertise in this area so not sure this is correct way)
> >
> > Not sure - the scope looks odd to me. Let's ask Andrea - I'm sure he kn=
ows
> what to do :).
>=20
> :) So I had a look at the top of this function (0xb0) in the upstream ker=
nel and
> no smp_processor_id() call is apparent, is this stock git or a ppc tree? =
The
> first few calls seem not to call it but I may have overlooked something. =
It's
> just quicker if somebody with vmlinux finds the location of it.
>=20
> static void free_hugepd_range(struct mmu_gather *tlb, hugepd_t *hpdp, int
> pdshift,
> unsigned long start, unsigned long end,
> unsigned long floor, unsigned long ceiling) {
> pte_t *hugepte =3D hugepd_page(*hpdp);
> int i;
>=20
> unsigned long pdmask =3D ~((1UL << pdshift) - 1);
> unsigned int num_hugepd =3D 1;
>=20
> #ifdef CONFIG_PPC_FSL_BOOK3E
> /* Note: On fsl the hpdp may be the first of several */
> num_hugepd =3D (1 << (hugepd_shift(*hpdp) - pdshift)); #else
> unsigned int shift =3D hugepd_shift(*hpdp); #endif
>=20
> start &=3D pdmask;
> if (start < floor)
> return;
> if (ceiling) {
> ceiling &=3D pdmask;
> if (! ceiling)
> return;
> }
> if (end - 1 > ceiling - 1)
> return;
>=20
> for (i =3D 0; i < num_hugepd; i++, hpdp++)
> hpdp->pd =3D 0;
>=20
> tlb->need_flush =3D 1;
>=20
> #ifdef CONFIG_PPC_FSL_BOOK3E
> hugepd_free(tlb, hugepte);
> #else
> pgtable_free_tlb(tlb, hugepte, pdshift - shift); #endif }
>=20
> Generally smp_processor_id should never be used, exactly to avoid problem=
s like
> above with preempion enabled in .config.
>=20
> Instead it should be replaced with a get_cpu()/put_cpu() pair that is exa=
ctly
> meant to fix bugs like this and define proper critical sections around th=
e per-
> cpu variables.
>=20
> #define get_cpu() ({ preempt_disable(); smp_processor_id(); })
> #define put_cpu() preempt_enable()
>=20
> After you find where that smp_processor_id() is located, you should simpl=
y
> replace it with a get_cpu() and then you should insert a put_cpu immediat=
ely
> after the "cpu" info is not used anymore. That will define a proper and s=
trict
> critical section around the use of the per-cpu variables.
>=20
> With a ppc vmlinux it should be immediate to find the location of
> smp_processor_id but I don't have the ppc vmlinux here.
Thanks Andrea for the details description. It is really helpful
I will look into this.
Thanks
-Bharat
>=20
> Thanks!
> Andrea
>=20
> >
> >
> > Alex
> >
> > >
> > > diff --git a/arch/powerpc/mm/hugetlbpage.c
> > > b/arch/powerpc/mm/hugetlbpage.c index d67db4b..6bf8459 100644
> > > --- a/arch/powerpc/mm/hugetlbpage.c
> > > +++ b/arch/powerpc/mm/hugetlbpage.c
> > > @@ -563,8 +563,10 @@ static void hugetlb_free_pmd_range(struct mmu_ga=
ther
> *tlb, pud_t *pud,
> > > */
> > > next =3D addr + (1 << hugepd_shift(*(hugepd_t *)pmd));
> > > #endif
> > > + preempt_disable();
> > > free_hugepd_range(tlb, (hugepd_t *)pmd, PMD_SHIFT,
> > > addr, next, floor, ceiling);
> > > + preempt_enable();
> > > } while (addr =3D next, addr !=3D end);
> > >
> > > start &=3D PUD_MASK;
> > >
> > >
> > > Thanks
> > > -Bharat
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe kvm-ppc"
> > > 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 3/3] powerpc: mm: optimize for the correctly placed page
From: Aneesh Kumar K.V @ 2013-12-05 10:58 UTC (permalink / raw)
To: Liu Ping Fan, linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <1386140348-7854-4-git-send-email-pingfank@linux.vnet.ibm.com>
Liu Ping Fan <kernelfans@gmail.com> writes:
> The period check of _PAGE_NUMA can probably trigger the check on
> the correctly placed page. For this case, we can just insert hpte and
> do fast exception return.
I still don't understand why we need to handle numa faults in hash
page ? Are you trying to optimize the code path ? If so can you explain
the benefits ? Some numbers showing it is helping ?
>
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/hash_utils_64.c | 34 +++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index 9bf1195..735678c 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -965,6 +965,10 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap)
> const struct cpumask *tmp;
> int rc, user_region = 0, local = 0;
> int psize, ssize;
> + pte_t old, new;
> + struct vm_area_struct *vma;
> + int page_nid, target_nid;
> + struct page *test_page;
>
> DBG_LOW("hash_page(ea=%016lx, access=%lx, trap=%lx\n",
> ea, access, trap);
> @@ -1033,12 +1037,40 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap)
>
> /* Get PTE and page size from page tables */
> ptep = find_linux_pte_or_hugepte(pgdir, ea, &hugeshift);
> - if (ptep == NULL || !pte_present(*ptep) || pte_numa(*ptep)) {
> + if (ptep == NULL || !pte_present(*ptep)) {
> DBG_LOW(" no PTE !\n");
> rc = 1;
> goto bail;
> }
>
> + old = pte_val(*ptep);
> + if (pte_numa(old)) {
> + /* If fail to lock, let do_page_fault() to handle it */
> + if (down_read_trylock(&mm->mmap_sem)) {
hmm is that something we want to do in hash_page ?
> + vma = find_vma(mm, ea);
> + up_read(&mm->mmap_sem);
> + test_page = pte_page(old);
> + page_nid = page_to_nid(test_page);
> + target_nid = numa_migrate_prep(test_page, vma, ea,
> + page_nid);
> + if (target_nid < 0) {
> + new = pte_mknonnuma(old);
> + /* If ptep is modified under us,
> + * just retry the access
> + */
> + if (unlikely(cmpxchg(ptep, old, new) != old)) {
> + put_page(test_page);
> + return 0;
> + }
> + put_page(test_page);
> + }
> + } else {
> + put_page(test_page);
> + rc = 1;
> + goto bail;
> + }
> + }
> +
> /* Add _PAGE_PRESENT to the required access perm */
> access |= _PAGE_PRESENT;
>
-aneesh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox