LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/5] memory-hotplug: ia64: suitable memory should go to ZONE_MOVABLE
From: Wang Nan @ 2014-07-18  7:56 UTC (permalink / raw)
  To: Ingo Molnar, Yinghai Lu, Mel Gorman, Andrew Morton
  Cc: linux-ia64, Pei Feiyue, linux-sh, x86, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <1405670163-53747-1-git-send-email-wangnan0@huawei.com>

This patch add new memory to ZONE_MOVABLE if movable zone is setup
and lower than newly added memory for ia64.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 arch/ia64/mm/init.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 25c3502..d81c916 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -625,6 +625,7 @@ int arch_add_memory(int nid, u64 start, u64 size)
 {
 	pg_data_t *pgdat;
 	struct zone *zone;
+	struct zone *movable_zone;
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	int ret;
@@ -632,6 +633,12 @@ int arch_add_memory(int nid, u64 start, u64 size)
 	pgdat = NODE_DATA(nid);
 
 	zone = pgdat->node_zones + ZONE_NORMAL;
+	movable_zone = pgdat->node_zones + ZONE_MOVABLE;
+	if (!zone_is_empty(movable_zone))
+		if (zone_spans_pfn(movable_zone, start_pfn) ||
+				(zone_end_pfn(movable_zone) <= start_pfn))
+			zone = movable_zone;
+
 	ret = __add_pages(nid, zone, start_pfn, nr_pages);
 
 	if (ret)
-- 
1.8.4

^ permalink raw reply related

* [PATCH 0/5] memory-hotplug: suitable memory should go to ZONE_MOVABLE
From: Wang Nan @ 2014-07-18  7:55 UTC (permalink / raw)
  To: Ingo Molnar, Yinghai Lu, Mel Gorman, Andrew Morton
  Cc: linux-ia64, Pei Feiyue, linux-sh, x86, linux-kernel, linux-mm,
	linuxppc-dev

This series of patches fix a problem when adding memory in bad manner.
For example: for a x86_64 machine booted with "mem=400M" and with 2GiB
memory installed, following commands cause problem:

 # echo 0x40000000 > /sys/devices/system/memory/probe
[   28.613895] init_memory_mapping: [mem 0x40000000-0x47ffffff]
 # echo 0x48000000 > /sys/devices/system/memory/probe
[   28.693675] init_memory_mapping: [mem 0x48000000-0x4fffffff]
 # echo online_movable > /sys/devices/system/memory/memory9/state
 # echo 0x50000000 > /sys/devices/system/memory/probe 
[   29.084090] init_memory_mapping: [mem 0x50000000-0x57ffffff]
 # echo 0x58000000 > /sys/devices/system/memory/probe 
[   29.151880] init_memory_mapping: [mem 0x58000000-0x5fffffff]
 # echo online_movable > /sys/devices/system/memory/memory11/state
 # echo online> /sys/devices/system/memory/memory8/state
 # echo online> /sys/devices/system/memory/memory10/state
 # echo offline> /sys/devices/system/memory/memory9/state
[   30.558819] Offlined Pages 32768
 # free
             total       used       free     shared    buffers     cached
Mem:        780588 18014398509432020     830552          0          0      51180
-/+ buffers/cache: 18014398509380840     881732
Swap:            0          0          0

This is because the above commands probe higher memory after online a
section with online_movable, which causes ZONE_HIGHMEM (or ZONE_NORMAL
for systems without ZONE_HIGHMEM) overlaps ZONE_MOVABLE.

After the second online_movable, the problem can be observed from
zoneinfo:

 # cat /proc/zoneinfo
...
Node 0, zone  Movable
  pages free     65491
        min      250
        low      312
        high     375
        scanned  0
        spanned  18446744073709518848
        present  65536
        managed  65536
...

This series of patches solve the problem by checking ZONE_MOVABLE when
choosing zone for new memory. If new memory is inside or higher than
ZONE_MOVABLE, makes it go there instead.


Wang Nan (5):
  memory-hotplug: x86_64: suitable memory should go to ZONE_MOVABLE
  memory-hotplug: x86_32: suitable memory should go to ZONE_MOVABLE
  memory-hotplug: ia64: suitable memory should go to ZONE_MOVABLE
  memory-hotplug: sh: suitable memory should go to ZONE_MOVABLE
  memory-hotplug: powerpc: suitable memory should go to ZONE_MOVABLE

 arch/ia64/mm/init.c   |  7 +++++++
 arch/powerpc/mm/mem.c |  6 ++++++
 arch/sh/mm/init.c     | 13 ++++++++-----
 arch/x86/mm/init_32.c |  6 ++++++
 arch/x86/mm/init_64.c | 10 ++++++++--
 5 files changed, 35 insertions(+), 7 deletions(-)

-- 
1.8.4

^ permalink raw reply

* [PATCH 1/5] memory-hotplug: x86_64: suitable memory should go to ZONE_MOVABLE
From: Wang Nan @ 2014-07-18  7:55 UTC (permalink / raw)
  To: Ingo Molnar, Yinghai Lu, Mel Gorman, Andrew Morton
  Cc: linux-ia64, Pei Feiyue, linux-sh, x86, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <1405670163-53747-1-git-send-email-wangnan0@huawei.com>

This patch add new memory to ZONE_MOVABLE if movable zone is setup
and lower than newly added memory for x86_64.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 arch/x86/mm/init_64.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index df1a992..825915e 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -685,17 +685,23 @@ static void  update_end_of_memory_vars(u64 start, u64 size)
 }
 
 /*
- * Memory is added always to NORMAL zone. This means you will never get
- * additional DMA/DMA32 memory.
+ * Memory is added always to NORMAL or MOVABLE zone. This means you
+ * will never get additional DMA/DMA32 memory.
  */
 int arch_add_memory(int nid, u64 start, u64 size)
 {
 	struct pglist_data *pgdat = NODE_DATA(nid);
 	struct zone *zone = pgdat->node_zones + ZONE_NORMAL;
+	struct zone *movable_zone = pgdat->node_zones + ZONE_MOVABLE;
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	int ret;
 
+	if (!zone_is_empty(movable_zone))
+		if (zone_spans_pfn(movable_zone, start_pfn) ||
+				(zone_end_pfn(movable_zone) <= start_pfn))
+			zone = movable_zone;
+
 	init_memory_mapping(start, start + size);
 
 	ret = __add_pages(nid, zone, start_pfn, nr_pages);
-- 
1.8.4

^ permalink raw reply related

* [PATCH 2/5] memory-hotplug: x86_32: suitable memory should go to ZONE_MOVABLE
From: Wang Nan @ 2014-07-18  7:56 UTC (permalink / raw)
  To: Ingo Molnar, Yinghai Lu, Mel Gorman, Andrew Morton
  Cc: linux-ia64, Pei Feiyue, linux-sh, x86, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <1405670163-53747-1-git-send-email-wangnan0@huawei.com>

This patch add new memory to ZONE_MOVABLE if movable zone is setup
and lower than newly added memory for x86_32.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 arch/x86/mm/init_32.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index e395048..dd69833 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -826,9 +826,15 @@ int arch_add_memory(int nid, u64 start, u64 size)
 {
 	struct pglist_data *pgdata = NODE_DATA(nid);
 	struct zone *zone = pgdata->node_zones + ZONE_HIGHMEM;
+	struct zone *movable_zone = pgdat->node_zones + ZONE_MOVABLE;
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 
+	if (!zone_is_empty(movable_zone))
+		if (zone_spans_pfn(movable_zone, start_pfn) ||
+				(zone_end_pfn(movable_zone) <= start_pfn))
+			zone = movable_zone;
+
 	return __add_pages(nid, zone, start_pfn, nr_pages);
 }
 
-- 
1.8.4

^ permalink raw reply related

* Re: [PATCH V3 0/3] Add new PowerPC specific ELF core notes
From: Anshuman Khandual @ 2014-07-18  8:13 UTC (permalink / raw)
  To: Sam Bobroff, Michael Neuling, Benjamin Herrenschmidt
  Cc: james.hogan, avagin, Paul.Clothier, David S. Miller,
	Peter Zijlstra, Pedro Alves, oleg, Linux Kernel Mailing List,
	dhowells, Linux PPC dev, davej, akpm, tglx
In-Reply-To: <53C85AE6.7080600@au1.ibm.com>

On 07/18/2014 04:53 AM, Sam Bobroff wrote:
> On 17/07/14 21:14, Michael Neuling wrote:
>>
>> On Jul 17, 2014 9:11 PM, "Benjamin Herrenschmidt"
>> <benh@kernel.crashing.org <mailto:benh@kernel.crashing.org>> wrote:
>>>
>>>>>
>>>>>> Outstanding Issues
>>>>>> ==================
>>>>>> (1) Running DSCR register value inside a transaction does not
>> seem to be saved
>>>>>>     at thread.dscr when the process stops for ptrace examination.
>>>>>
>>>>> Hey Ben,
>>>>>
>>>>> Any updates on this patch series ?
>>>>
>>>> Ben,
>>>>
>>>> Any updates on this patch series ?
>>>
>>> I haven't had a chance to review yet, I was hoping somebody else would..
>>>
>>> Have you made any progress vs. the DSCR outstanding issue mentioned
>>> above ?
>>
>> The DSCR issue should be resolved with Sam Bobroff's recent  DSCR
>> fixes.  I've not tested them though.
>>
>> Actually... Sam did you review this series?
>>
>> Mikey
>>
> 
> I did, and applying "powerpc: Correct DSCR during TM context switch"
> corrected the DSCR value in the test program (the one in the patch notes
> for this series).
> 
> (In fact, IIRC, the reason for my patch set was the bug exposed by this
> one ;-)

Yeah the test program worked correctly with the fix from Sam. The first patch
is a generic code change which Pedro had reviewed before. The second and third
patches are powerpc specific.

^ permalink raw reply

* Re: [PATCH v4 1/2] Split out struct kvmppc_vcore creation to separate function
From: Paul Mackerras @ 2014-07-18  7:47 UTC (permalink / raw)
  To: Stewart Smith; +Cc: linuxppc-dev, Alexander Graf, kvm-ppc
In-Reply-To: <1405657123-20087-2-git-send-email-stewart@linux.vnet.ibm.com>

On Fri, Jul 18, 2014 at 02:18:42PM +1000, Stewart Smith wrote:
> No code changes, just split it out to a function so that with the addition
> of micro partition prefetch buffer allocation (in subsequent patch) looks
> neater and doesn't require excessive indentation.
> 
> Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

Acked-by: Paul Mackerras <paulus@samba.org>

^ permalink raw reply

* Re: [PATCH v4 2/2] Use the POWER8 Micro Partition Prefetch Engine in KVM HV on POWER8
From: Paul Mackerras @ 2014-07-18  7:48 UTC (permalink / raw)
  To: Stewart Smith; +Cc: linuxppc-dev, Alexander Graf, kvm-ppc
In-Reply-To: <1405657123-20087-3-git-send-email-stewart@linux.vnet.ibm.com>

On Fri, Jul 18, 2014 at 02:18:43PM +1000, Stewart Smith wrote:
> The POWER8 processor has a Micro Partition Prefetch Engine, which is
> a fancy way of saying "has way to store and load contents of L2 or
> L2+MRU way of L3 cache". We initiate the storing of the log (list of
> addresses) using the logmpp instruction and start restore by writing
> to a SPR.
> 
> The logmpp instruction takes parameters in a single 64bit register:
> - starting address of the table to store log of L2/L2+L3 cache contents
>   - 32kb for L2
>   - 128kb for L2+L3
>   - Aligned relative to maximum size of the table (32kb or 128kb)
> - Log control (no-op, L2 only, L2 and L3, abort logout)
> 
> We should abort any ongoing logging before initiating one.
> 
> To initiate restore, we write to the MPPR SPR. The format of what to write
> to the SPR is similar to the logmpp instruction parameter:
> - starting address of the table to read from (same alignment requirements)
> - table size (no data, until end of table)
> - prefetch rate (from fastest possible to slower. about every 8, 16, 24 or
>   32 cycles)
> 
> The idea behind loading and storing the contents of L2/L3 cache is to
> reduce memory latency in a system that is frequently swapping vcores on
> a physical CPU.
> 
> The best case scenario for doing this is when some vcores are doing very
> cache heavy workloads. The worst case is when they have about 0 cache hits,
> so we just generate needless memory operations.
> 
> This implementation just does L2 store/load. In my benchmarks this proves
> to be useful.
> 
> Benchmark 1:
>  - 16 core POWER8
>  - 3x Ubuntu 14.04LTS guests (LE) with 8 VCPUs each
>  - No split core/SMT
>  - two guests running sysbench memory test.
>    sysbench --test=memory --num-threads=8 run
>  - one guest running apache bench (of default HTML page)
>    ab -n 490000 -c 400 http://localhost/
> 
> This benchmark aims to measure performance of real world application (apache)
> where other guests are cache hot with their own workloads. The sysbench memory
> benchmark does pointer sized writes to a (small) memory buffer in a loop.
> 
> In this benchmark with this patch I can see an improvement both in requests
> per second (~5%) and in mean and median response times (again, about 5%).
> The spread of minimum and maximum response times were largely unchanged.
> 
> benchmark 2:
>  - Same VM config as benchmark 1
>  - all three guests running sysbench memory benchmark
> 
> This benchmark aims to see if there is a positive or negative affect to this
> cache heavy benchmark. Although due to the nature of the benchmark (stores) we
> may not see a difference in performance, but rather hopefully an improvement
> in consistency of performance (when vcore switched in, don't have to wait
> many times for cachelines to be pulled in)
> 
> The results of this benchmark are improvements in consistency of performance
> rather than performance itself. With this patch, the few outliers in duration
> go away and we get more consistent performance in each guest.
> 
> benchmark 3:
>  - same 3 guests and CPU configuration as benchmark 1 and 2.
>  - two idle guests
>  - 1 guest running STREAM benchmark
> 
> This scenario also saw performance improvement with this patch. On Copy and
> Scale workloads from STREAM, I got 5-6% improvement with this patch. For
> Add and triad, it was around 10% (or more).
> 
> benchmark 4:
>  - same 3 guests as previous benchmarks
>  - two guests running sysbench --memory, distinctly different cache heavy
>    workload
>  - one guest running STREAM benchmark.
> 
> Similar improvements to benchmark 3.
> 
> benchmark 5:
>  - 1 guest, 8 VCPUs, Ubuntu 14.04
>  - Host configured with split core (SMT8, subcores-per-core=4)
>  - STREAM benchmark
> 
> In this benchmark, we see a 10-20% performance improvement across the board
> of STREAM benchmark results with this patch.
> 
> Based on preliminary investigation and microbenchmarks
> by Prerna Saxena <prerna@linux.vnet.ibm.com>
> 
> Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

Acked-by: Paul Mackerras <paulus@samba.org>

^ permalink raw reply

* Re: [PATCH] CMA: generalize CMA reserved area management functionality (fixup)
From: Marek Szyprowski @ 2014-07-18  7:33 UTC (permalink / raw)
  To: Andrew Morton
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Joonsoo Kim, Zhang Yanfei, linuxppc-dev,
	linux-arm-kernel
In-Reply-To: <20140717150615.32c48786b6bdbc880bdc5ed4@linux-foundation.org>

Hello,

On 2014-07-18 00:06, Andrew Morton wrote:
> On Thu, 17 Jul 2014 11:36:07 +0200 Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
>> MAX_CMA_AREAS is used by other subsystems (i.e. arch/arm/mm/dma-mapping.c),
>> so we need to provide correct definition even if CMA is disabled.
>> This patch fixes this issue.
>>
>> Reported-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   include/linux/cma.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/include/linux/cma.h b/include/linux/cma.h
>> index 9a18a2b1934c..c077635cad76 100644
>> --- a/include/linux/cma.h
>> +++ b/include/linux/cma.h
>> @@ -5,7 +5,11 @@
>>    * There is always at least global CMA area and a few optional
>>    * areas configured in kernel .config.
>>    */
>> +#ifdef CONFIG_CMA
>>   #define MAX_CMA_AREAS	(1 + CONFIG_CMA_AREAS)
>> +#else
>> +#define MAX_CMA_AREAS	(0)
>> +#endif
>>   
>>   struct cma;
> Joonsoo already fixed this up, a bit differently:
> http://ozlabs.org/~akpm/mmots/broken-out/cma-generalize-cma-reserved-area-management-functionality-fix.patch
>
> Which approach makes more sense?

CMA_AREAS depends on CMA being enabled, so both approaches works exactly
the same way. Please keep Joonsoo's patch and just ignore mine to avoid
confusing others by disappearing patches. I'm sorry that I've missed it
before sending mine.

> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Subject: CMA: fix ARM build failure related to MAX_CMA_AREAS definition
>
> If CMA is disabled, CONFIG_CMA_AREAS isn't defined so compile error
> happens. To fix it, define MAX_CMA_AREAS if CONFIG_CMA_AREAS
> isn't defined.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>   include/linux/cma.h |    6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff -puN include/linux/cma.h~cma-generalize-cma-reserved-area-management-functionality-fix include/linux/cma.h
> --- a/include/linux/cma.h~cma-generalize-cma-reserved-area-management-functionality-fix
> +++ a/include/linux/cma.h
> @@ -5,8 +5,14 @@
>    * There is always at least global CMA area and a few optional
>    * areas configured in kernel .config.
>    */
> +#ifdef CONFIG_CMA_AREAS
>   #define MAX_CMA_AREAS	(1 + CONFIG_CMA_AREAS)
>   
> +#else
> +#define MAX_CMA_AREAS	(0)
> +
> +#endif
> +
>   struct cma;
>   
>   extern phys_addr_t cma_get_base(struct cma *cma);
> _

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply

* [PATCH v13 4/8] powerpc: add pmd_[dirty|mkclean] for THP
From: Minchan Kim @ 2014-07-18  6:53 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jason Evans, Rik van Riel, Minchan Kim, Aneesh Kumar K.V,
	Linux API, Hugh Dickins, linux-kernel, linux-mm, Zhang Yanfei,
	Michael Kerrisk, KOSAKI Motohiro, Johannes Weiner,
	Kirill A. Shutemov, linuxppc-dev, Paul Mackerras, Mel Gorman
In-Reply-To: <1405666386-15095-1-git-send-email-minchan@kernel.org>

MADV_FREE needs pmd_dirty and pmd_mkclean for detecting recent
overwrite of the contents since MADV_FREE syscall is called for
THP page.

This patch adds pmd_dirty and pmd_mkclean for THP page MADV_FREE
support.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 arch/powerpc/include/asm/pgtable-ppc64.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/include/asm/pgtable-ppc64.h b/arch/powerpc/include/asm/pgtable-ppc64.h
index eb9261024f51..c9a4bbe8e179 100644
--- a/arch/powerpc/include/asm/pgtable-ppc64.h
+++ b/arch/powerpc/include/asm/pgtable-ppc64.h
@@ -468,9 +468,11 @@ static inline pte_t *pmdp_ptep(pmd_t *pmd)
 
 #define pmd_pfn(pmd)		pte_pfn(pmd_pte(pmd))
 #define pmd_young(pmd)		pte_young(pmd_pte(pmd))
+#define pmd_dirty(pmd)		pte_dirty(pmd_pte(pmd))
 #define pmd_mkold(pmd)		pte_pmd(pte_mkold(pmd_pte(pmd)))
 #define pmd_wrprotect(pmd)	pte_pmd(pte_wrprotect(pmd_pte(pmd)))
 #define pmd_mkdirty(pmd)	pte_pmd(pte_mkdirty(pmd_pte(pmd)))
+#define pmd_mkclean(pmd)	pte_pmd(pte_mkclean(pmd_pte(pmd)))
 #define pmd_mkyoung(pmd)	pte_pmd(pte_mkyoung(pmd_pte(pmd)))
 #define pmd_mkwrite(pmd)	pte_pmd(pte_mkwrite(pmd_pte(pmd)))
 
-- 
2.0.0

^ permalink raw reply related

* Re: [PATCH] powerpc: Add cputable definition for POWER8 DD1
From: Michael Neuling @ 2014-07-18  5:10 UTC (permalink / raw)
  To: Joel Stanley; +Cc: paulus, anton, preeti, linuxppc-dev
In-Reply-To: <1405649497-679-1-git-send-email-joel@jms.id.au>

On Fri, 2014-07-18 at 11:41 +0930, Joel Stanley wrote:
> These processors do not currently support doorbell IPIs, so remove them
> from the feature list if we are at DD 1.xx for the 0x004d part.

We GAed with DD2.1 and generally we don't upstream anything that isn't
GAed.  Plus, if you wanna go down this path, you are going to have to
fix a lot more bugs than this one (not that our early chips had bugs or
anything). =20

I suggested you crush your crappy DD1 parts into cubes and ask for some
shiny new DD2.1 parts.

> This fixes a regression caused by d4e58e5928f8 (powerpc/powernv: Enable
> POWER8 doorbell IPIs). With that patch the kernel would hang at boot
> when calling smp_call_function_many, as the doorbell would not be
> received by the target CPUs:
>=20
>   .smp_call_function_many+0x2bc/0x3c0 (unreliable)
>   .on_each_cpu_mask+0x30/0x100
>   .cpuidle_register_driver+0x158/0x1a0
>   .cpuidle_register+0x2c/0x110
>   .powernv_processor_idle_init+0x23c/0x2c0
>   .do_one_initcall+0xd4/0x260
>   .kernel_init_freeable+0x25c/0x33c
>   .kernel_init+0x1c/0x120
>   .ret_from_kernel_thread+0x58/0x7c

Humm, well doorbells worked on DD1 so i'm not sure this is entirely your
problem.  There was an issue related to powersave that Ian posted a fix
for but we never needed it as the issue was fixed before GA.

http://patchwork.ozlabs.org/patch/240338/

So NAK again.

Mikey

> Fixes: d4e58e5928f8 (powerpc/powernv: Enable POWER8 doorbell IPIs)
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  arch/powerpc/include/asm/cputable.h |  1 +
>  arch/powerpc/kernel/cputable.c      | 20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+)
>=20
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/a=
sm/cputable.h
> index bc23477..0fdd7ee 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -447,6 +447,7 @@ extern const char *powerpc_base_platform;
>  	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
>  	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP)
>  #define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)
> +#define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)
>  #define CPU_FTRS_CELL	(CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
>  	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
>  	    CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
> diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputabl=
e.c
> index 965291b..0c15764 100644
> --- a/arch/powerpc/kernel/cputable.c
> +++ b/arch/powerpc/kernel/cputable.c
> @@ -527,6 +527,26 @@ static struct cpu_spec __initdata cpu_specs[] =3D {
>  		.machine_check_early	=3D __machine_check_early_realmode_p8,
>  		.platform		=3D "power8",
>  	},
> +	{	/* Power8 DD1: Does not support doorbell IPIs */
> +		.pvr_mask		=3D 0xffffff00,
> +		.pvr_value		=3D 0x004d0100,
> +		.cpu_name		=3D "POWER8 (raw)",
> +		.cpu_features		=3D CPU_FTRS_POWER8_DD1,
> +		.cpu_user_features	=3D COMMON_USER_POWER8,
> +		.cpu_user_features2	=3D COMMON_USER2_POWER8,
> +		.mmu_features		=3D MMU_FTRS_POWER8,
> +		.icache_bsize		=3D 128,
> +		.dcache_bsize		=3D 128,
> +		.num_pmcs		=3D 6,
> +		.pmc_type		=3D PPC_PMC_IBM,
> +		.oprofile_cpu_type	=3D "ppc64/power8",
> +		.oprofile_type		=3D PPC_OPROFILE_INVALID,
> +		.cpu_setup		=3D __setup_cpu_power8,
> +		.cpu_restore		=3D __restore_cpu_power8,
> +		.flush_tlb		=3D __flush_tlb_power8,
> +		.machine_check_early	=3D __machine_check_early_realmode_p8,
> +		.platform		=3D "power8",
> +	},
>  	{	/* Power8 */
>  		.pvr_mask		=3D 0xffff0000,
>  		.pvr_value		=3D 0x004d0000,

^ permalink raw reply

* Fix mes in crash.c
From: Nick Krause @ 2014-07-18  4:27 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Peter Zijlstra, linux-kernel@vger.kernel.org, paul.gortmaker,
	Paul Mackerras, Paul McKenney, linuxppc-dev, Ingo Molnar

Hey again Ben,
I am hitting quite a few fix mes in this file. I am wondering how you
would like me to fix them.
Cheers Nick

^ permalink raw reply

* [PATCH v4 1/2] Split out struct kvmppc_vcore creation to separate function
From: Stewart Smith @ 2014-07-18  4:18 UTC (permalink / raw)
  To: linuxppc-dev, paulus, kvm-ppc, Alexander Graf; +Cc: Stewart Smith
In-Reply-To: <1405657123-20087-1-git-send-email-stewart@linux.vnet.ibm.com>

No code changes, just split it out to a function so that with the addition
of micro partition prefetch buffer allocation (in subsequent patch) looks
neater and doesn't require excessive indentation.

Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c |   31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 7a12edb..cfe14f8 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1228,6 +1228,26 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
 	return r;
 }
 
+static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
+{
+	struct kvmppc_vcore *vcore;
+
+	vcore = kzalloc(sizeof(struct kvmppc_vcore), GFP_KERNEL);
+
+	if (vcore == NULL)
+		return NULL;
+
+	INIT_LIST_HEAD(&vcore->runnable_threads);
+	spin_lock_init(&vcore->lock);
+	init_waitqueue_head(&vcore->wq);
+	vcore->preempt_tb = TB_NIL;
+	vcore->lpcr = kvm->arch.lpcr;
+	vcore->first_vcpuid = core * threads_per_subcore;
+	vcore->kvm = kvm;
+
+	return vcore;
+}
+
 static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
 						   unsigned int id)
 {
@@ -1279,16 +1299,7 @@ static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
 	mutex_lock(&kvm->lock);
 	vcore = kvm->arch.vcores[core];
 	if (!vcore) {
-		vcore = kzalloc(sizeof(struct kvmppc_vcore), GFP_KERNEL);
-		if (vcore) {
-			INIT_LIST_HEAD(&vcore->runnable_threads);
-			spin_lock_init(&vcore->lock);
-			init_waitqueue_head(&vcore->wq);
-			vcore->preempt_tb = TB_NIL;
-			vcore->lpcr = kvm->arch.lpcr;
-			vcore->first_vcpuid = core * threads_per_subcore;
-			vcore->kvm = kvm;
-		}
+		vcore = kvmppc_vcore_create(kvm, core);
 		kvm->arch.vcores[core] = vcore;
 		kvm->arch.online_vcores++;
 	}
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v4 2/2] Use the POWER8 Micro Partition Prefetch Engine in KVM HV on POWER8
From: Stewart Smith @ 2014-07-18  4:18 UTC (permalink / raw)
  To: linuxppc-dev, paulus, kvm-ppc, Alexander Graf; +Cc: Stewart Smith
In-Reply-To: <1405657123-20087-1-git-send-email-stewart@linux.vnet.ibm.com>

The POWER8 processor has a Micro Partition Prefetch Engine, which is
a fancy way of saying "has way to store and load contents of L2 or
L2+MRU way of L3 cache". We initiate the storing of the log (list of
addresses) using the logmpp instruction and start restore by writing
to a SPR.

The logmpp instruction takes parameters in a single 64bit register:
- starting address of the table to store log of L2/L2+L3 cache contents
  - 32kb for L2
  - 128kb for L2+L3
  - Aligned relative to maximum size of the table (32kb or 128kb)
- Log control (no-op, L2 only, L2 and L3, abort logout)

We should abort any ongoing logging before initiating one.

To initiate restore, we write to the MPPR SPR. The format of what to write
to the SPR is similar to the logmpp instruction parameter:
- starting address of the table to read from (same alignment requirements)
- table size (no data, until end of table)
- prefetch rate (from fastest possible to slower. about every 8, 16, 24 or
  32 cycles)

The idea behind loading and storing the contents of L2/L3 cache is to
reduce memory latency in a system that is frequently swapping vcores on
a physical CPU.

The best case scenario for doing this is when some vcores are doing very
cache heavy workloads. The worst case is when they have about 0 cache hits,
so we just generate needless memory operations.

This implementation just does L2 store/load. In my benchmarks this proves
to be useful.

Benchmark 1:
 - 16 core POWER8
 - 3x Ubuntu 14.04LTS guests (LE) with 8 VCPUs each
 - No split core/SMT
 - two guests running sysbench memory test.
   sysbench --test=memory --num-threads=8 run
 - one guest running apache bench (of default HTML page)
   ab -n 490000 -c 400 http://localhost/

This benchmark aims to measure performance of real world application (apache)
where other guests are cache hot with their own workloads. The sysbench memory
benchmark does pointer sized writes to a (small) memory buffer in a loop.

In this benchmark with this patch I can see an improvement both in requests
per second (~5%) and in mean and median response times (again, about 5%).
The spread of minimum and maximum response times were largely unchanged.

benchmark 2:
 - Same VM config as benchmark 1
 - all three guests running sysbench memory benchmark

This benchmark aims to see if there is a positive or negative affect to this
cache heavy benchmark. Although due to the nature of the benchmark (stores) we
may not see a difference in performance, but rather hopefully an improvement
in consistency of performance (when vcore switched in, don't have to wait
many times for cachelines to be pulled in)

The results of this benchmark are improvements in consistency of performance
rather than performance itself. With this patch, the few outliers in duration
go away and we get more consistent performance in each guest.

benchmark 3:
 - same 3 guests and CPU configuration as benchmark 1 and 2.
 - two idle guests
 - 1 guest running STREAM benchmark

This scenario also saw performance improvement with this patch. On Copy and
Scale workloads from STREAM, I got 5-6% improvement with this patch. For
Add and triad, it was around 10% (or more).

benchmark 4:
 - same 3 guests as previous benchmarks
 - two guests running sysbench --memory, distinctly different cache heavy
   workload
 - one guest running STREAM benchmark.

Similar improvements to benchmark 3.

benchmark 5:
 - 1 guest, 8 VCPUs, Ubuntu 14.04
 - Host configured with split core (SMT8, subcores-per-core=4)
 - STREAM benchmark

In this benchmark, we see a 10-20% performance improvement across the board
of STREAM benchmark results with this patch.

Based on preliminary investigation and microbenchmarks
by Prerna Saxena <prerna@linux.vnet.ibm.com>

Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

--
changes since v3:
- use kvmppc namespace
- MPP_BUFFER_ORDER of 3 not 4, as we only need 32k and it's already 32k aligned
- split out kvmppc_vcore_create in separate patch
- give a variable a better name: s/tmp/mpp_addr/
- logmpp becomes static inline function

changes since v2:
- based on feedback from Alexander Graf:
  - move save and restore of cache to separate functions
  - move allocation of mpp_buffer to vcore creation
  - get_free_pages() does actually allocate pages aligned to order
    (Mel Gorman confirms)
  - make SPR and logmpp parameters a bit less magic, especially around abort

changes since v1:
- s/mppe/mpp_buffer/
- add MPP_BUFFER_ORDER define.
---
 arch/powerpc/include/asm/cache.h      |    7 ++++
 arch/powerpc/include/asm/kvm_host.h   |    2 ++
 arch/powerpc/include/asm/ppc-opcode.h |   17 ++++++++++
 arch/powerpc/include/asm/reg.h        |    1 +
 arch/powerpc/kvm/book3s_hv.c          |   57 ++++++++++++++++++++++++++++++++-
 5 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cache.h b/arch/powerpc/include/asm/cache.h
index ed0afc1..34a05a1 100644
--- a/arch/powerpc/include/asm/cache.h
+++ b/arch/powerpc/include/asm/cache.h
@@ -3,6 +3,7 @@
 
 #ifdef __KERNEL__
 
+#include <asm/reg.h>
 
 /* bytes per L1 cache line */
 #if defined(CONFIG_8xx) || defined(CONFIG_403GCX)
@@ -39,6 +40,12 @@ struct ppc64_caches {
 };
 
 extern struct ppc64_caches ppc64_caches;
+
+static inline void logmpp(u64 x)
+{
+	asm volatile(PPC_LOGMPP(R1) : : "r" (x));
+}
+
 #endif /* __powerpc64__ && ! __ASSEMBLY__ */
 
 #if defined(__ASSEMBLY__)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index bb66d8b..5add9ac 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -305,6 +305,8 @@ struct kvmppc_vcore {
 	u32 arch_compat;
 	ulong pcr;
 	ulong dpdes;		/* doorbell state (POWER8) */
+	void *mpp_buffer; /* Micro Partition Prefetch buffer */
+	bool mpp_buffer_is_valid;
 };
 
 #define VCORE_ENTRY_COUNT(vc)	((vc)->entry_exit_count & 0xff)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 3132bb9..c636841 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -139,6 +139,7 @@
 #define PPC_INST_ISEL			0x7c00001e
 #define PPC_INST_ISEL_MASK		0xfc00003e
 #define PPC_INST_LDARX			0x7c0000a8
+#define PPC_INST_LOGMPP			0x7c0007e4
 #define PPC_INST_LSWI			0x7c0004aa
 #define PPC_INST_LSWX			0x7c00042a
 #define PPC_INST_LWARX			0x7c000028
@@ -275,6 +276,20 @@
 #define __PPC_EH(eh)	0
 #endif
 
+/* POWER8 Micro Partition Prefetch (MPP) parameters */
+/* Address mask is common for LOGMPP instruction and MPPR SPR */
+#define PPC_MPPE_ADDRESS_MASK 0xffffffffc000
+
+/* Bits 60 and 61 of MPP SPR should be set to one of the following */
+/* Aborting the fetch is indeed setting 00 in the table size bits */
+#define PPC_MPPR_FETCH_ABORT (0x0ULL << 60)
+#define PPC_MPPR_FETCH_WHOLE_TABLE (0x2ULL << 60)
+
+/* Bits 54 and 55 of register for LOGMPP instruction should be set to: */
+#define PPC_LOGMPP_LOG_L2 (0x02ULL << 54)
+#define PPC_LOGMPP_LOG_L2L3 (0x01ULL << 54)
+#define PPC_LOGMPP_LOG_ABORT (0x03ULL << 54)
+
 /* Deal with instructions that older assemblers aren't aware of */
 #define	PPC_DCBAL(a, b)		stringify_in_c(.long PPC_INST_DCBAL | \
 					__PPC_RA(a) | __PPC_RB(b))
@@ -283,6 +298,8 @@
 #define PPC_LDARX(t, a, b, eh)	stringify_in_c(.long PPC_INST_LDARX | \
 					___PPC_RT(t) | ___PPC_RA(a) | \
 					___PPC_RB(b) | __PPC_EH(eh))
+#define PPC_LOGMPP(b)		stringify_in_c(.long PPC_INST_LOGMPP | \
+					__PPC_RB(b))
 #define PPC_LWARX(t, a, b, eh)	stringify_in_c(.long PPC_INST_LWARX | \
 					___PPC_RT(t) | ___PPC_RA(a) | \
 					___PPC_RB(b) | __PPC_EH(eh))
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index bffd89d..68cc9f9 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -225,6 +225,7 @@
 #define   CTRL_TE	0x00c00000	/* thread enable */
 #define   CTRL_RUNLATCH	0x1
 #define SPRN_DAWR	0xB4
+#define SPRN_MPPR	0xB8	/* Micro Partition Prefetch Register */
 #define SPRN_RPR	0xBA	/* Relative Priority Register */
 #define SPRN_CIABR	0xBB
 #define   CIABR_PRIV		0x3
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index cfe14f8..03b758c 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -35,6 +35,7 @@
 
 #include <asm/reg.h>
 #include <asm/cputable.h>
+#include <asm/cache.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
 #include <asm/uaccess.h>
@@ -67,6 +68,13 @@
 /* Used as a "null" value for timebase values */
 #define TB_NIL	(~(u64)0)
 
+#if defined(CONFIG_PPC_64K_PAGES)
+#define MPP_BUFFER_ORDER	0
+#elif defined(CONFIG_PPC_4K_PAGES)
+#define MPP_BUFFER_ORDER	3
+#endif
+
+
 static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
 static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
 
@@ -1245,6 +1253,13 @@ static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
 	vcore->first_vcpuid = core * threads_per_subcore;
 	vcore->kvm = kvm;
 
+	vcore->mpp_buffer_is_valid = false;
+
+	if (cpu_has_feature(CPU_FTR_ARCH_207S))
+		vcore->mpp_buffer = (void *)__get_free_pages(
+			GFP_KERNEL|__GFP_ZERO,
+			MPP_BUFFER_ORDER);
+
 	return vcore;
 }
 
@@ -1511,6 +1526,33 @@ static int on_primary_thread(void)
 	return 1;
 }
 
+static void kvmppc_start_saving_l2_cache(struct kvmppc_vcore *vc)
+{
+	phys_addr_t phy_addr, mpp_addr;
+
+	phy_addr = (phys_addr_t)virt_to_phys(vc->mpp_buffer);
+	mpp_addr = phy_addr & PPC_MPPE_ADDRESS_MASK;
+
+	mtspr(SPRN_MPPR, mpp_addr | PPC_MPPR_FETCH_ABORT);
+	logmpp(mpp_addr | PPC_LOGMPP_LOG_L2);
+
+	vc->mpp_buffer_is_valid = true;
+}
+
+static void kvmppc_start_restoring_l2_cache(const struct kvmppc_vcore *vc)
+{
+	phys_addr_t phy_addr, mpp_addr;
+
+	phy_addr = virt_to_phys(vc->mpp_buffer);
+	mpp_addr = phy_addr & PPC_MPPE_ADDRESS_MASK;
+
+	/* We must abort any in-progress save operations to ensure
+	 * the table is valid so that prefetch engine knows when to
+	 * stop prefetching. */
+	logmpp(mpp_addr | PPC_LOGMPP_LOG_ABORT);
+	mtspr(SPRN_MPPR, mpp_addr | PPC_MPPR_FETCH_WHOLE_TABLE);
+}
+
 /*
  * Run a set of guest threads on a physical core.
  * Called with vc->lock held.
@@ -1588,9 +1630,16 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
 
 	srcu_idx = srcu_read_lock(&vc->kvm->srcu);
 
+	if (vc->mpp_buffer_is_valid)
+		kvmppc_start_restoring_l2_cache(vc);
+
 	__kvmppc_vcore_entry();
 
 	spin_lock(&vc->lock);
+
+	if (vc->mpp_buffer)
+		kvmppc_start_saving_l2_cache(vc);
+
 	/* disable sending of IPIs on virtual external irqs */
 	list_for_each_entry(vcpu, &vc->runnable_threads, arch.run_list)
 		vcpu->cpu = -1;
@@ -2334,8 +2383,14 @@ static void kvmppc_free_vcores(struct kvm *kvm)
 {
 	long int i;
 
-	for (i = 0; i < KVM_MAX_VCORES; ++i)
+	for (i = 0; i < KVM_MAX_VCORES; ++i) {
+		if (kvm->arch.vcores[i] && kvm->arch.vcores[i]->mpp_buffer) {
+			struct kvmppc_vcore *vc = kvm->arch.vcores[i];
+			free_pages((unsigned long)vc->mpp_buffer,
+				   MPP_BUFFER_ORDER);
+		}
 		kfree(kvm->arch.vcores[i]);
+	}
 	kvm->arch.online_vcores = 0;
 }
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v4 0/2] Use the POWER8 Micro Partition Prefetch Engine in KVM HV
From: Stewart Smith @ 2014-07-18  4:18 UTC (permalink / raw)
  To: linuxppc-dev, paulus, kvm-ppc, Alexander Graf; +Cc: Stewart Smith
In-Reply-To: <1405567197-23333-1-git-send-email-stewart@linux.vnet.ibm.com>

changes since v3:
- use kvmppc namespace
- MPP_BUFFER_ORDER of 3 not 4, as we only need 32k and it's already 32k aligned
- split out kvmppc_vcore_create in separate patch
- give a variable a better name: s/tmp/mpp_addr/
- logmpp becomes static inline function

Stewart Smith (2):
  Split out struct kvmppc_vcore creation to separate function
  Use the POWER8 Micro Partition Prefetch Engine in KVM HV on POWER8

 arch/powerpc/include/asm/cache.h      |    7 +++
 arch/powerpc/include/asm/kvm_host.h   |    2 +
 arch/powerpc/include/asm/ppc-opcode.h |   17 +++++++
 arch/powerpc/include/asm/reg.h        |    1 +
 arch/powerpc/kvm/book3s_hv.c          |   88 ++++++++++++++++++++++++++++-----
 5 files changed, 104 insertions(+), 11 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* Re: [PATCH v3] Use the POWER8 Micro Partition Prefetch Engine in KVM HV on POWER8
From: Stewart Smith @ 2014-07-18  4:10 UTC (permalink / raw)
  To: Alexander Graf, linuxppc-dev, paulus, kvm-ppc
In-Reply-To: <53C78161.9040700@suse.de>

Alexander Graf <agraf@suse.de> writes:
>> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> index 1eaea2d..5769497 100644
>> --- a/arch/powerpc/include/asm/kvm_host.h
>> +++ b/arch/powerpc/include/asm/kvm_host.h
>> @@ -305,6 +305,8 @@ struct kvmppc_vcore {
>>   	u32 arch_compat;
>>   	ulong pcr;
>>   	ulong dpdes;		/* doorbell state (POWER8) */
>> +	unsigned long mpp_buffer; /* Micro Partition Prefetch buffer */
>
> Just make this a void*?

get_free_pages returns an unsigned long and free_pages accepts an
unsigned long, so I was just avoiding the cast. Is the style in this
case to do void* rather than unsigned long and cast it everywhere?

In v4 of patch I've gone to void* anyway.

>> @@ -1516,6 +1540,37 @@ static int on_primary_thread(void)
>>   	return 1;
>>   }
>>   
>> +static void ppc_start_saving_l2_cache(struct kvmppc_vcore *vc)
>
> Please use the "kvmppc" name space.

ack, done.

>> +	phys_addr_t phy_addr, tmp;
>> +
>> +	phy_addr = (phys_addr_t)virt_to_phys((void *)vc->mpp_buffer);
>> +
>> +	tmp = phy_addr & PPC_MPPE_ADDRESS_MASK;
>
> I would prefer if you give the variable a more telling name.

ack.

>> +
>> +	mtspr(SPRN_MPPR, tmp | PPC_MPPR_FETCH_ABORT);
>> +
>> +	asm volatile(PPC_LOGMPP(R1) : : "r" (tmp | PPC_LOGMPP_LOG_L2));
>
> Can you move this asm() into a static inline function in generic code 
> somewhere?

okay. It seems the best place may be powerpc/include/asm/cache.h -
simply because it deals with cache things. I'm open to better
suggestions :)

>
>> +
>> +	vc->mpp_buffer_is_valid = true;
>
> Where does this ever get unset? And what point does this variable make? 
> Can't you just check on if (vc->mpp_buffer)?

The problem with having moved the memory allocation for mpp_buffer to
vcore setup is that we'll have vc->mpp_buffer != NULL but have some
random contents in it, so when we first start executing the vcore, we
shouldn't initiate prefetching (hence mpp_buffer_is_valid).

If we point the prefetch engine to random memory contents, we get the
most amazing array of incomprehensible illegal accesses :)

The aborting of saving the L2 contents before starting the reading back
in makes the hardware ensure the content of that buffer is finished
correctly.

The hardware docs don't describe the exact format of what it puts in the
buffer, just that there's an 'end of table' bit set in the last entry.

> Also, a single whitespace line between every instruction you do looks 
> weird ;). When you have the feeling that the code flow is weird enough 
> that you need empty lines between every real line, there's probably 
> something wrong in the code flow :).

ok, looks a bit better with logmpp as func rather than asm block.

^ permalink raw reply

* Re: [PATCH v3] Use the POWER8 Micro Partition Prefetch Engine in KVM HV on POWER8
From: Stewart Smith @ 2014-07-18  4:10 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev, Alexander Graf, kvm-ppc
In-Reply-To: <20140717235214.GB8513@iris.ozlabs.ibm.com>

Paul Mackerras <paulus@samba.org> writes:
> On Thu, Jul 17, 2014 at 01:19:57PM +1000, Stewart Smith wrote:
>
>> The POWER8 processor has a Micro Partition Prefetch Engine, which is
>> a fancy way of saying "has way to store and load contents of L2 or
>> L2+MRU way of L3 cache". We initiate the storing of the log (list of
>> addresses) using the logmpp instruction and start restore by writing
>> to a SPR.
>> 
>> The logmpp instruction takes parameters in a single 64bit register:
>> - starting address of the table to store log of L2/L2+L3 cache contents
>>   - 32kb for L2
>>   - 128kb for L2+L3
>>   - Aligned relative to maximum size of the table (32kb or 128kb)
>> - Log control (no-op, L2 only, L2 and L3, abort logout)
>> 
>> We should abort any ongoing logging before initiating one.
>
> Do we ever want to wait for ongoing logging to finish?

*Probably* not... but as far as I can see the hardware doesn't expose a
way to find out if there is any ongoing logging or a way to be notified
when it's done.

>> +#if defined(CONFIG_PPC_64K_PAGES)
>> +#define MPP_BUFFER_ORDER	0
>> +#elif defined(CONFIG_PPC_4K_PAGES)
>> +#define MPP_BUFFER_ORDER	4
>
> Why 4 not 3?  You only need 32kB, don't you?

Correct. whoops.

>> +static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
>> +{
>> +	struct kvmppc_vcore *vcore;
>> +
>> +	vcore = kzalloc(sizeof(struct kvmppc_vcore), GFP_KERNEL);
>> +
>> +	if (vcore == NULL)
>> +		return NULL;
>> +
>> +	INIT_LIST_HEAD(&vcore->runnable_threads);
>> +	spin_lock_init(&vcore->lock);
>> +	init_waitqueue_head(&vcore->wq);
>> +	vcore->preempt_tb = TB_NIL;
>> +	vcore->lpcr = kvm->arch.lpcr;
>> +	vcore->first_vcpuid = core * threads_per_core;
>> +	vcore->kvm = kvm;
>
> Is there a particular reason why you need to pull this code out into a
> separate function?  If so, it would be a little nicer if you did that
> in a separate patch, to make it easier to see that the code motion
> changes nothing.

ack, done.

>> @@ -1590,9 +1645,16 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
>>  
>>  	srcu_idx = srcu_read_lock(&vc->kvm->srcu);
>>  
>> +	if (vc->mpp_buffer_is_valid)
>> +		ppc_start_restoring_l2_cache(vc);
>> +
>>  	__kvmppc_vcore_entry();
>>  
>>  	spin_lock(&vc->lock);
>> +
>> +	if (vc->mpp_buffer)
>> +		ppc_start_saving_l2_cache(vc);
>
> I wonder if we would get better performance improvements if we kicked
> this off earlier, for instance before we save all the FP/VSX state and
> switch the MMU?  I guess that could be a subsequent patch.

Possibly, yes. Maybe something to look at in future patch, along with if
also doing the L3 save/restore is a benefit.

^ permalink raw reply

* RE: Allyesconfig Still Failing
From: Nick Krause @ 2014-07-18  3:23 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev, Paul Mackerras, linux-kernel@vger.kernel.org

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

Hey Ben,
Me and Steven have discussed that the allyesconfig build is still
failing.I seem to hitting this in the main branch too and not the
linux next tree .
Furthermore I will attach my log of my build failing as I am cross
compiling on an Intel Sandy Bridge i5 2500k at 4.5ghz. Perhaps we can
clean this up
for the next kernel stable release if we still have time before this
mergewindow.
Cheers Nick

[-- Attachment #2: allyesconfig --]
[-- Type: application/octet-stream, Size: 6723 bytes --]

lib/cpumask.c: In function ‘cpumask_set_cpu_local_first’:
lib/cpumask.c:194:25: warning: the address of ‘cpu_all_bits’ will always evaluate as ‘true’ [-Waddress]
  if (numa_node == -1 || !cpumask_of_node(numa_node)) {
                         ^
make[1]: *** No rule to make target `/media/nick/Nick's', needed by `kernel/x509_certificate_list'.  Stop.
make[1]: *** Waiting for unfinished jobs....
make: *** [kernel] Error 2
make: *** Waiting for unfinished jobs....
sound/soc/codecs/adau1977.c: In function ‘adau1977_probe’:
sound/soc/codecs/adau1977.c:971:3: warning: large integer implicitly truncated to unsigned type [-Woverflow]
   power_off_mask = ~ADAU1977_BLOCK_POWER_SAI_LDO_EN;
   ^
drivers/gpio/gpio-sch311x.c: In function ‘sch311x_gpio_probe’:
drivers/gpio/gpio-sch311x.c:286:18: warning: ignoring return value of ‘gpiochip_remove’, declared with attribute warn_unused_result [-Wunused-result]
   gpiochip_remove(&priv->blocks[i].chip);
                  ^
sound/soc/fsl/fsl_sai.c: In function ‘fsl_sai_trigger’:
sound/soc/fsl/fsl_sai.c:337:7: warning: large integer implicitly truncated to unsigned type [-Woverflow]
       ~FSL_SAI_CR2_SYNC);
       ^
sound/soc/fsl/imx-audmux.c: In function ‘audmux_read_file’:
sound/soc/fsl/imx-audmux.c:70:13: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  int port = (int)file->private_data;
             ^
sound/soc/fsl/imx-audmux.c: In function ‘audmux_debugfs_init’:
sound/soc/fsl/imx-audmux.c:162:7: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
       (void *)i, &audmux_debugfs_fops))
       ^
drivers/iio/adc/exynos_adc.c: In function ‘exynos_adc_get_version’:
drivers/iio/adc/exynos_adc.c:112:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  return (unsigned int)match->data;
         ^
drivers/input/joystick/analog.c:171:2: warning: #warning Precise timer not defined for this architecture. [-Wcpp]
 #warning Precise timer not defined for this architecture.
  ^
drivers/mfd/stmpe-i2c.c: In function ‘stmpe_i2c_probe’:
drivers/mfd/stmpe-i2c.c:88:13: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
   partnum = (int)of_id->data;
             ^
drivers/mfd/arizona-core.c: In function ‘arizona_of_get_type’:
drivers/mfd/arizona-core.c:505:10: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
   return (int)id->data;
          ^
drivers/mfd/tps65910.c: In function ‘tps65910_parse_dt’:
drivers/mfd/tps65910.c:404:14: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  *chip_id  = (int)match->data;
              ^
drivers/net/ethernet/amd/nmclan_cs.c: In function ‘nmclan_config’:
drivers/net/ethernet/amd/nmclan_cs.c:624:3: warning: ‘pcmcia_request_exclusive_irq’ is deprecated (declared at include/pcmcia/ds.h:213) [-Wdeprecated-declarations]
   ret = pcmcia_request_exclusive_irq(link, mace_interrupt);
   ^
drivers/regulator/tps6586x-regulator.c: In function ‘tps6586x_parse_regulator_dt’:
drivers/regulator/tps6586x-regulator.c:423:8: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
   id = (int)tps6586x_matches[i].driver_data;
        ^
drivers/spi/spi-atmel.c: In function ‘atmel_spi_setup’:
drivers/spi/spi-atmel.c:1021:13: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  npcs_pin = (unsigned int)spi->controller_data;
             ^
drivers/spi/spi-atmel.c: In function ‘atmel_spi_cleanup’:
drivers/spi/spi-atmel.c:1256:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  unsigned  gpio = (unsigned) spi->controller_data;
                   ^
drivers/staging/bcm/CmHost.c: In function ‘StoreCmControlResponseMessage’:
drivers/staging/bcm/CmHost.c:1502:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   (struct bcm_connect_mgr_params *) ntohl(
   ^
drivers/staging/bcm/CmHost.c:1545:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   (struct bcm_connect_mgr_params *) ntohl(
   ^
drivers/staging/bcm/CmHost.c:1563:3: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   (struct bcm_connect_mgr_params *) ntohl(
   ^
drivers/net/ethernet/ibm/ehea/ehea_main.c:106:28: warning: ‘ehea_module_device_table’ defined but not used [-Wunused-variable]
 static struct of_device_id ehea_module_device_table[] = {
                            ^
In file included from include/linux/kernel.h:12:0,
                 from include/linux/delay.h:10,
                 from drivers/net/ethernet/xilinx/ll_temac_main.c:30:
drivers/net/ethernet/xilinx/ll_temac_main.c: In function ‘temac_indirect_busywait’:
include/linux/typecheck.h:11:18: warning: comparison of distinct pointer types lacks a cast [enabled by default]
  (void)(&__dummy == &__dummy2); \
                  ^
include/linux/jiffies.h:109:3: note: in expansion of macro ‘typecheck’
   typecheck(unsigned long, b) && \
   ^
include/linux/jiffies.h:111:29: note: in expansion of macro ‘time_after_eq’
 #define time_before_eq(a,b) time_after_eq(b,a)
                             ^
drivers/net/ethernet/xilinx/ll_temac_main.c:78:7: note: in expansion of macro ‘time_before_eq’
   if (time_before_eq(end, jiffies)) {
       ^
drivers/net/ethernet/xilinx/ll_temac_main.c: In function ‘temac_start_xmit_done’:
drivers/net/ethernet/xilinx/ll_temac_main.c:633:22: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
    dev_kfree_skb_irq((struct sk_buff *)cur_p->app4);
                      ^
drivers/scsi/ips.c:210:2: warning: #warning "This driver has only been tested on the x86/ia64/x86_64 platforms" [-Wcpp]
 #warning "This driver has only been tested on the x86/ia64/x86_64 platforms"
  ^
drivers/scsi/fdomain.c:1774:29: warning: ‘fdomain_pci_tbl’ defined but not used [-Wunused-variable]
 static struct pci_device_id fdomain_pci_tbl[] = {
                             ^
drivers/scsi/hpsa.c: In function ‘hpsa_set_driver_support_bits’:
drivers/scsi/hpsa.c:6373:17: warning: ‘driver_support’ is used uninitialized in this function [-Wuninitialized]
  driver_support |= ENABLE_UNIT_ATTN;
                 ^
drivers/scsi/initio.c:131:29: warning: ‘i91u_pci_devices’ defined but not used [-Wunused-variable]
 static struct pci_device_id i91u_pci_devices[] = {
                             ^
drivers/staging/usbip/stub_dev.c:33:29: warning: ‘stub_table’ defined but not used [-Wunused-variable]
 static struct usb_device_id stub_table[] = {
                             ^

^ permalink raw reply

* RE: [PATCH 0/2] ASoC: fsl_sai: Two bug fixes for fsl_sai driver
From: Li.Xiubo @ 2014-07-18  2:38 UTC (permalink / raw)
  To: Nicolin Chen, broonie@kernel.org
  Cc: shengjiu.wang@freescale.com, timur@tabi.org,
	alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
	guangyu.chen@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <cover.1405603108.git.nicoleotsuka@gmail.com>

> -----Original Message-----
> From: Nicolin Chen [mailto:nicoleotsuka@gmail.com]
> Sent: Thursday, July 17, 2014 9:22 PM
> To: broonie@kernel.org
> Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; alsa-
> devel@alsa-project.org; timur@tabi.org; Xiubo Li-B47053; Wang Shengjiu-B0=
2247;
> Chen Guangyu-B42378
> Subject: [PATCH 0/2] ASoC: fsl_sai: Two bug fixes for fsl_sai driver
>=20
> Nicolin Chen (2):
>   ASoC: fsl_sai: Reset FIFOs after disabling TE/RE
>   ASoC: fsl_sai: Fix incorrect register writing in fsl_sai_isr()
>=20

Yes, they are all looks good to me.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>

Thanks,

BRs
Xiubo


>  sound/soc/fsl/fsl_sai.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>=20
> --
> 1.8.4

^ permalink raw reply

* [PATCH] powerpc: Add cputable definition for POWER8 DD1
From: Joel Stanley @ 2014-07-18  2:11 UTC (permalink / raw)
  To: benh; +Cc: mikey, paulus, anton, preeti, linuxppc-dev

These processors do not currently support doorbell IPIs, so remove them
from the feature list if we are at DD 1.xx for the 0x004d part.

This fixes a regression caused by d4e58e5928f8 (powerpc/powernv: Enable
POWER8 doorbell IPIs). With that patch the kernel would hang at boot
when calling smp_call_function_many, as the doorbell would not be
received by the target CPUs:

  .smp_call_function_many+0x2bc/0x3c0 (unreliable)
  .on_each_cpu_mask+0x30/0x100
  .cpuidle_register_driver+0x158/0x1a0
  .cpuidle_register+0x2c/0x110
  .powernv_processor_idle_init+0x23c/0x2c0
  .do_one_initcall+0xd4/0x260
  .kernel_init_freeable+0x25c/0x33c
  .kernel_init+0x1c/0x120
  .ret_from_kernel_thread+0x58/0x7c

Fixes: d4e58e5928f8 (powerpc/powernv: Enable POWER8 doorbell IPIs)
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/powerpc/include/asm/cputable.h |  1 +
 arch/powerpc/kernel/cputable.c      | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index bc23477..0fdd7ee 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -447,6 +447,7 @@ extern const char *powerpc_base_platform;
 	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
 	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP)
 #define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)
+#define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)
 #define CPU_FTRS_CELL	(CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
 	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
 	    CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 965291b..0c15764 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -527,6 +527,26 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.machine_check_early	= __machine_check_early_realmode_p8,
 		.platform		= "power8",
 	},
+	{	/* Power8 DD1: Does not support doorbell IPIs */
+		.pvr_mask		= 0xffffff00,
+		.pvr_value		= 0x004d0100,
+		.cpu_name		= "POWER8 (raw)",
+		.cpu_features		= CPU_FTRS_POWER8_DD1,
+		.cpu_user_features	= COMMON_USER_POWER8,
+		.cpu_user_features2	= COMMON_USER2_POWER8,
+		.mmu_features		= MMU_FTRS_POWER8,
+		.icache_bsize		= 128,
+		.dcache_bsize		= 128,
+		.num_pmcs		= 6,
+		.pmc_type		= PPC_PMC_IBM,
+		.oprofile_cpu_type	= "ppc64/power8",
+		.oprofile_type		= PPC_OPROFILE_INVALID,
+		.cpu_setup		= __setup_cpu_power8,
+		.cpu_restore		= __restore_cpu_power8,
+		.flush_tlb		= __flush_tlb_power8,
+		.machine_check_early	= __machine_check_early_realmode_p8,
+		.platform		= "power8",
+	},
 	{	/* Power8 */
 		.pvr_mask		= 0xffff0000,
 		.pvr_value		= 0x004d0000,
-- 
2.0.1

^ permalink raw reply related

* Re: [PATCH v3] Use the POWER8 Micro Partition Prefetch Engine in KVM HV on POWER8
From: Paul Mackerras @ 2014-07-17 23:52 UTC (permalink / raw)
  To: Stewart Smith; +Cc: linuxppc-dev, Alexander Graf, kvm-ppc
In-Reply-To: <1405567197-23333-1-git-send-email-stewart@linux.vnet.ibm.com>

On Thu, Jul 17, 2014 at 01:19:57PM +1000, Stewart Smith wrote:

> The POWER8 processor has a Micro Partition Prefetch Engine, which is
> a fancy way of saying "has way to store and load contents of L2 or
> L2+MRU way of L3 cache". We initiate the storing of the log (list of
> addresses) using the logmpp instruction and start restore by writing
> to a SPR.
> 
> The logmpp instruction takes parameters in a single 64bit register:
> - starting address of the table to store log of L2/L2+L3 cache contents
>   - 32kb for L2
>   - 128kb for L2+L3
>   - Aligned relative to maximum size of the table (32kb or 128kb)
> - Log control (no-op, L2 only, L2 and L3, abort logout)
> 
> We should abort any ongoing logging before initiating one.

Do we ever want to wait for ongoing logging to finish?

[snip]

> +#if defined(CONFIG_PPC_64K_PAGES)
> +#define MPP_BUFFER_ORDER	0
> +#elif defined(CONFIG_PPC_4K_PAGES)
> +#define MPP_BUFFER_ORDER	4

Why 4 not 3?  You only need 32kB, don't you?

> +static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
> +{
> +	struct kvmppc_vcore *vcore;
> +
> +	vcore = kzalloc(sizeof(struct kvmppc_vcore), GFP_KERNEL);
> +
> +	if (vcore == NULL)
> +		return NULL;
> +
> +	INIT_LIST_HEAD(&vcore->runnable_threads);
> +	spin_lock_init(&vcore->lock);
> +	init_waitqueue_head(&vcore->wq);
> +	vcore->preempt_tb = TB_NIL;
> +	vcore->lpcr = kvm->arch.lpcr;
> +	vcore->first_vcpuid = core * threads_per_core;
> +	vcore->kvm = kvm;

Is there a particular reason why you need to pull this code out into a
separate function?  If so, it would be a little nicer if you did that
in a separate patch, to make it easier to see that the code motion
changes nothing.

> @@ -1590,9 +1645,16 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
>  
>  	srcu_idx = srcu_read_lock(&vc->kvm->srcu);
>  
> +	if (vc->mpp_buffer_is_valid)
> +		ppc_start_restoring_l2_cache(vc);
> +
>  	__kvmppc_vcore_entry();
>  
>  	spin_lock(&vc->lock);
> +
> +	if (vc->mpp_buffer)
> +		ppc_start_saving_l2_cache(vc);

I wonder if we would get better performance improvements if we kicked
this off earlier, for instance before we save all the FP/VSX state and
switch the MMU?  I guess that could be a subsequent patch.

Paul.

^ permalink raw reply

* Re: [PATCH V3 0/3] Add new PowerPC specific ELF core notes
From: Sam Bobroff @ 2014-07-17 23:23 UTC (permalink / raw)
  To: Michael Neuling, Benjamin Herrenschmidt
  Cc: james.hogan, avagin, Paul.Clothier, David S. Miller,
	Peter Zijlstra, Pedro Alves, oleg, Linux Kernel Mailing List,
	dhowells, Linux PPC dev, davej, akpm, tglx, Anshuman Khandual
In-Reply-To: <CAEjGV6yK_9DS1cLQ2BH=HFi029N8rz-4Wyvwzdb_gy6PnJgbwg@mail.gmail.com>

On 17/07/14 21:14, Michael Neuling wrote:
> 
> On Jul 17, 2014 9:11 PM, "Benjamin Herrenschmidt"
> <benh@kernel.crashing.org <mailto:benh@kernel.crashing.org>> wrote:
>>
>> > >
>> > >> Outstanding Issues
>> > >> ==================
>> > >> (1) Running DSCR register value inside a transaction does not
> seem to be saved
>> > >>     at thread.dscr when the process stops for ptrace examination.
>> > >
>> > > Hey Ben,
>> > >
>> > > Any updates on this patch series ?
>> >
>> > Ben,
>> >
>> > Any updates on this patch series ?
>>
>> I haven't had a chance to review yet, I was hoping somebody else would..
>>
>> Have you made any progress vs. the DSCR outstanding issue mentioned
>> above ?
> 
> The DSCR issue should be resolved with Sam Bobroff's recent  DSCR
> fixes.  I've not tested them though.
> 
> Actually... Sam did you review this series?
> 
> Mikey
> 

I did, and applying "powerpc: Correct DSCR during TM context switch"
corrected the DSCR value in the test program (the one in the patch notes
for this series).

(In fact, IIRC, the reason for my patch set was the bug exposed by this
one ;-)

Cheers,
Sam.

^ permalink raw reply

* [RFC 2/2] powerpc: reorder per-cpu NUMA information's initialization
From: Nishanth Aravamudan @ 2014-07-17 23:15 UTC (permalink / raw)
  To: benh
  Cc: Fenghua Yu, Tony Luck, linux-ia64, linux-kernel, linux-mm,
	David Rientjes, Joonsoo Kim, linuxppc-dev, Jiang Liu, Wanpeng Li
In-Reply-To: <20140717230958.GB32660@linux.vnet.ibm.com>

There is an issue currently where NUMA information is used on powerpc
(and possibly ia64) before it has been read from the device-tree, which
leads to large slab consumption with CONFIG_SLUB and memoryless nodes.

NUMA powerpc non-boot CPU's cpu_to_node/cpu_to_mem is only accurate
after start_secondary(), similar to ia64, which is invoked via
smp_init().

Commit 6ee0578b4daae ("workqueue: mark init_workqueues() as
early_initcall()") made init_workqueues() be invoked via
do_pre_smp_initcalls(), which is obviously before the secondary
processors are online.

Additionally, the following commits changed init_workqueues() to use
cpu_to_node to determine the node to use for kthread_create_on_node:

bce903809ab3f ("workqueue: add wq_numa_tbl_len and
wq_numa_possible_cpumask[]")
f3f90ad469342 ("workqueue: determine NUMA node of workers accourding to
the allowed cpumask")

Therefore, when init_workqueues() runs, it sees all CPUs as being on
Node 0. On LPARs or KVM guests where Node 0 is memoryless, this leads to
a high number of slab deactivations
(http://www.spinics.net/lists/linux-mm/msg67489.html).

Fix this by initializing the powerpc-specific CPU<->node/local memory
node mapping as early as possible, which on powerpc is
do_init_bootmem(). Currently that function initializes the mapping for
the boot CPU, but we extend it to setup the mapping for all possible
CPUs. Then, in smp_prepare_cpus(), we can correspondingly set the
per-cpu values for all possible CPUs. That ensures that before the
early_initcalls run (and really as early as possible), the per-cpu NUMA
mapping is accurate.

While testing memoryless nodes on PowerKVM guests with a fix to the
workqueue logic to use cpu_to_mem() instead of cpu_to_node(), with a
guest topology of:

available: 2 nodes (0-1)
node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99
node 1 size: 16336 MB
node 1 free: 15329 MB
node distances:
node   0   1
  0:  10  40
  1:  40  10

the slab consumption decreases from

Slab:             932416 kB
SUnreclaim:       902336 kB

to

Slab:             395264 kB
SUnreclaim:       359424 kB

And we a corresponding increase in the slab efficiency from

slab                                   mem     objs    slabs
                                      used   active   active
------------------------------------------------------------
kmalloc-16384                       337 MB   11.28%  100.00%
task_struct                         288 MB    9.93%  100.00%

to

slab                                   mem     objs    slabs
                                      used   active   active
------------------------------------------------------------
kmalloc-16384                        37 MB  100.00%  100.00%
task_struct                          31 MB  100.00%  100.00%

Powerpc didn't support memoryless nodes until recently (64bb80d87f01
"powerpc/numa: Enable CONFIG_HAVE_MEMORYLESS_NODES" and 8c272261194d
"powerpc/numa: Enable USE_PERCPU_NUMA_NODE_ID"). Those commits also
helped improve memory consumption with these kind of environments.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 51a3ff7..91ff531 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -376,6 +376,11 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 					GFP_KERNEL, cpu_to_node(cpu));
 		zalloc_cpumask_var_node(&per_cpu(cpu_core_map, cpu),
 					GFP_KERNEL, cpu_to_node(cpu));
+		/*
+		 * numa_node_id() works after this.
+		 */
+		set_cpu_numa_node(cpu, numa_cpu_lookup_table[cpu]);
+		set_cpu_numa_mem(cpu, local_memory_node(numa_cpu_lookup_table[cpu]));
 	}
 
 	cpumask_set_cpu(boot_cpuid, cpu_sibling_mask(boot_cpuid));
@@ -723,12 +728,6 @@ void start_secondary(void *unused)
 	}
 	traverse_core_siblings(cpu, true);
 
-	/*
-	 * numa_node_id() works after this.
-	 */
-	set_numa_node(numa_cpu_lookup_table[cpu]);
-	set_numa_mem(local_memory_node(numa_cpu_lookup_table[cpu]));
-
 	smp_wmb();
 	notify_cpu_starting(cpu);
 	set_cpu_online(cpu, true);
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 3b181b2..b1f0b86 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1049,7 +1049,7 @@ static void __init mark_reserved_regions_for_nid(int nid)
 
 void __init do_init_bootmem(void)
 {
-	int nid;
+	int nid, cpu;
 
 	min_low_pfn = 0;
 	max_low_pfn = memblock_end_of_DRAM() >> PAGE_SHIFT;
@@ -1122,8 +1122,15 @@ void __init do_init_bootmem(void)
 
 	reset_numa_cpu_lookup_table();
 	register_cpu_notifier(&ppc64_numa_nb);
-	cpu_numa_callback(&ppc64_numa_nb, CPU_UP_PREPARE,
-			  (void *)(unsigned long)boot_cpuid);
+	/*
+	 * We need the numa_cpu_lookup_table to be accurate for all CPUs,
+	 * even before we online them, so that we can use cpu_to_{node,mem}
+	 * early in boot, cf. smp_prepare_cpus().
+	 */
+	for_each_possible_cpu(cpu) {
+		cpu_numa_callback(&ppc64_numa_nb, CPU_UP_PREPARE,
+				  (void *)(unsigned long)cpu);
+	}
 }
 
 void __init paging_init(void)

^ permalink raw reply related

* [RFC 1/2] workqueue: use the nearest NUMA node, not the local one
From: Nishanth Aravamudan @ 2014-07-17 23:09 UTC (permalink / raw)
  To: benh
  Cc: Fenghua Yu, Tony Luck, linux-ia64, linux-kernel, linux-mm,
	David Rientjes, Joonsoo Kim, linuxppc-dev, Jiang Liu, Wanpeng Li
In-Reply-To: <20140717230923.GA32660@linux.vnet.ibm.com>

In the presence of memoryless nodes, the workqueue code incorrectly uses
cpu_to_node() to determine what node to prefer memory allocations come
from. cpu_to_mem() should be used instead, which will use the nearest
NUMA node with memory.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 35974ac..0bba022 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3547,7 +3547,12 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
 		for_each_node(node) {
 			if (cpumask_subset(pool->attrs->cpumask,
 					   wq_numa_possible_cpumask[node])) {
-				pool->node = node;
+				/*
+				 * We could use local_memory_node(node) here,
+				 * but it is expensive and the following caches
+				 * the same value.
+				 */
+				pool->node = cpu_to_mem(cpumask_first(pool->attrs->cpumask));
 				break;
 			}
 		}
@@ -4921,7 +4926,7 @@ static int __init init_workqueues(void)
 			pool->cpu = cpu;
 			cpumask_copy(pool->attrs->cpumask, cpumask_of(cpu));
 			pool->attrs->nice = std_nice[i++];
-			pool->node = cpu_to_node(cpu);
+			pool->node = cpu_to_mem(cpu);
 
 			/* alloc pool ID */
 			mutex_lock(&wq_pool_mutex);

^ permalink raw reply related

* [RFC 0/2] Memoryless nodes and kworker
From: Nishanth Aravamudan @ 2014-07-17 23:09 UTC (permalink / raw)
  To: benh
  Cc: Fenghua Yu, Tony Luck, linux-ia64, linux-kernel, linux-mm,
	David Rientjes, Joonsoo Kim, linuxppc-dev, Jiang Liu, Wanpeng Li

[Apologies for the large Cc list, but I believe we have the following
interested parties:

x86 (recently posted memoryless node support)
ia64 (existing memoryless node support)
ppc (existing memoryless node support)
previous discussion of how to solve Anton's issue with slab usage
workqueue contributors/maintainers]

There is an issue currently where NUMA information is used on powerpc
(and possibly ia64) before it has been read from the device-tree, which
leads to large slab consumption with CONFIG_SLUB and memoryless nodes.

While testing memoryless nodes on PowerKVM guests with the patches in
this series, with a guest topology of
    
    available: 2 nodes (0-1)
    node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49
    node 0 size: 0 MB
    node 0 free: 0 MB
    node 1 cpus: 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99
    node 1 size: 16336 MB
    node 1 free: 15329 MB
    node distances:
    node   0   1
      0:  10  40
      1:  40  10
    
the slab consumption decreases from
    
    Slab:             932416 kB
    SUnreclaim:       902336 kB
    
to
    
    Slab:             395264 kB
    SUnreclaim:       359424 kB
    
And we see a corresponding increase in the slab efficiency from
    
    slab                                   mem     objs    slabs
                                          used   active   active
    ------------------------------------------------------------
    kmalloc-16384                       337 MB   11.28%  100.00%
    task_struct                         288 MB    9.93%  100.00%
    
to
    
    slab                                   mem     objs    slabs
                                          used   active   active
    ------------------------------------------------------------
    kmalloc-16384                        37 MB  100.00%  100.00%
    task_struct                          31 MB  100.00%  100.00%

It turns out we see this large slab usage due to using the wrong NUMA
information when creating kthreads.
    
Two changes are required, one of which is in the workqueue code and one
of which is in the powerpc initialization. Note that ia64 may want to
consider something similar.

^ permalink raw reply

* Re: [PATCH] CMA: generalize CMA reserved area management functionality (fixup)
From: Andrew Morton @ 2014-07-17 22:06 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Joonsoo Kim, Zhang Yanfei, linuxppc-dev,
	linux-arm-kernel
In-Reply-To: <1405589767-17513-1-git-send-email-m.szyprowski@samsung.com>

On Thu, 17 Jul 2014 11:36:07 +0200 Marek Szyprowski <m.szyprowski@samsung.com> wrote:

> MAX_CMA_AREAS is used by other subsystems (i.e. arch/arm/mm/dma-mapping.c),
> so we need to provide correct definition even if CMA is disabled.
> This patch fixes this issue.
> 
> Reported-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  include/linux/cma.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/cma.h b/include/linux/cma.h
> index 9a18a2b1934c..c077635cad76 100644
> --- a/include/linux/cma.h
> +++ b/include/linux/cma.h
> @@ -5,7 +5,11 @@
>   * There is always at least global CMA area and a few optional
>   * areas configured in kernel .config.
>   */
> +#ifdef CONFIG_CMA
>  #define MAX_CMA_AREAS	(1 + CONFIG_CMA_AREAS)
> +#else
> +#define MAX_CMA_AREAS	(0)
> +#endif
>  
>  struct cma;

Joonsoo already fixed this up, a bit differently:
http://ozlabs.org/~akpm/mmots/broken-out/cma-generalize-cma-reserved-area-management-functionality-fix.patch

Which approach makes more sense?



From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Subject: CMA: fix ARM build failure related to MAX_CMA_AREAS definition

If CMA is disabled, CONFIG_CMA_AREAS isn't defined so compile error
happens. To fix it, define MAX_CMA_AREAS if CONFIG_CMA_AREAS
isn't defined.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/cma.h |    6 ++++++
 1 file changed, 6 insertions(+)

diff -puN include/linux/cma.h~cma-generalize-cma-reserved-area-management-functionality-fix include/linux/cma.h
--- a/include/linux/cma.h~cma-generalize-cma-reserved-area-management-functionality-fix
+++ a/include/linux/cma.h
@@ -5,8 +5,14 @@
  * There is always at least global CMA area and a few optional
  * areas configured in kernel .config.
  */
+#ifdef CONFIG_CMA_AREAS
 #define MAX_CMA_AREAS	(1 + CONFIG_CMA_AREAS)
 
+#else
+#define MAX_CMA_AREAS	(0)
+
+#endif
+
 struct cma;
 
 extern phys_addr_t cma_get_base(struct cma *cma);
_

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox