LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc/fsl_booke: avoid creating duplicate tlb1 entry
From: Michael Ellerman @ 2020-03-17 13:14 UTC (permalink / raw)
  To: Laurentiu Tudor, oss@buserror.net, linuxppc-dev@lists.ozlabs.org
  Cc: Diana Madalina Craciun, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, Laurentiu Tudor
In-Reply-To: <20200123111914.2565-1-laurentiu.tudor@nxp.com>

On Thu, 2020-01-23 at 11:19:25 UTC, Laurentiu Tudor wrote:
> In the current implementation, the call to loadcam_multi() is wrapped
> between switch_to_as1() and restore_to_as0() calls so, when it tries
> to create its own temporary AS=3D1 TLB1 entry, it ends up duplicating the
> existing one created by switch_to_as1(). Add a check to skip creating
> the temporary entry if already running in AS=3D1.
> 
> Fixes: d9e1831a4202 ("powerpc/85xx: Load all early TLB entries at once")
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> Cc: stable@vger.kernel.org

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/aa4113340ae6c2811e046f08c2bc21011d20a072

cheers

^ permalink raw reply

* Re: [PATCH 1/2] cpufreq/powernv: Fix use-after-free
From: Michael Ellerman @ 2020-03-17 13:14 UTC (permalink / raw)
  To: Oliver O'Halloran, linuxppc-dev
  Cc: Vaidyanathan Srinivasan, Oliver O'Halloran
In-Reply-To: <20200206062622.28235-1-oohall@gmail.com>

On Thu, 2020-02-06 at 06:26:21 UTC, Oliver O'Halloran wrote:
> The cpufreq driver has a use-after-free that we can hit if:
> 
> a) There's an OCC message pending when the notifier is registered, and
> b) The cpufreq driver fails to register with the core.
> 
> When a) occurs the notifier schedules a workqueue item to handle the
> message. The backing work_struct is located on chips[].throttle and when b)
> happens we clean up by freeing the array. Once we get to the (now free)
> queued item and the kernel crashes.
> 
> Cc: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
> Fixes: c5e29ea ("cpufreq: powernv: Fix bugs in powernv_cpufreq_{init/exit}")
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/d0a72efac89d1c35ac55197895201b7b94c5e6ef

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/vdso: remove deprecated VDS64_HAS_DESCRIPTORS references
From: Michael Ellerman @ 2020-03-17 13:14 UTC (permalink / raw)
  To: Joe Lawrence, linuxppc-dev
In-Reply-To: <20200224211848.26087-1-joe.lawrence@redhat.com>

On Mon, 2020-02-24 at 21:18:48 UTC, Joe Lawrence wrote:
> The original 2005 patch that introduced the powerpc vdso, pre-git
> ("ppc64: Implement a vDSO and use it for signal trampoline") notes that:
> 
>   ... symbols exposed by the vDSO aren't "normal" function symbols, apps
>   can't be expected to link against them directly, the vDSO's are both
>   seen as if they were linked at 0 and the symbols just contain offsets
>   to the various functions.  This is done on purpose to avoid a
>   relocation step (ppc64 functions normally have descriptors with abs
>   addresses in them).  When glibc uses those functions, it's expected to
>   use it's own trampolines that know how to reach them.
> 
> Despite that explanation, there remains dead #ifdef
> VDS64_HAS_DESCRIPTORS code-blocks that provide alternate function
> definitions that setup function descriptors.
> 
> Since VDS64_HAS_DESCRIPTORS has been unused for all these years, we
> might as well finally remove it from the codebase.
> 
> Link: https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-February/204430.=
> html
> Link: https://lore.kernel.org/lkml/1108002773.7733.196.camel@gaston/
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ffd3eaf178b0f616a071e510e289d937330b0b35

cheers

^ permalink raw reply

* Re: [PATCH] powerpc: fix emulate_step std test
From: Michael Ellerman @ 2020-03-17 13:14 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20200226055302.1577954-1-npiggin@gmail.com>

On Wed, 2020-02-26 at 05:53:02 UTC, Nicholas Piggin wrote:
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/59ed2adf393109c56d383e568f2e57bb5ad6d901

cheers

^ permalink raw reply

* Re: [PATCH] selftests: powerpc: Add tlbie_test in .gitignore
From: Michael Ellerman @ 2020-03-17 13:14 UTC (permalink / raw)
  To: Christophe Leroy, Aneesh Kumar K.V, Shuah Khan
  Cc: linux-mm, linuxppc-dev, linux-kernel, linux-kselftest
In-Reply-To: <259f9c06ed4563c4fa4fa8ffa652347278d769e7.1582847784.git.christophe.leroy@c-s.fr>

On Fri, 2020-02-28 at 00:00:09 UTC, Christophe Leroy wrote:
> The commit identified below added tlbie_test but
> forgot to add it in .gitignore.
> 
> Fixes: 93cad5f78995 ("selftests/powerpc: Add test case for tlbie vs mtpidr ordering issue")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/47bf235f324c696395c30541fe4fcf99fcd24188

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/64s/radix: Fix !SMP build
From: Michael Ellerman @ 2020-03-17 13:14 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20200302010410.2957362-1-npiggin@gmail.com>

On Mon, 2020-03-02 at 01:04:10 UTC, Nicholas Piggin wrote:
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/993cfecc59e57de237ae27fadc84ed24efa87a4d

cheers

^ permalink raw reply

* Re: [PATCH v2 -next] powerpc/pmac/smp: drop unnecessary volatile qualifier
From: Michael Ellerman @ 2020-03-17 13:14 UTC (permalink / raw)
  To: YueHaibing, benh, paulus, christophe.leroy
  Cc: linuxppc-dev, YueHaibing, linux-kernel
In-Reply-To: <20200303085604.24952-1-yuehaibing@huawei.com>

On Tue, 2020-03-03 at 08:56:04 UTC, YueHaibing wrote:
> core99_l2_cache/core99_l3_cache no need to mark as volatile,
> just remove it.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/a4037d1f1fc4e92b69d7196d4568c33078d465ea

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/32: Fix missing NULL pmd check in virt_to_kpte()
From: Michael Ellerman @ 2020-03-17 13:14 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	ndesaulniers
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b1177cdfc6af74a3e277bba5d9e708c4b3315ebe.1583575707.git.christophe.leroy@c-s.fr>

On Sat, 2020-03-07 at 10:09:15 UTC, Christophe Leroy wrote:
> Commit 2efc7c085f05 ("powerpc/32: drop get_pteptr()"),
> replaced get_pteptr() by virt_to_kpte(). But virt_to_kpte() lacks a
> NULL pmd check and returns an invalid non NULL pointer when there
> is no page table.
> 
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Fixes: 2efc7c085f05 ("powerpc/32: drop get_pteptr()")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/cc6f0e39000900e5dd1448103a9571f0eccd7d4e

cheers

^ permalink raw reply

* [PATCH 2/4] mm/slub: Use mem_node to allocate a new slab
From: Srikar Dronamraju @ 2020-03-17 13:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sachin Sant, Srikar Dronamraju, Michal Hocko, linux-mm,
	Kirill Tkhai, Mel Gorman, Christopher Lameter, Bharata B Rao,
	linuxppc-dev, Joonsoo Kim, Vlastimil Babka
In-Reply-To: <20200317131753.4074-1-srikar@linux.vnet.ibm.com>

Currently while allocating a slab for a offline node, we use its
associated node_numa_mem to search for a partial slab. If we don't find
a partial slab, we try allocating a slab from the offline node using
__alloc_pages_node. However this is bound to fail.

NIP [c00000000039a300] __alloc_pages_nodemask+0x130/0x3b0
LR [c00000000039a3c4] __alloc_pages_nodemask+0x1f4/0x3b0
Call Trace:
[c0000008b36837f0] [c00000000039a3b4] __alloc_pages_nodemask+0x1e4/0x3b0 (unreliable)
[c0000008b3683870] [c0000000003d1ff8] new_slab+0x128/0xcf0
[c0000008b3683950] [c0000000003d6060] ___slab_alloc+0x410/0x820
[c0000008b3683a40] [c0000000003d64a4] __slab_alloc+0x34/0x60
[c0000008b3683a70] [c0000000003d78b0] __kmalloc_node+0x110/0x490
[c0000008b3683af0] [c000000000343a08] kvmalloc_node+0x58/0x110
[c0000008b3683b30] [c0000000003ffd44] mem_cgroup_css_online+0x104/0x270
[c0000008b3683b90] [c000000000234e08] online_css+0x48/0xd0
[c0000008b3683bc0] [c00000000023dedc] cgroup_apply_control_enable+0x2ec/0x4d0
[c0000008b3683ca0] [c0000000002416f8] cgroup_mkdir+0x228/0x5f0
[c0000008b3683d10] [c000000000520360] kernfs_iop_mkdir+0x90/0xf0
[c0000008b3683d50] [c00000000043e400] vfs_mkdir+0x110/0x230
[c0000008b3683da0] [c000000000441ee0] do_mkdirat+0xb0/0x1a0
[c0000008b3683e20] [c00000000000b278] system_call+0x5c/0x68

Mitigate this by allocating the new slab from the node_numa_mem.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: Mel Gorman <mgorman@suse.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Sachin Sant <sachinp@linux.vnet.ibm.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Bharata B Rao <bharata@linux.ibm.com>

Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
 mm/slub.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 1c55bf7892bf..fdf7f38f96e6 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1970,14 +1970,8 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
 		struct kmem_cache_cpu *c)
 {
 	void *object;
-	int searchnode = node;
 
-	if (node == NUMA_NO_NODE)
-		searchnode = numa_mem_id();
-	else if (!node_present_pages(node))
-		searchnode = node_to_mem_node(node);
-
-	object = get_partial_node(s, get_node(s, searchnode), c, flags);
+	object = get_partial_node(s, get_node(s, node), c, flags);
 	if (object || node != NUMA_NO_NODE)
 		return object;
 
@@ -2470,6 +2464,11 @@ static inline void *new_slab_objects(struct kmem_cache *s, gfp_t flags,
 
 	WARN_ON_ONCE(s->ctor && (flags & __GFP_ZERO));
 
+	if (node == NUMA_NO_NODE)
+		node = numa_mem_id();
+	else if (!node_present_pages(node))
+		node = node_to_mem_node(node);
+
 	freelist = get_partial(s, flags, node, c);
 
 	if (freelist)
@@ -2569,12 +2568,10 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
 redo:
 
 	if (unlikely(!node_match(page, node))) {
-		int searchnode = node;
-
 		if (node != NUMA_NO_NODE && !node_present_pages(node))
-			searchnode = node_to_mem_node(node);
+			node = node_to_mem_node(node);
 
-		if (unlikely(!node_match(page, searchnode))) {
+		if (unlikely(!node_match(page, node))) {
 			stat(s, ALLOC_NODE_MISMATCH);
 			deactivate_slab(s, page, c->freelist, c);
 			goto new_slab;
-- 
2.18.1


^ permalink raw reply related

* [PATCH 0/4] Fix kmalloc_node on offline nodes
From: Srikar Dronamraju @ 2020-03-17 13:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sachin Sant, Srikar Dronamraju, Michal Hocko, linux-mm,
	Kirill Tkhai, Mel Gorman, Christopher Lameter, Bharata B Rao,
	linuxppc-dev, Joonsoo Kim, Vlastimil Babka
In-Reply-To: <3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com>

Sachin recently reported that linux-next was no more bootable on few
systems.
https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/

# numactl -H
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 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
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node   0   1
  0:  10  40
  1:  40  10
#

Sachin bisected the problem to Commit a75056fc1e7c ("mm/memcontrol.c: allocate
shrinker_map on appropriate NUMA node")

The root cause analysis showed that mm/slub and powerpc/numa had some shortcomings
with respect to offline nodes.

This patch series is on top of patches posted at
https://lore.kernel.org/linuxppc-dev/20200311110237.5731-1-srikar@linux.vnet.ibm.com/t/#u

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: Mel Gorman <mgorman@suse.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Sachin Sant <sachinp@linux.vnet.ibm.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Bharata B Rao <bharata@linux.ibm.com>

Srikar Dronamraju (4):
  mm: Check for node_online in node_present_pages
  mm/slub: Use mem_node to allocate a new slab
  mm: Implement reset_numa_mem
  powerpc/numa: Set fallback nodes for offline nodes

 arch/powerpc/mm/numa.c         | 11 ++++++++++-
 include/asm-generic/topology.h |  3 +++
 include/linux/mmzone.h         |  6 ++++--
 include/linux/topology.h       |  7 +++++++
 mm/slub.c                      | 19 ++++++++-----------
 5 files changed, 32 insertions(+), 14 deletions(-)

--
2.18.1


^ permalink raw reply

* [PATCH 3/4] mm: Implement reset_numa_mem
From: Srikar Dronamraju @ 2020-03-17 13:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sachin Sant, Srikar Dronamraju, Michal Hocko, linux-mm,
	Kirill Tkhai, Mel Gorman, Christopher Lameter, Bharata B Rao,
	linuxppc-dev, Joonsoo Kim, Vlastimil Babka
In-Reply-To: <20200317131753.4074-1-srikar@linux.vnet.ibm.com>

For a memoryless or offline nodes, node_numa_mem refers to a N_MEMORY
fallback node. Currently kernel has an API set_numa_mem that sets
node_numa_mem for memoryless node. However this API cannot be used for
offline nodes. Hence all offline nodes will have their node_numa_mem set
to 0. However systems can themselves have node 0 as offline i.e
memoryless and cpuless at this time. In such cases,
node_to_mem_node() fails to provide a N_MEMORY fallback node.

Mitigate this by having a new API that sets the default node_numa_mem for
offline nodes to be first_memory_node.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: Mel Gorman <mgorman@suse.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Sachin Sant <sachinp@linux.vnet.ibm.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Bharata B Rao <bharata@linux.ibm.com>

Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
 include/asm-generic/topology.h | 3 +++
 include/linux/topology.h       | 7 +++++++
 2 files changed, 10 insertions(+)

diff --git a/include/asm-generic/topology.h b/include/asm-generic/topology.h
index 238873739550..e803ee7850e6 100644
--- a/include/asm-generic/topology.h
+++ b/include/asm-generic/topology.h
@@ -68,6 +68,9 @@
 #ifndef set_numa_mem
 #define set_numa_mem(node)
 #endif
+#ifndef reset_numa_mem
+#define reset_numa_mem(node)
+#endif
 #ifndef set_cpu_numa_mem
 #define set_cpu_numa_mem(cpu, node)
 #endif
diff --git a/include/linux/topology.h b/include/linux/topology.h
index eb2fe6edd73c..bebda80038bf 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -147,6 +147,13 @@ static inline int node_to_mem_node(int node)
 }
 #endif
 
+#ifndef reset_numa_mem
+static inline void reset_numa_mem(int node)
+{
+	_node_numa_mem_[node] = first_memory_node;
+}
+#endif
+
 #ifndef numa_mem_id
 /* Returns the number of the nearest Node with memory */
 static inline int numa_mem_id(void)
-- 
2.18.1


^ permalink raw reply related

* [PATCH 4/4] powerpc/numa: Set fallback nodes for offline nodes
From: Srikar Dronamraju @ 2020-03-17 13:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sachin Sant, Srikar Dronamraju, Michal Hocko, linux-mm,
	Kirill Tkhai, Mel Gorman, Christopher Lameter, Bharata B Rao,
	linuxppc-dev, Joonsoo Kim, Vlastimil Babka
In-Reply-To: <20200317131753.4074-1-srikar@linux.vnet.ibm.com>

Currently fallback nodes for offline nodes aren't set. Hence by default
node 0 ends up being the default node. However node 0 might be offline.

Fix this by explicitly setting fallback node. Ensure first_memory_node
is set before kernel does explicit setting of fallback node.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: Mel Gorman <mgorman@suse.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Sachin Sant <sachinp@linux.vnet.ibm.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Bharata B Rao <bharata@linux.ibm.com>

Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 281531340230..6e97ab6575cb 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -827,7 +827,16 @@ void __init dump_numa_cpu_topology(void)
 	if (!numa_enabled)
 		return;
 
-	for_each_online_node(node) {
+	for_each_node(node) {
+		/*
+		 * For all possible but not yet online nodes, ensure their
+		 * node_numa_mem is set correctly so that kmalloc_node works
+		 * for such nodes.
+		 */
+		if (!node_online(node)) {
+			reset_numa_mem(node);
+			continue;
+		}
 		pr_info("Node %d CPUs:", node);
 
 		count = 0;
-- 
2.18.1


^ permalink raw reply related

* [PATCH 1/4] mm: Check for node_online in node_present_pages
From: Srikar Dronamraju @ 2020-03-17 13:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sachin Sant, Srikar Dronamraju, Michal Hocko, linux-mm,
	Kirill Tkhai, Mel Gorman, Christopher Lameter, Bharata B Rao,
	linuxppc-dev, Joonsoo Kim, Vlastimil Babka
In-Reply-To: <20200317131753.4074-1-srikar@linux.vnet.ibm.com>

Calling a kmalloc_node on a possible node which is not yet onlined can
lead to panic. Currently node_present_pages() doesn't verify the node is
online before accessing the pgdat for the node. However pgdat struct may
not be available resulting in a crash.

NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
LR [c0000000003d5b94] __slab_alloc+0x34/0x60
Call Trace:
[c0000008b3783960] [c0000000003d5734] ___slab_alloc+0x334/0x760 (unreliable)
[c0000008b3783a40] [c0000000003d5b94] __slab_alloc+0x34/0x60
[c0000008b3783a70] [c0000000003d6fa0] __kmalloc_node+0x110/0x490
[c0000008b3783af0] [c0000000003443d8] kvmalloc_node+0x58/0x110
[c0000008b3783b30] [c0000000003fee38] mem_cgroup_css_online+0x108/0x270
[c0000008b3783b90] [c000000000235aa8] online_css+0x48/0xd0
[c0000008b3783bc0] [c00000000023eaec] cgroup_apply_control_enable+0x2ec/0x4d0
[c0000008b3783ca0] [c000000000242318] cgroup_mkdir+0x228/0x5f0
[c0000008b3783d10] [c00000000051e170] kernfs_iop_mkdir+0x90/0xf0
[c0000008b3783d50] [c00000000043dc00] vfs_mkdir+0x110/0x230
[c0000008b3783da0] [c000000000441c90] do_mkdirat+0xb0/0x1a0
[c0000008b3783e20] [c00000000000b278] system_call+0x5c/0x68

Fix this by verifying the node is online before accessing the pgdat
structure. Fix the same for node_spanned_pages() too.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: Mel Gorman <mgorman@suse.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Sachin Sant <sachinp@linux.vnet.ibm.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Bharata B Rao <bharata@linux.ibm.com>

Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
 include/linux/mmzone.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index f3f264826423..88078a3b95e5 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -756,8 +756,10 @@ typedef struct pglist_data {
 	atomic_long_t		vm_stat[NR_VM_NODE_STAT_ITEMS];
 } pg_data_t;
 
-#define node_present_pages(nid)	(NODE_DATA(nid)->node_present_pages)
-#define node_spanned_pages(nid)	(NODE_DATA(nid)->node_spanned_pages)
+#define node_present_pages(nid)		\
+	(node_online(nid) ? NODE_DATA(nid)->node_present_pages : 0)
+#define node_spanned_pages(nid)		\
+	(node_online(nid) ? NODE_DATA(nid)->node_spanned_pages : 0)
 #ifdef CONFIG_FLAT_NODE_MEM_MAP
 #define pgdat_page_nr(pgdat, pagenr)	((pgdat)->node_mem_map + (pagenr))
 #else
-- 
2.18.1


^ permalink raw reply related

* Re: [PATCH 08/12] docs: fix broken references to text files
From: Jason Gunthorpe @ 2020-03-17 13:25 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: kvm, Linux Doc Mailing List, Peter Zijlstra, Akira Yokosawa,
	dri-devel, linux-unionfs, linux-mm, Harry Wei, Alex Shi,
	Will Deacon, kvmarm, linux-arch, Jonathan Corbet, linux-rdma,
	kvm-ppc, David Airlie, Doug Ledford, Alan Stern, linux-arm-kernel,
	Federico Vaga, Jade Alglave, Daniel Lustig, Julien Thierry,
	Mike Leach, Andrea Parri, Daniel Vetter, Paul E. McKenney,
	Suzuki K Poulose, Boqun Feng, Maarten Lankhorst, Nicholas Piggin,
	Maxime Ripard, Luc Maranget, OGAWA Hirofumi, David Howells,
	Mathieu Poirier, Miklos Szeredi, linux-kernel, Alexander Shishkin,
	James Morse, Thomas Zimmermann, Marc Zyngier, linux-fsdevel,
	Paolo Bonzini, Andrew Morton, linuxppc-dev
In-Reply-To: <35dcbefd50875b92aaf5b7671d4c57c50b4274fe.1584450500.git.mchehab+huawei@kernel.org>

On Tue, Mar 17, 2020 at 02:10:47PM +0100, Mauro Carvalho Chehab wrote:
> Several references got broken due to txt to ReST conversion.
> 
> Several of them can be automatically fixed with:
> 
> 	scripts/documentation-file-ref-check --fix
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>  Documentation/admin-guide/kernel-parameters.txt      |  2 +-
>  Documentation/memory-barriers.txt                    |  2 +-
>  Documentation/process/submit-checklist.rst           |  2 +-
>  .../translations/it_IT/process/submit-checklist.rst  |  2 +-
>  Documentation/translations/ko_KR/memory-barriers.txt |  2 +-
>  .../translations/zh_CN/filesystems/sysfs.txt         |  2 +-
>  .../translations/zh_CN/process/submit-checklist.rst  |  2 +-
>  Documentation/virt/kvm/arm/pvtime.rst                |  2 +-
>  Documentation/virt/kvm/devices/vcpu.rst              |  2 +-
>  Documentation/virt/kvm/hypercalls.rst                |  4 ++--
>  arch/powerpc/include/uapi/asm/kvm_para.h             |  2 +-
>  drivers/gpu/drm/Kconfig                              |  2 +-
>  drivers/gpu/drm/drm_ioctl.c                          |  2 +-
>  drivers/hwtracing/coresight/Kconfig                  |  2 +-
>  fs/fat/Kconfig                                       |  8 ++++----
>  fs/fuse/Kconfig                                      |  2 +-
>  fs/fuse/dev.c                                        |  2 +-
>  fs/overlayfs/Kconfig                                 |  6 +++---
>  include/linux/mm.h                                   |  4 ++--
>  include/uapi/linux/ethtool_netlink.h                 |  2 +-
>  include/uapi/rdma/rdma_user_ioctl_cmds.h             |  2 +-

For the rdma files

Acked-by: Jason Gunthorpe <jgg@mellanox.com>

Jason

^ permalink raw reply

* Re: [PATCH 2/4] mm/slub: Use mem_node to allocate a new slab
From: Vlastimil Babka @ 2020-03-17 13:34 UTC (permalink / raw)
  To: Srikar Dronamraju, Andrew Morton
  Cc: Sachin Sant, Michal Hocko, linux-mm, Kirill Tkhai, Mel Gorman,
	Christopher Lameter, Bharata B Rao, linuxppc-dev, Joonsoo Kim
In-Reply-To: <20200317131753.4074-3-srikar@linux.vnet.ibm.com>

On 3/17/20 2:17 PM, Srikar Dronamraju wrote:
> Currently while allocating a slab for a offline node, we use its
> associated node_numa_mem to search for a partial slab. If we don't find
> a partial slab, we try allocating a slab from the offline node using
> __alloc_pages_node. However this is bound to fail.
> 
> NIP [c00000000039a300] __alloc_pages_nodemask+0x130/0x3b0
> LR [c00000000039a3c4] __alloc_pages_nodemask+0x1f4/0x3b0
> Call Trace:
> [c0000008b36837f0] [c00000000039a3b4] __alloc_pages_nodemask+0x1e4/0x3b0 (unreliable)
> [c0000008b3683870] [c0000000003d1ff8] new_slab+0x128/0xcf0
> [c0000008b3683950] [c0000000003d6060] ___slab_alloc+0x410/0x820
> [c0000008b3683a40] [c0000000003d64a4] __slab_alloc+0x34/0x60
> [c0000008b3683a70] [c0000000003d78b0] __kmalloc_node+0x110/0x490
> [c0000008b3683af0] [c000000000343a08] kvmalloc_node+0x58/0x110
> [c0000008b3683b30] [c0000000003ffd44] mem_cgroup_css_online+0x104/0x270
> [c0000008b3683b90] [c000000000234e08] online_css+0x48/0xd0
> [c0000008b3683bc0] [c00000000023dedc] cgroup_apply_control_enable+0x2ec/0x4d0
> [c0000008b3683ca0] [c0000000002416f8] cgroup_mkdir+0x228/0x5f0
> [c0000008b3683d10] [c000000000520360] kernfs_iop_mkdir+0x90/0xf0
> [c0000008b3683d50] [c00000000043e400] vfs_mkdir+0x110/0x230
> [c0000008b3683da0] [c000000000441ee0] do_mkdirat+0xb0/0x1a0
> [c0000008b3683e20] [c00000000000b278] system_call+0x5c/0x68
> 
> Mitigate this by allocating the new slab from the node_numa_mem.

Are you sure this is really needed and the other 3 patches are not enough for
the current SLUB code to work as needed? It seems you are changing the semantics
here...

> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1970,14 +1970,8 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
>  		struct kmem_cache_cpu *c)
>  {
>  	void *object;
> -	int searchnode = node;
>  
> -	if (node == NUMA_NO_NODE)
> -		searchnode = numa_mem_id();
> -	else if (!node_present_pages(node))
> -		searchnode = node_to_mem_node(node);
> -
> -	object = get_partial_node(s, get_node(s, searchnode), c, flags);
> +	object = get_partial_node(s, get_node(s, node), c, flags);
>  	if (object || node != NUMA_NO_NODE)>  		return object;
>
>       return get_any_partial(s, flags, c);

I.e. here in this if(), now node will never equal NUMA_NO_NODE (thanks to the
hunk below), thus the get_any_partial() call becomes dead code?

> @@ -2470,6 +2464,11 @@ static inline void *new_slab_objects(struct kmem_cache *s, gfp_t flags,
>  
>  	WARN_ON_ONCE(s->ctor && (flags & __GFP_ZERO));
>  
> +	if (node == NUMA_NO_NODE)
> +		node = numa_mem_id();
> +	else if (!node_present_pages(node))
> +		node = node_to_mem_node(node);
> +
>  	freelist = get_partial(s, flags, node, c);
>  
>  	if (freelist)
> @@ -2569,12 +2568,10 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>  redo:
>  
>  	if (unlikely(!node_match(page, node))) {
> -		int searchnode = node;
> -
>  		if (node != NUMA_NO_NODE && !node_present_pages(node))
> -			searchnode = node_to_mem_node(node);
> +			node = node_to_mem_node(node);
>  
> -		if (unlikely(!node_match(page, searchnode))) {
> +		if (unlikely(!node_match(page, node))) {
>  			stat(s, ALLOC_NODE_MISMATCH);
>  			deactivate_slab(s, page, c->freelist, c);
>  			goto new_slab;
> 


^ permalink raw reply

* Re: [PATCH 1/4] mm: Check for node_online in node_present_pages
From: Srikar Dronamraju @ 2020-03-17 13:37 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sachin Sant, Michal Hocko, linux-mm, Kirill Tkhai, Mel Gorman,
	Christopher Lameter, Bharata B Rao, linuxppc-dev, Joonsoo Kim,
	Vlastimil Babka
In-Reply-To: <20200317131753.4074-2-srikar@linux.vnet.ibm.com>

* Srikar Dronamraju <srikar@linux.vnet.ibm.com> [2020-03-17 18:47:50]:

> 
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
>  include/linux/mmzone.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index f3f264826423..88078a3b95e5 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -756,8 +756,10 @@ typedef struct pglist_data {
>  	atomic_long_t		vm_stat[NR_VM_NODE_STAT_ITEMS];
>  } pg_data_t;
> 
> -#define node_present_pages(nid)	(NODE_DATA(nid)->node_present_pages)
> -#define node_spanned_pages(nid)	(NODE_DATA(nid)->node_spanned_pages)
> +#define node_present_pages(nid)		\
> +	(node_online(nid) ? NODE_DATA(nid)->node_present_pages : 0)
> +#define node_spanned_pages(nid)		\
> +	(node_online(nid) ? NODE_DATA(nid)->node_spanned_pages : 0)
>  #ifdef CONFIG_FLAT_NODE_MEM_MAP
>  #define pgdat_page_nr(pgdat, pagenr)	((pgdat)->node_mem_map + (pagenr))
>  #else

If we indeed start having pgdat/NODE_DATA for even offline nodes as Michal
Hocko, then we may not this particular patch.

-- 
Thanks and Regards
Srikar Dronamraju


^ permalink raw reply

* Re: [PATCH 1/3] powerpc/numa: Set numa_node for all possible cpus
From: Vlastimil Babka @ 2020-03-17 13:44 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Sachin Sant, Srikar Dronamraju, linuxppc-dev, LKML, linux-mm,
	Kirill Tkhai, Mel Gorman, Joonsoo Kim, Kirill A. Shutemov,
	Andrew Morton, Linus Torvalds, Christopher Lameter
In-Reply-To: <20200316090652.GC11482@dhcp22.suse.cz>

On 3/16/20 10:06 AM, Michal Hocko wrote:
> On Thu 12-03-20 17:41:58, Vlastimil Babka wrote:
> [...]
>> with nid present in:
>> N_POSSIBLE - pgdat might not exist, node_to_mem_node() must return some online
> 
> I would rather have a dummy pgdat for those. Have a look at 
> $ git grep "NODE_DATA.*->" | wc -l
> 63
> 
> Who knows how many else we have there. I haven't looked more closely.
> Besides that what is a real reason to not have pgdat ther and force all
> users of a $random node from those that the platform considers possible
> for special casing? Is that a memory overhead? Is that really a thing?

I guess we can ignore memory overhead. I guess there only might be some concern
that for nodes that are initially offline, we will allocate the pgdat on a
different node, and after they are online, it will stay on a different node with
more access latency from local cpus. If we only allocate for online nodes, it
can always be local? But I guess it doesn't matter that much.

> Somebody has suggested to tweak some of the low level routines to do the
> special casing but I really have to say I do not like that. We shouldn't
> use the first online node or anything like that. We should simply always
> follow the topology presented by FW and of that we need to have a pgdat.
> 


^ permalink raw reply

* Re: [PATCH 2/4] mm/slub: Use mem_node to allocate a new slab
From: Srikar Dronamraju @ 2020-03-17 13:45 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Sachin Sant, Michal Hocko, linux-mm, Kirill Tkhai, Mel Gorman,
	Joonsoo Kim, Andrew Morton, Bharata B Rao, linuxppc-dev,
	Christopher Lameter
In-Reply-To: <ef34b0bb-4dfa-cb0e-1830-9ad59119da5e@suse.cz>

* Vlastimil Babka <vbabka@suse.cz> [2020-03-17 14:34:25]:

> On 3/17/20 2:17 PM, Srikar Dronamraju wrote:
> > Currently while allocating a slab for a offline node, we use its
> > associated node_numa_mem to search for a partial slab. If we don't find
> > a partial slab, we try allocating a slab from the offline node using
> > __alloc_pages_node. However this is bound to fail.
> > 
> > NIP [c00000000039a300] __alloc_pages_nodemask+0x130/0x3b0
> > LR [c00000000039a3c4] __alloc_pages_nodemask+0x1f4/0x3b0
> > Call Trace:
> > [c0000008b36837f0] [c00000000039a3b4] __alloc_pages_nodemask+0x1e4/0x3b0 (unreliable)
> > [c0000008b3683870] [c0000000003d1ff8] new_slab+0x128/0xcf0
> > [c0000008b3683950] [c0000000003d6060] ___slab_alloc+0x410/0x820
> > [c0000008b3683a40] [c0000000003d64a4] __slab_alloc+0x34/0x60
> > [c0000008b3683a70] [c0000000003d78b0] __kmalloc_node+0x110/0x490
> > [c0000008b3683af0] [c000000000343a08] kvmalloc_node+0x58/0x110
> > [c0000008b3683b30] [c0000000003ffd44] mem_cgroup_css_online+0x104/0x270
> > [c0000008b3683b90] [c000000000234e08] online_css+0x48/0xd0
> > [c0000008b3683bc0] [c00000000023dedc] cgroup_apply_control_enable+0x2ec/0x4d0
> > [c0000008b3683ca0] [c0000000002416f8] cgroup_mkdir+0x228/0x5f0
> > [c0000008b3683d10] [c000000000520360] kernfs_iop_mkdir+0x90/0xf0
> > [c0000008b3683d50] [c00000000043e400] vfs_mkdir+0x110/0x230
> > [c0000008b3683da0] [c000000000441ee0] do_mkdirat+0xb0/0x1a0
> > [c0000008b3683e20] [c00000000000b278] system_call+0x5c/0x68
> > 
> > Mitigate this by allocating the new slab from the node_numa_mem.
> 
> Are you sure this is really needed and the other 3 patches are not enough for
> the current SLUB code to work as needed? It seems you are changing the semantics
> here...
> 

The other 3 patches are not enough because we don't carry the searchnode
when the actual alloc_pages_node gets called. 

With only the 3 patches, we see the above Panic, its signature is slightly
different from what Sachin first reported and which I have carried in 1st
patch.

> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1970,14 +1970,8 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
> >  		struct kmem_cache_cpu *c)
> >  {
> >  	void *object;
> > -	int searchnode = node;
> >  
> > -	if (node == NUMA_NO_NODE)
> > -		searchnode = numa_mem_id();
> > -	else if (!node_present_pages(node))
> > -		searchnode = node_to_mem_node(node);
> > -
> > -	object = get_partial_node(s, get_node(s, searchnode), c, flags);
> > +	object = get_partial_node(s, get_node(s, node), c, flags);
> >  	if (object || node != NUMA_NO_NODE)>  		return object;
> >
> >       return get_any_partial(s, flags, c);
> 
> I.e. here in this if(), now node will never equal NUMA_NO_NODE (thanks to the
> hunk below), thus the get_any_partial() call becomes dead code?
> 
> > @@ -2470,6 +2464,11 @@ static inline void *new_slab_objects(struct kmem_cache *s, gfp_t flags,
> >  
> >  	WARN_ON_ONCE(s->ctor && (flags & __GFP_ZERO));
> >  
> > +	if (node == NUMA_NO_NODE)
> > +		node = numa_mem_id();
> > +	else if (!node_present_pages(node))
> > +		node = node_to_mem_node(node);
> > +
> >  	freelist = get_partial(s, flags, node, c);
> >  
> >  	if (freelist)
> > @@ -2569,12 +2568,10 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
> >  redo:
> >  
> >  	if (unlikely(!node_match(page, node))) {
> > -		int searchnode = node;
> > -
> >  		if (node != NUMA_NO_NODE && !node_present_pages(node))
> > -			searchnode = node_to_mem_node(node);
> > +			node = node_to_mem_node(node);
> >  
> > -		if (unlikely(!node_match(page, searchnode))) {
> > +		if (unlikely(!node_match(page, node))) {
> >  			stat(s, ALLOC_NODE_MISMATCH);
> >  			deactivate_slab(s, page, c->freelist, c);
> >  			goto new_slab;
> > 
> 

-- 
Thanks and Regards
Srikar Dronamraju


^ permalink raw reply

* Re: [PATCH 2/4] mm/slub: Use mem_node to allocate a new slab
From: Vlastimil Babka @ 2020-03-17 13:53 UTC (permalink / raw)
  To: Srikar Dronamraju
  Cc: Sachin Sant, Michal Hocko, linux-mm, Kirill Tkhai, Mel Gorman,
	Joonsoo Kim, Andrew Morton, Bharata B Rao, linuxppc-dev,
	Christopher Lameter
In-Reply-To: <20200317134523.GB4334@linux.vnet.ibm.com>

On 3/17/20 2:45 PM, Srikar Dronamraju wrote:
> * Vlastimil Babka <vbabka@suse.cz> [2020-03-17 14:34:25]:
> 
>> On 3/17/20 2:17 PM, Srikar Dronamraju wrote:
>> > Currently while allocating a slab for a offline node, we use its
>> > associated node_numa_mem to search for a partial slab. If we don't find
>> > a partial slab, we try allocating a slab from the offline node using
>> > __alloc_pages_node. However this is bound to fail.
>> > 
>> > NIP [c00000000039a300] __alloc_pages_nodemask+0x130/0x3b0
>> > LR [c00000000039a3c4] __alloc_pages_nodemask+0x1f4/0x3b0
>> > Call Trace:
>> > [c0000008b36837f0] [c00000000039a3b4] __alloc_pages_nodemask+0x1e4/0x3b0 (unreliable)
>> > [c0000008b3683870] [c0000000003d1ff8] new_slab+0x128/0xcf0
>> > [c0000008b3683950] [c0000000003d6060] ___slab_alloc+0x410/0x820
>> > [c0000008b3683a40] [c0000000003d64a4] __slab_alloc+0x34/0x60
>> > [c0000008b3683a70] [c0000000003d78b0] __kmalloc_node+0x110/0x490
>> > [c0000008b3683af0] [c000000000343a08] kvmalloc_node+0x58/0x110
>> > [c0000008b3683b30] [c0000000003ffd44] mem_cgroup_css_online+0x104/0x270
>> > [c0000008b3683b90] [c000000000234e08] online_css+0x48/0xd0
>> > [c0000008b3683bc0] [c00000000023dedc] cgroup_apply_control_enable+0x2ec/0x4d0
>> > [c0000008b3683ca0] [c0000000002416f8] cgroup_mkdir+0x228/0x5f0
>> > [c0000008b3683d10] [c000000000520360] kernfs_iop_mkdir+0x90/0xf0
>> > [c0000008b3683d50] [c00000000043e400] vfs_mkdir+0x110/0x230
>> > [c0000008b3683da0] [c000000000441ee0] do_mkdirat+0xb0/0x1a0
>> > [c0000008b3683e20] [c00000000000b278] system_call+0x5c/0x68
>> > 
>> > Mitigate this by allocating the new slab from the node_numa_mem.
>> 
>> Are you sure this is really needed and the other 3 patches are not enough for
>> the current SLUB code to work as needed? It seems you are changing the semantics
>> here...
>> 
> 
> The other 3 patches are not enough because we don't carry the searchnode
> when the actual alloc_pages_node gets called. 
> 
> With only the 3 patches, we see the above Panic, its signature is slightly
> different from what Sachin first reported and which I have carried in 1st
> patch.

Ah, I see. So that's the missing pgdat after your series [1] right?

That sounds like an argument for Michal's suggestions that pgdats exist and have
correctly populated zonelists for all possible nodes.
node_to_mem_node() could be just a shortcut for the first zone's node in the
zonelist, so that fallback follows the topology.

[1]
https://lore.kernel.org/linuxppc-dev/20200311110237.5731-1-srikar@linux.vnet.ibm.com/t/#m76e5b4c4084380b1d4b193d5aa0359b987f2290e

>> > --- a/mm/slub.c
>> > +++ b/mm/slub.c
>> > @@ -1970,14 +1970,8 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
>> >  		struct kmem_cache_cpu *c)
>> >  {
>> >  	void *object;
>> > -	int searchnode = node;
>> >  
>> > -	if (node == NUMA_NO_NODE)
>> > -		searchnode = numa_mem_id();
>> > -	else if (!node_present_pages(node))
>> > -		searchnode = node_to_mem_node(node);
>> > -
>> > -	object = get_partial_node(s, get_node(s, searchnode), c, flags);
>> > +	object = get_partial_node(s, get_node(s, node), c, flags);
>> >  	if (object || node != NUMA_NO_NODE)>  		return object;
>> >
>> >       return get_any_partial(s, flags, c);
>> 
>> I.e. here in this if(), now node will never equal NUMA_NO_NODE (thanks to the
>> hunk below), thus the get_any_partial() call becomes dead code?
>> 
>> > @@ -2470,6 +2464,11 @@ static inline void *new_slab_objects(struct kmem_cache *s, gfp_t flags,
>> >  
>> >  	WARN_ON_ONCE(s->ctor && (flags & __GFP_ZERO));
>> >  
>> > +	if (node == NUMA_NO_NODE)
>> > +		node = numa_mem_id();
>> > +	else if (!node_present_pages(node))
>> > +		node = node_to_mem_node(node);
>> > +
>> >  	freelist = get_partial(s, flags, node, c);
>> >  
>> >  	if (freelist)
>> > @@ -2569,12 +2568,10 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>> >  redo:
>> >  
>> >  	if (unlikely(!node_match(page, node))) {
>> > -		int searchnode = node;
>> > -
>> >  		if (node != NUMA_NO_NODE && !node_present_pages(node))
>> > -			searchnode = node_to_mem_node(node);
>> > +			node = node_to_mem_node(node);
>> >  
>> > -		if (unlikely(!node_match(page, searchnode))) {
>> > +		if (unlikely(!node_match(page, node))) {
>> >  			stat(s, ALLOC_NODE_MISMATCH);
>> >  			deactivate_slab(s, page, c->freelist, c);
>> >  			goto new_slab;
>> > 
>> 
> 


^ permalink raw reply

* Re: [PATCH 1/3] powerpc/numa: Set numa_node for all possible cpus
From: Michal Hocko @ 2020-03-17 14:01 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Sachin Sant, Srikar Dronamraju, linuxppc-dev, LKML, linux-mm,
	Kirill Tkhai, Mel Gorman, Joonsoo Kim, Kirill A. Shutemov,
	Andrew Morton, Linus Torvalds, Christopher Lameter
In-Reply-To: <65b99db6-3bdf-6caa-74e5-6d6b681f16b5@suse.cz>

On Tue 17-03-20 14:44:45, Vlastimil Babka wrote:
> On 3/16/20 10:06 AM, Michal Hocko wrote:
> > On Thu 12-03-20 17:41:58, Vlastimil Babka wrote:
> > [...]
> >> with nid present in:
> >> N_POSSIBLE - pgdat might not exist, node_to_mem_node() must return some online
> > 
> > I would rather have a dummy pgdat for those. Have a look at 
> > $ git grep "NODE_DATA.*->" | wc -l
> > 63
> > 
> > Who knows how many else we have there. I haven't looked more closely.
> > Besides that what is a real reason to not have pgdat ther and force all
> > users of a $random node from those that the platform considers possible
> > for special casing? Is that a memory overhead? Is that really a thing?
> 
> I guess we can ignore memory overhead. I guess there only might be some concern
> that for nodes that are initially offline, we will allocate the pgdat on a
> different node, and after they are online, it will stay on a different node with
> more access latency from local cpus. If we only allocate for online nodes, it
> can always be local? But I guess it doesn't matter that much.

This is not the case even now because of chicke&egg. You need a memory
to allocate from and that memory has to be managed somewhere per node
(pgdat). Keep in mind we do not have the bootmem allocator for the
hotplug. Have a look at hotadd_new_pgdat and when it is called. There
are some attempts to allocate memmap from the hotpluged memory but I am
not sure we can do the whole thing without pgdat in place. If we can
then can come up with some replace the pgdat magic. But still I am not
even sure this is something we really have to optimize for.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH v4 3/3] powerpc/powernv: Parse device tree, population of SPR support
From: Pratik Sampat @ 2020-03-17 14:08 UTC (permalink / raw)
  To: Michael Ellerman, linux-kernel, linuxppc-dev, svaidy, ego,
	linuxram, pratik.sampat, pratik.r.sampat
In-Reply-To: <87tv2nptb8.fsf@mpe.ellerman.id.au>

Thank you Michael for the review.


On 17/03/20 8:43 am, Michael Ellerman wrote:
> Pratik Rajesh Sampat <psampat@linux.ibm.com> writes:
>> Parse the device tree for nodes self-save, self-restore and populate
>> support for the preferred SPRs based what was advertised by the device
>> tree.
> These should be documented in:
>    Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt

Sure thing I'll add them.

>> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
>> index 97aeb45e897b..27dfadf609e8 100644
>> --- a/arch/powerpc/platforms/powernv/idle.c
>> +++ b/arch/powerpc/platforms/powernv/idle.c
>> @@ -1436,6 +1436,85 @@ static void __init pnv_probe_idle_states(void)
>>   		supported_cpuidle_states |= pnv_idle_states[i].flags;
>>   }
>>   
>> +/*
>> + * Extracts and populates the self save or restore capabilities
>> + * passed from the device tree node
>> + */
>> +static int extract_save_restore_state_dt(struct device_node *np, int type)
>> +{
>> +	int nr_sprns = 0, i, bitmask_index;
>> +	int rc = 0;
>> +	u64 *temp_u64;
>> +	u64 bit_pos;
>> +
>> +	nr_sprns = of_property_count_u64_elems(np, "sprn-bitmask");
>> +	if (nr_sprns <= 0)
>> +		return rc;
> Using <= 0 means zero SPRs is treated by success as the caller, is that
> intended? If so a comment would be appropriate.

My bad, this is undesirable. This should be treated as a failure.
I'll fix this.

>> +	temp_u64 = kcalloc(nr_sprns, sizeof(u64), GFP_KERNEL);
>> +	if (of_property_read_u64_array(np, "sprn-bitmask",
>> +				       temp_u64, nr_sprns)) {
>> +		pr_warn("cpuidle-powernv: failed to find registers in DT\n");
>> +		kfree(temp_u64);
>> +		return -EINVAL;
>> +	}
>> +	/*
>> +	 * Populate acknowledgment of support for the sprs in the global vector
>> +	 * gotten by the registers supplied by the firmware.
>> +	 * The registers are in a bitmask, bit index within
>> +	 * that specifies the SPR
>> +	 */
>> +	for (i = 0; i < nr_preferred_sprs; i++) {
>> +		bitmask_index = preferred_sprs[i].spr / 64;
>> +		bit_pos = preferred_sprs[i].spr % 64;
> This is basically a hand coded bitmap, see eg. BIT_WORD(), BIT_MASK() etc.
>
> I don't think there's an easy way to convert temp_u64 into a proper
> bitmap, so it's probably not worth doing that. But at least use the macros.

Right. I'll use the macros for a cleaner abstraction.

>> +		if ((temp_u64[bitmask_index] & (1UL << bit_pos)) == 0) {
>> +			if (type == SELF_RESTORE_TYPE)
>> +				preferred_sprs[i].supported_mode &=
>> +					~SELF_RESTORE_STRICT;
>> +			else
>> +				preferred_sprs[i].supported_mode &=
>> +					~SELF_SAVE_STRICT;
>> +			continue;
>> +		}
>> +		if (type == SELF_RESTORE_TYPE) {
>> +			preferred_sprs[i].supported_mode |=
>> +				SELF_RESTORE_STRICT;
>> +		} else {
>> +			preferred_sprs[i].supported_mode |=
>> +				SELF_SAVE_STRICT;
>> +		}
>> +	}
>> +
>> +	kfree(temp_u64);
>> +	return rc;
>> +}
>> +
>> +static int pnv_parse_deepstate_dt(void)
>> +{
>> +	struct device_node *sr_np, *ss_np;
> You never use these concurrently AFAICS, so you could just have a single *np.

Sure, got rid of it.

>> +	int rc = 0, i;
>> +
>> +	/* Self restore register population */
>> +	sr_np = of_find_node_by_path("/ibm,opal/power-mgt/self-restore");
> I know the existing idle code uses of_find_node_by_path(), but that's
> because it's old and crufty. Please don't add new searches by path. You
> should be searching by compatible.
>
Alright, I'll replace of_find_node_by_path() calls with of_find_compatible_node()

>> +	if (!sr_np) {
>> +		pr_warn("opal: self restore Node not found");
> This warning and the others below will fire on all existing firmware
> versions, which is not OK.

I'll get rid of the warnings. They seem unnecessary to me also now.

>> +	} else {
>> +		rc = extract_save_restore_state_dt(sr_np, SELF_RESTORE_TYPE);
>> +		if (rc != 0)
>> +			return rc;
>> +	}
>> +	/* Self save register population */
>> +	ss_np = of_find_node_by_path("/ibm,opal/power-mgt/self-save");
>> +	if (!ss_np) {
>> +		pr_warn("opal: self save Node not found");
>> +		pr_warn("Legacy firmware. Assuming default self-restore support");
>> +		for (i = 0; i < nr_preferred_sprs; i++)
>> +			preferred_sprs[i].supported_mode &= ~SELF_SAVE_STRICT;
>> +	} else {
>> +		rc = extract_save_restore_state_dt(ss_np, SELF_SAVE_TYPE);
>> +	}
>> +	return rc;
> You're leaking references on all the device_nodes in here, you need
> of_node_put() before exiting.

Got it. I'll clear the refcount before exitting.

>> +}
>
> cheers
Thanks!
Pratik


^ permalink raw reply

* [PATCH v5 1/3] powerpc/powernv: Interface to define support and preference for a SPR
From: Pratik Rajesh Sampat @ 2020-03-17 14:10 UTC (permalink / raw)
  To: linux-kernel, linuxppc-dev, mpe, ego, linuxram, psampat,
	pratik.r.sampat
In-Reply-To: <20200317141018.42380-1-psampat@linux.ibm.com>

Define a bitmask interface to determine support for the Self Restore,
Self Save or both.

Also define an interface to determine the preference of that SPR to
be strictly saved or restored or encapsulated with an order of preference.

The preference bitmask is shown as below:
----------------------------
|... | 2nd pref | 1st pref |
----------------------------
MSB			  LSB

The preference from higher to lower is from LSB to MSB with a shift of 8
bits.
Example:
Prefer self save first, if not available then prefer self
restore
The preference mask for this scenario will be seen as below.
((SELF_RESTORE_STRICT << PREFERENCE_SHIFT) | SELF_SAVE_STRICT)
---------------------------------
|... | Self restore | Self save |
---------------------------------
MSB			        LSB

Finally, declare a list of preferred SPRs which encapsulate the bitmaks
for preferred and supported with defaults of both being set to support
legacy firmware.

This commit also implements using the above interface and retains the
legacy functionality of self restore.

Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/platforms/powernv/idle.c | 316 +++++++++++++++++++++-----
 1 file changed, 259 insertions(+), 57 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 78599bca66c2..03fe835aadd1 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -32,9 +32,112 @@
 #define P9_STOP_SPR_MSR 2000
 #define P9_STOP_SPR_PSSCR      855
 
+/* Interface for the stop state supported and preference */
+#define SELF_RESTORE_TYPE    0
+#define SELF_SAVE_TYPE       1
+
+#define NR_PREFERENCES    2
+#define PREFERENCE_SHIFT  4
+#define PREFERENCE_MASK   0xf
+
+#define UNSUPPORTED         0x0
+#define SELF_RESTORE_STRICT 0x1
+#define SELF_SAVE_STRICT    0x2
+
+/*
+ * Bitmask defining the kind of preferences available.
+ * Note : The higher to lower preference is from LSB to MSB, with a shift of
+ * 4 bits.
+ * ----------------------------
+ * |    | 2nd pref | 1st pref |
+ * ----------------------------
+ * MSB			      LSB
+ */
+/* Prefer Restore if available, otherwise unsupported */
+#define PREFER_SELF_RESTORE_ONLY	SELF_RESTORE_STRICT
+/* Prefer Save if available, otherwise unsupported */
+#define PREFER_SELF_SAVE_ONLY		SELF_SAVE_STRICT
+/* Prefer Restore when available, otherwise prefer Save */
+#define PREFER_RESTORE_SAVE		((SELF_SAVE_STRICT << \
+					  PREFERENCE_SHIFT)\
+					  | SELF_RESTORE_STRICT)
+/* Prefer Save when available, otherwise prefer Restore*/
+#define PREFER_SAVE_RESTORE		((SELF_RESTORE_STRICT <<\
+					  PREFERENCE_SHIFT)\
+					  | SELF_SAVE_STRICT)
 static u32 supported_cpuidle_states;
 struct pnv_idle_states_t *pnv_idle_states;
 int nr_pnv_idle_states;
+/* Caching the lpcr & ptcr support to use later */
+static bool is_lpcr_self_save;
+static bool is_ptcr_self_save;
+
+struct preferred_sprs {
+	u64 spr;
+	u32 preferred_mode;
+	u32 supported_mode;
+};
+
+/*
+ * Preferred mode: Order of precedence when both self-save and self-restore
+ *		   supported
+ * Supported mode: Default support. Can be overwritten during system
+ *		   initialization
+ */
+struct preferred_sprs preferred_sprs[] = {
+	{
+		.spr = SPRN_HSPRG0,
+		.preferred_mode = PREFER_RESTORE_SAVE,
+		.supported_mode = SELF_RESTORE_STRICT,
+	},
+	{
+		.spr = SPRN_LPCR,
+		.preferred_mode = PREFER_RESTORE_SAVE,
+		.supported_mode = SELF_RESTORE_STRICT,
+	},
+	{
+		.spr = SPRN_PTCR,
+		.preferred_mode = PREFER_SAVE_RESTORE,
+		.supported_mode = SELF_RESTORE_STRICT,
+	},
+	{
+		.spr = SPRN_HMEER,
+		.preferred_mode = PREFER_RESTORE_SAVE,
+		.supported_mode = SELF_RESTORE_STRICT,
+	},
+	{
+		.spr = SPRN_HID0,
+		.preferred_mode = PREFER_RESTORE_SAVE,
+		.supported_mode = SELF_RESTORE_STRICT,
+	},
+	{
+		.spr = P9_STOP_SPR_MSR,
+		.preferred_mode = PREFER_RESTORE_SAVE,
+		.supported_mode = SELF_RESTORE_STRICT,
+	},
+	{
+		.spr = P9_STOP_SPR_PSSCR,
+		.preferred_mode = PREFER_SAVE_RESTORE,
+		.supported_mode = SELF_RESTORE_STRICT,
+	},
+	{
+		.spr = SPRN_HID1,
+		.preferred_mode = PREFER_RESTORE_SAVE,
+		.supported_mode = SELF_RESTORE_STRICT,
+	},
+	{
+		.spr = SPRN_HID4,
+		.preferred_mode = PREFER_RESTORE_SAVE,
+		.supported_mode = SELF_RESTORE_STRICT,
+	},
+	{
+		.spr = SPRN_HID5,
+		.preferred_mode = PREFER_RESTORE_SAVE,
+		.supported_mode = SELF_RESTORE_STRICT,
+	}
+};
+
+const int nr_preferred_sprs = ARRAY_SIZE(preferred_sprs);
 
 /*
  * The default stop state that will be used by ppc_md.power_save
@@ -61,78 +164,170 @@ static bool deepest_stop_found;
 
 static unsigned long power7_offline_type;
 
-static int pnv_save_sprs_for_deep_states(void)
+static int pnv_self_restore_sprs(u64 pir, int cpu, u64 spr)
 {
-	int cpu;
+	u64 reg_val;
 	int rc;
 
-	/*
-	 * hid0, hid1, hid4, hid5, hmeer and lpcr values are symmetric across
-	 * all cpus at boot. Get these reg values of current cpu and use the
-	 * same across all cpus.
-	 */
-	uint64_t lpcr_val	= mfspr(SPRN_LPCR);
-	uint64_t hid0_val	= mfspr(SPRN_HID0);
-	uint64_t hid1_val	= mfspr(SPRN_HID1);
-	uint64_t hid4_val	= mfspr(SPRN_HID4);
-	uint64_t hid5_val	= mfspr(SPRN_HID5);
-	uint64_t hmeer_val	= mfspr(SPRN_HMEER);
-	uint64_t msr_val = MSR_IDLE;
-	uint64_t psscr_val = pnv_deepest_stop_psscr_val;
-
-	for_each_present_cpu(cpu) {
-		uint64_t pir = get_hard_smp_processor_id(cpu);
-		uint64_t hsprg0_val = (uint64_t)paca_ptrs[cpu];
-
-		rc = opal_slw_set_reg(pir, SPRN_HSPRG0, hsprg0_val);
+	switch (spr) {
+	case SPRN_HSPRG0:
+		reg_val = (uint64_t)paca_ptrs[cpu];
+		rc = opal_slw_set_reg(pir, SPRN_HSPRG0, reg_val);
 		if (rc != 0)
 			return rc;
-
-		rc = opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
+		break;
+	case SPRN_LPCR:
+		reg_val = mfspr(SPRN_LPCR);
+		rc = opal_slw_set_reg(pir, SPRN_LPCR, reg_val);
 		if (rc != 0)
 			return rc;
-
+		break;
+	case P9_STOP_SPR_MSR:
+		reg_val = MSR_IDLE;
 		if (cpu_has_feature(CPU_FTR_ARCH_300)) {
-			rc = opal_slw_set_reg(pir, P9_STOP_SPR_MSR, msr_val);
+			rc = opal_slw_set_reg(pir, P9_STOP_SPR_MSR, reg_val);
 			if (rc)
 				return rc;
-
-			rc = opal_slw_set_reg(pir,
-					      P9_STOP_SPR_PSSCR, psscr_val);
-
+		}
+		break;
+	case P9_STOP_SPR_PSSCR:
+		reg_val = pnv_deepest_stop_psscr_val;
+		if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+			rc = opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, reg_val);
 			if (rc)
 				return rc;
 		}
-
-		/* HIDs are per core registers */
+		break;
+	case SPRN_HMEER:
+		reg_val = mfspr(SPRN_HMEER);
 		if (cpu_thread_in_core(cpu) == 0) {
-
-			rc = opal_slw_set_reg(pir, SPRN_HMEER, hmeer_val);
-			if (rc != 0)
+			rc = opal_slw_set_reg(pir, SPRN_HMEER, reg_val);
+			if (rc)
 				return rc;
-
-			rc = opal_slw_set_reg(pir, SPRN_HID0, hid0_val);
-			if (rc != 0)
+		}
+		break;
+	case SPRN_HID0:
+		reg_val = mfspr(SPRN_HID0);
+		if (cpu_thread_in_core(cpu) == 0) {
+			rc = opal_slw_set_reg(pir, SPRN_HID0, reg_val);
+			if (rc)
 				return rc;
+		}
+		break;
+	case SPRN_HID1:
+		reg_val = mfspr(SPRN_HID1);
+		if (cpu_thread_in_core(cpu) == 0 &&
+		    !cpu_has_feature(CPU_FTR_ARCH_300)) {
+			rc = opal_slw_set_reg(pir, SPRN_HID1, reg_val);
+			if (rc)
+				return rc;
+		}
+		break;
+	case SPRN_HID4:
+		reg_val = mfspr(SPRN_HID4);
+		if (cpu_thread_in_core(cpu) == 0 &&
+		    !cpu_has_feature(CPU_FTR_ARCH_300)) {
+			rc = opal_slw_set_reg(pir, SPRN_HID4, reg_val);
+			if (rc)
+				return rc;
+		}
+		break;
+	case SPRN_HID5:
+		reg_val = mfspr(SPRN_HID5);
+		if (cpu_thread_in_core(cpu) == 0 &&
+		    !cpu_has_feature(CPU_FTR_ARCH_300)) {
+			rc = opal_slw_set_reg(pir, SPRN_HID5, reg_val);
+			if (rc)
+				return rc;
+		}
+		break;
+	case SPRN_PTCR:
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
 
-			/* Only p8 needs to set extra HID regiters */
-			if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
-
-				rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
-				if (rc != 0)
-					return rc;
-
-				rc = opal_slw_set_reg(pir, SPRN_HID4, hid4_val);
-				if (rc != 0)
-					return rc;
-
-				rc = opal_slw_set_reg(pir, SPRN_HID5, hid5_val);
-				if (rc != 0)
-					return rc;
+static int pnv_self_save_restore_sprs(void)
+{
+	int rc, index, cpu, k;
+	u64 pir;
+	struct preferred_sprs curr_spr;
+	bool is_initialized;
+	u32 preferred;
+
+	is_lpcr_self_save = false;
+	is_ptcr_self_save = false;
+	for_each_present_cpu(cpu) {
+		pir = get_hard_smp_processor_id(cpu);
+		for (index = 0; index < nr_preferred_sprs; index++) {
+			curr_spr = preferred_sprs[index];
+			is_initialized = false;
+			/*
+			 * Go through each of the preferences
+			 * Check if it is preferred as well as supported
+			 */
+			for (k = 0; k < NR_PREFERENCES; k++) {
+				preferred = curr_spr.preferred_mode
+						& PREFERENCE_MASK;
+				if (preferred & curr_spr.supported_mode
+				    & SELF_RESTORE_STRICT) {
+					is_initialized = true;
+					rc = pnv_self_restore_sprs(pir, cpu,
+								curr_spr.spr);
+					if (rc != 0)
+						return rc;
+					break;
+				}
+				preferred_sprs[index].preferred_mode =
+					preferred_sprs[index].preferred_mode >>
+					PREFERENCE_SHIFT;
+				curr_spr = preferred_sprs[index];
+			}
+			if (!is_initialized) {
+				if (preferred_sprs[index].spr == SPRN_PTCR ||
+				    (cpu_has_feature(CPU_FTR_ARCH_300) &&
+				    (preferred_sprs[index].spr == SPRN_HID1 ||
+				     preferred_sprs[index].spr == SPRN_HID4 ||
+				     preferred_sprs[index].spr == SPRN_HID5)))
+					continue;
+				return OPAL_UNSUPPORTED;
 			}
 		}
 	}
+	return 0;
+}
 
+static int pnv_save_sprs_for_deep_states(void)
+{
+	int rc;
+	int index;
+
+	/*
+	 * Iterate over the preffered SPRs and if even one of them is
+	 * still unsupported We cut support for deep stop states
+	 */
+	for (index = 0; index < nr_preferred_sprs; index++) {
+		if (preferred_sprs[index].supported_mode == UNSUPPORTED) {
+			if (preferred_sprs[index].spr == SPRN_PTCR ||
+			    (cpu_has_feature(CPU_FTR_ARCH_300) &&
+			    (preferred_sprs[index].spr == SPRN_HID1 ||
+			     preferred_sprs[index].spr == SPRN_HID4 ||
+			     preferred_sprs[index].spr == SPRN_HID5)))
+				continue;
+			return OPAL_UNSUPPORTED;
+		}
+	}
+	/*
+	 * Try to self-restore the registers that can be self restored if self
+	 * restore is active, try the same for the registers that
+	 * can be self saved too.
+	 * Note : If both are supported, self restore is given more priority
+	 */
+	rc = pnv_self_save_restore_sprs();
+	if (rc != 0)
+		return rc;
 	return 0;
 }
 
@@ -658,7 +853,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
 		mmcr0		= mfspr(SPRN_MMCR0);
 	}
 	if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) {
-		sprs.lpcr	= mfspr(SPRN_LPCR);
+		if (!is_lpcr_self_save)
+			sprs.lpcr	= mfspr(SPRN_LPCR);
 		sprs.hfscr	= mfspr(SPRN_HFSCR);
 		sprs.fscr	= mfspr(SPRN_FSCR);
 		sprs.pid	= mfspr(SPRN_PID);
@@ -672,7 +868,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
 		sprs.mmcr1	= mfspr(SPRN_MMCR1);
 		sprs.mmcr2	= mfspr(SPRN_MMCR2);
 
-		sprs.ptcr	= mfspr(SPRN_PTCR);
+		if (!is_ptcr_self_save)
+			sprs.ptcr	= mfspr(SPRN_PTCR);
 		sprs.rpr	= mfspr(SPRN_RPR);
 		sprs.tscr	= mfspr(SPRN_TSCR);
 		if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
@@ -756,7 +953,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
 		goto core_woken;
 
 	/* Per-core SPRs */
-	mtspr(SPRN_PTCR,	sprs.ptcr);
+	if (!is_ptcr_self_save)
+		mtspr(SPRN_PTCR,	sprs.ptcr);
 	mtspr(SPRN_RPR,		sprs.rpr);
 	mtspr(SPRN_TSCR,	sprs.tscr);
 
@@ -777,7 +975,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
 	atomic_unlock_and_stop_thread_idle();
 
 	/* Per-thread SPRs */
-	mtspr(SPRN_LPCR,	sprs.lpcr);
+	if (!is_lpcr_self_save)
+		mtspr(SPRN_LPCR,	sprs.lpcr);
 	mtspr(SPRN_HFSCR,	sprs.hfscr);
 	mtspr(SPRN_FSCR,	sprs.fscr);
 	mtspr(SPRN_PID,		sprs.pid);
@@ -956,8 +1155,11 @@ void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val)
 	 * Program the LPCR via stop-api only if the deepest stop state
 	 * can lose hypervisor context.
 	 */
-	if (supported_cpuidle_states & OPAL_PM_LOSE_FULL_CONTEXT)
-		opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
+	if (supported_cpuidle_states & OPAL_PM_LOSE_FULL_CONTEXT) {
+		if (!is_lpcr_self_save)
+			opal_slw_set_reg(pir, SPRN_LPCR,
+					 lpcr_val);
+	}
 }
 
 /*
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 2/3] powerpc/powernv: Introduce Self save support
From: Pratik Rajesh Sampat @ 2020-03-17 14:10 UTC (permalink / raw)
  To: linux-kernel, linuxppc-dev, mpe, ego, linuxram, psampat,
	pratik.r.sampat
In-Reply-To: <20200317141018.42380-1-psampat@linux.ibm.com>

This commit introduces and leverages the Self save API which OPAL now
supports.

Add the new Self Save OPAL API call in the list of OPAL calls.
Implement the self saving of the SPRs based on the support populated
while respecting it's preferences.

This implementation allows mixing of support for the SPRs, which
means that a SPR can be self restored while another SPR be self saved if
they support and prefer it to be so.

Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/opal-api.h        |  3 ++-
 arch/powerpc/include/asm/opal.h            |  1 +
 arch/powerpc/platforms/powernv/idle.c      | 22 ++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal-call.c |  1 +
 4 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index c1f25a760eb1..1b6e1a68d431 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -214,7 +214,8 @@
 #define OPAL_SECVAR_GET				176
 #define OPAL_SECVAR_GET_NEXT			177
 #define OPAL_SECVAR_ENQUEUE_UPDATE		178
-#define OPAL_LAST				178
+#define OPAL_SLW_SELF_SAVE_REG			181
+#define OPAL_LAST				181
 
 #define QUIESCE_HOLD			1 /* Spin all calls at entry */
 #define QUIESCE_REJECT			2 /* Fail all calls with OPAL_BUSY */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 9986ac34b8e2..389a85b63805 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -203,6 +203,7 @@ int64_t opal_handle_hmi(void);
 int64_t opal_handle_hmi2(__be64 *out_flags);
 int64_t opal_register_dump_region(uint32_t id, uint64_t start, uint64_t end);
 int64_t opal_unregister_dump_region(uint32_t id);
+int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
 int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
 int64_t opal_config_cpu_idle_state(uint64_t state, uint64_t flag);
 int64_t opal_pci_set_phb_cxl_mode(uint64_t phb_id, uint64_t mode, uint64_t pe_number);
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 03fe835aadd1..97aeb45e897b 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -279,6 +279,26 @@ static int pnv_self_save_restore_sprs(void)
 					if (rc != 0)
 						return rc;
 					break;
+				} else if (preferred & curr_spr.supported_mode
+					   & SELF_SAVE_STRICT) {
+					is_initialized = true;
+					if (curr_spr.spr == SPRN_HMEER &&
+					    cpu_thread_in_core(cpu) != 0) {
+						continue;
+					}
+					rc = opal_slw_self_save_reg(pir,
+								curr_spr.spr);
+					if (rc != 0)
+						return rc;
+					switch (curr_spr.spr) {
+					case SPRN_LPCR:
+						is_lpcr_self_save = true;
+						break;
+					case SPRN_PTCR:
+						is_ptcr_self_save = true;
+						break;
+					}
+					break;
 				}
 				preferred_sprs[index].preferred_mode =
 					preferred_sprs[index].preferred_mode >>
@@ -1159,6 +1179,8 @@ void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val)
 		if (!is_lpcr_self_save)
 			opal_slw_set_reg(pir, SPRN_LPCR,
 					 lpcr_val);
+		else
+			opal_slw_self_save_reg(pir, SPRN_LPCR);
 	}
 }
 
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 5cd0f52d258f..11e0ceb90de0 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -223,6 +223,7 @@ OPAL_CALL(opal_handle_hmi,			OPAL_HANDLE_HMI);
 OPAL_CALL(opal_handle_hmi2,			OPAL_HANDLE_HMI2);
 OPAL_CALL(opal_config_cpu_idle_state,		OPAL_CONFIG_CPU_IDLE_STATE);
 OPAL_CALL(opal_slw_set_reg,			OPAL_SLW_SET_REG);
+OPAL_CALL(opal_slw_self_save_reg,		OPAL_SLW_SELF_SAVE_REG);
 OPAL_CALL(opal_register_dump_region,		OPAL_REGISTER_DUMP_REGION);
 OPAL_CALL(opal_unregister_dump_region,		OPAL_UNREGISTER_DUMP_REGION);
 OPAL_CALL(opal_pci_set_phb_cxl_mode,		OPAL_PCI_SET_PHB_CAPI_MODE);
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 3/3] powerpc/powernv: Parse device tree, population of SPR support
From: Pratik Rajesh Sampat @ 2020-03-17 14:10 UTC (permalink / raw)
  To: linux-kernel, linuxppc-dev, mpe, ego, linuxram, psampat,
	pratik.r.sampat
In-Reply-To: <20200317141018.42380-1-psampat@linux.ibm.com>

Parse the device tree for nodes self-save, self-restore and populate
support for the preferred SPRs based what was advertised by the device
tree.

Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
---
 .../bindings/powerpc/opal/power-mgt.txt       | 10 +++
 arch/powerpc/platforms/powernv/idle.c         | 78 +++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
index 9d619e955576..093cb5fe3d2d 100644
--- a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
+++ b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
@@ -116,3 +116,13 @@ otherwise. The length of all the property arrays must be the same.
 	which of the fields of the PMICR are set in the corresponding
 	entries in ibm,cpu-idle-state-pmicr. This is an optional
 	property on POWER8 and is absent on POWER9.
+
+- self-restore:
+ Array of unsigned 64-bit values containing a property for sprn-mask
+ with each bit indicating the index of the supported SPR for the
+ functionality. This is an optional property for both Power8 and Power9
+
+- self-save:
+  Array of unsigned 64-bit values containing a property for sprn-mask
+  with each bit indicating the index of the supported SPR for the
+  functionality. This is an optional property for both Power8 and Power9
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 97aeb45e897b..c39111b338ff 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -1436,6 +1436,81 @@ static void __init pnv_probe_idle_states(void)
 		supported_cpuidle_states |= pnv_idle_states[i].flags;
 }
 
+/*
+ * Extracts and populates the self save or restore capabilities
+ * passed from the device tree node
+ */
+static int extract_save_restore_state_dt(struct device_node *np, int type)
+{
+	int nr_sprns = 0, i, bitmask_index;
+	u64 *temp_u64;
+	u64 bit_pos;
+
+	nr_sprns = of_property_count_u64_elems(np, "sprn-bitmask");
+	if (nr_sprns <= 0)
+		return -EINVAL;
+	temp_u64 = kcalloc(nr_sprns, sizeof(u64), GFP_KERNEL);
+	if (of_property_read_u64_array(np, "sprn-bitmask",
+				       temp_u64, nr_sprns)) {
+		pr_warn("cpuidle-powernv: failed to find registers in DT\n");
+		kfree(temp_u64);
+		return -EINVAL;
+	}
+	/*
+	 * Populate acknowledgment of support for the sprs in the global vector
+	 * gotten by the registers supplied by the firmware.
+	 * The registers are in a bitmask, bit index within
+	 * that specifies the SPR
+	 */
+	for (i = 0; i < nr_preferred_sprs; i++) {
+		bitmask_index = BIT_WORD(preferred_sprs[i].spr);
+		bit_pos = BIT_MASK(preferred_sprs[i].spr);
+		if ((temp_u64[bitmask_index] & bit_pos) == 0) {
+			if (type == SELF_RESTORE_TYPE)
+				preferred_sprs[i].supported_mode &=
+					~SELF_RESTORE_STRICT;
+			else
+				preferred_sprs[i].supported_mode &=
+					~SELF_SAVE_STRICT;
+			continue;
+		}
+		if (type == SELF_RESTORE_TYPE) {
+			preferred_sprs[i].supported_mode |=
+				SELF_RESTORE_STRICT;
+		} else {
+			preferred_sprs[i].supported_mode |=
+				SELF_SAVE_STRICT;
+		}
+	}
+
+	kfree(temp_u64);
+	return 0;
+}
+
+static int pnv_parse_deepstate_dt(void)
+{
+	struct device_node *np;
+	int rc = 0, i;
+
+	/* Self restore register population */
+	np = of_find_compatible_node(NULL, NULL, "ibm,opal-self-restore");
+	if (np) {
+		rc = extract_save_restore_state_dt(np, SELF_RESTORE_TYPE);
+		if (rc != 0)
+			return rc;
+	}
+	/* Self save register population */
+	np = of_find_compatible_node(NULL, NULL, "ibm,opal-self-save");
+	if (!np) {
+		for (i = 0; i < nr_preferred_sprs; i++)
+			preferred_sprs[i].supported_mode &= ~SELF_SAVE_STRICT;
+	} else {
+		rc = extract_save_restore_state_dt(np, SELF_SAVE_TYPE);
+	}
+	of_node_put(np);
+	return rc;
+}
+
 /*
  * This function parses device-tree and populates all the information
  * into pnv_idle_states structure. It also sets up nr_pnv_idle_states
@@ -1584,6 +1659,9 @@ static int __init pnv_init_idle_states(void)
 		return rc;
 	pnv_probe_idle_states();
 
+	rc = pnv_parse_deepstate_dt();
+	if (rc)
+		return rc;
 	if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
 		if (!(supported_cpuidle_states & OPAL_PM_SLEEP_ENABLED_ER1)) {
 			power7_fastsleep_workaround_entry = false;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 0/3] Introduce Self-Save API for deep stop states
From: Pratik Rajesh Sampat @ 2020-03-17 14:10 UTC (permalink / raw)
  To: linux-kernel, linuxppc-dev, mpe, ego, linuxram, psampat,
	pratik.r.sampat

Skiboot patches v4: https://lists.ozlabs.org/pipermail/skiboot/2020-February/016404.html
Linux patches v4: https://lkml.org/lkml/2020/2/12/446
Changelog
v4 --> v5
Based on Michael Ellerman's comments, in patch3:
1. Added documentation in power-mgt.txt for self-save and self-restore
2. Used bitmap abstractions while parsing device tree
3. Fixed scenario where zero or less SPRs were treated as a success
4. Removed redundant device_node pointer
5. Removed unnecessary pr_warn messages
6. Changed the old of_find_node_by_path calls to of_find_compatible_node
7. Fixed leaking reference of the parsed device_node pointer 

Currently the stop-API supports a mechanism called as self-restore
which allows us to restore the values of certain SPRs on wakeup from a
deep-stop state to a desired value. To use this, the Kernel makes an
OPAL call passing the PIR of the CPU, the SPR number and the value to
which the SPR should be restored when that CPU wakes up from a deep
stop state.

Recently, a new feature, named self-save has been enabled in the
stop-api, which is an alternative mechanism to do the same, except
that self-save will save the current content of the SPR before
entering a deep stop state and also restore the content back on
waking up from a deep stop state.

This patch series aims at introducing and leveraging the self-save feature in
the kernel.

Now, as the kernel has a choice to prefer one mode over the other and
there can be registers in both the save/restore SPR list which are sent
from the device tree, a new interface has been defined for the seamless
handing of the modes for each SPR.

A list of preferred SPRs are maintained in the kernel which contains two
properties:
1. supported_mode: Helps in identifying if it strictly supports self
                   save or restore or both.
                   Initialized using the information from device tree.
2. preferred_mode: Calls out what mode is preferred for each SPR. It
                   could be strictly self save or restore, or it can also
                   determine the preference of  mode over the other if both
                   are present by encapsulating the other in bitmask from
                   LSB to MSB.
                   Initialized statically.

Below is a table to show the Scenario::Consequence when the self save and
self restore modes are available or disabled in different combinations as
perceived from the device tree thus giving complete backwards compatibly
regardless of an older firmware running a newer kernel or vise-versa.
Support for self save or self-restore is embedded in the device tree,
along with the set of registers it supports.

SR = Self restore; SS = Self save

.-----------------------------------.----------------------------------------.
|             Scenario              |                Consequence             |
:-----------------------------------+----------------------------------------:
| Legacy Firmware. No SS or SR node | Self restore is called for all         |
|                                   | supported SPRs                         |
:-----------------------------------+----------------------------------------:
| SR: !active SS: !active           | Deep stop states disabled              |
:-----------------------------------+----------------------------------------:
| SR: active SS: !active            | Self restore is called for all         |
|                                   | supported SPRs                         |
:-----------------------------------+----------------------------------------:
| SR: active SS: active             | Goes through the preferences for each  |
|                                   | SPR and executes of the modes          |
|                                   | accordingly. Currently, Self restore is|
|                                   | called for all the SPRs except PSSCR   |
|                                   | which is self saved                    |
:-----------------------------------+----------------------------------------:
| SR: active(only HID0) SS: active  | Self save called for all supported     |
|                                   | registers expect HID0 (as HID0 cannot  |
|                                   | be self saved currently)               |
:-----------------------------------+----------------------------------------:
| SR: !active SS: active            | currently will disable deep states as  |
|                                   | HID0 is needed to be self restored and |
|                                   | cannot be self saved                   |
'-----------------------------------'----------------------------------------'

Pratik Rajesh Sampat (3):
  powerpc/powernv: Interface to define support and preference for a SPR
  powerpc/powernv: Introduce Self save support
  powerpc/powernv: Parse device tree, population of SPR support

 .../bindings/powerpc/opal/power-mgt.txt       |  10 +
 arch/powerpc/include/asm/opal-api.h           |   3 +-
 arch/powerpc/include/asm/opal.h               |   1 +
 arch/powerpc/platforms/powernv/idle.c         | 416 +++++++++++++++---
 arch/powerpc/platforms/powernv/opal-call.c    |   1 +
 5 files changed, 373 insertions(+), 58 deletions(-)

-- 
2.17.1


^ 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