Thread (7 messages) 7 messages, 4 authors, 2019-10-30

Re: [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2019-07-29 12:38:38

Anju T Sudhakar [off-list ref] writes:
Hi Qian,

On 7/16/19 12:11 AM, Qian Cai wrote:
quoted
On Thu, 2019-07-11 at 14:53 +1000, Michael Ellerman wrote:
quoted
Hi Maddy,

Madhavan Srinivasan [off-list ref] writes:
quoted
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c
b/arch/powerpc/platforms/powernv/opal-imc.c
index 186109bdd41b..e04b20625cb9 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -69,20 +69,20 @@ static void export_imc_mode_and_cmd(struct device_node
*node,
  	if (of_property_read_u32(node, "cb_offset", &cb_offset))
  		cb_offset = IMC_CNTL_BLK_OFFSET;
  
-	for_each_node(nid) {
-		loc = (u64)(pmu_ptr->mem_info[chip].vbase) + cb_offset;
+	while (ptr->vbase != NULL) {
This means you'll bail out as soon as you find a node with no vbase, but
it's possible we could have a CPU-less node intermingled with other
nodes.

So I think you want to keep the for loop, but continue if you see a NULL
vbase?
Not sure if this will also takes care of some of those messages during the boot
on today's linux-next even without this patch.


[   18.077780][    T1] debugfs: Directory 'imc' with parent 'powerpc' already
present!
This is introduced by a recent commit: c33d442328f55 (debugfs: make 
error message a bit more verbose).

So basically, the debugfs imc_* file is created per node, and is created 
by the first nest unit which is

being registered. For the subsequent nest units, debugfs_create_dir() 
will just return since the imc_* file already

exist.

The commit "c33d442328f55 (debugfs: make error message a bit more 
verbose)", prints

a message if the debugfs file already exists in debugfs_create_dir(). 
That is why we are encountering these

messages now.


This patch (i.e, powerpc/imc: Dont create debugfs files for cpu-less 
nodes) will address the initial issue, i.e

"numa crash while reading imc_* debugfs files for cpu less nodes", and 
will not address these debugfs messages.


But yeah this is a good catch. We can have some checks to avoid these 
debugfs messages.


Hi Michael,

Do we need to have a separate patch to address these debugfs messages, 
or can we address the same

in the next version of this patch itself?
No, please do one logical change per patch.

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help