All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes
@ 2019-07-02  9:21 Madhavan Srinivasan
  2019-07-11  4:53 ` Michael Ellerman
  0 siblings, 1 reply; 7+ messages in thread
From: Madhavan Srinivasan @ 2019-07-02  9:21 UTC (permalink / raw)
  To: mpe; +Cc: Qian Cai, Madhavan Srinivasan, linuxppc-dev

Commit <684d984038aa> ('powerpc/powernv: Add debugfs interface for imc-mode
and imc') added debugfs interface for the nest imc pmu devices to support
changing of different ucode modes. Primarily adding this capability for
debug. But when doing so, the code did not consider the case of cpu-less
nodes. So when reading the _cmd_ or _mode_ file of a cpu-less node
will create this crash.

[ 1139.415461][ T5301] Faulting instruction address: 0xc0000000000d0d58
[ 1139.415492][ T5301] Oops: Kernel access of bad area, sig: 11 [#1]
[ 1139.415509][ T5301] LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=256
DEBUG_PAGEALLOC NUMA PowerNV
[ 1139.415542][ T5301] Modules linked in: i2c_opal i2c_core ip_tables x_tables
xfs sd_mod bnx2x mdio ahci libahci tg3 libphy libata firmware_class dm_mirror
dm_region_hash dm_log dm_mod
[ 1139.415595][ T5301] CPU: 67 PID: 5301 Comm: cat Not tainted 5.2.0-rc6-next-
20190627+ #19
[ 1139.415634][ T5301] NIP:  c0000000000d0d58 LR: c00000000049aa18 CTR:c0000000000d0d50
[ 1139.415675][ T5301] REGS: c00020194548f9e0 TRAP: 0300   Not tainted  (5.2.0-rc6-next-20190627+)
[ 1139.415705][ T5301] MSR:  9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR:28022822  XER: 00000000
[ 1139.415777][ T5301] CFAR: c00000000049aa14 DAR: 000000000003fc08 DSISR:40000000 IRQMASK: 0
[ 1139.415777][ T5301] GPR00: c00000000049aa18 c00020194548fc70 c0000000016f8b00000000000003fc08
[ 1139.415777][ T5301] GPR04: c00020194548fcd0 0000000000000000 0000000014884e73ffffffff00011eaa
[ 1139.415777][ T5301] GPR08: 000000007eea5a52 c0000000000d0d50 00000000000000000000000000000000
[ 1139.415777][ T5301] GPR12: c0000000000d0d50 c000201fff7f8c00 00000000000000000000000000000000
[ 1139.415777][ T5301] GPR16: 000000000000000d 00007fffeb0c3368 ffffffffffffffff0000000000000000
[ 1139.415777][ T5301] GPR20: 0000000000000000 0000000000000000 00000000000000000000000000020000
[ 1139.415777][ T5301] GPR24: 0000000000000000 0000000000000000 0000000000020000000000010ec90000
[ 1139.415777][ T5301] GPR28: c00020194548fdf0 c00020049a584ef8 0000000000000000c00020049a584ea8
[ 1139.416116][ T5301] NIP [c0000000000d0d58] imc_mem_get+0x8/0x20
[ 1139.416143][ T5301] LR [c00000000049aa18] simple_attr_read+0x118/0x170
[ 1139.416158][ T5301] Call Trace:
[ 1139.416182][ T5301] [c00020194548fc70] [c00000000049a970]simple_attr_read+0x70/0x170 (unreliable)
[ 1139.416255][ T5301] [c00020194548fd10] [c00000000054385c]debugfs_attr_read+0x6c/0xb0
[ 1139.416305][ T5301] [c00020194548fd60] [c000000000454c1c]__vfs_read+0x3c/0x70
[ 1139.416363][ T5301] [c00020194548fd80] [c000000000454d0c] vfs_read+0xbc/0x1a0
[ 1139.416392][ T5301] [c00020194548fdd0] [c00000000045519c]ksys_read+0x7c/0x140
[ 1139.416434][ T5301] [c00020194548fe20] [c00000000000b108]system_call+0x5c/0x70
[ 1139.416473][ T5301] Instruction dump:
[ 1139.416511][ T5301] 4e800020 60000000 7c0802a6 60000000 7c801d28 38600000 4e800020 60000000
[ 1139.416572][ T5301] 60000000 60000000 7c0802a6 60000000 <7d201c28> 38600000 f9240000 4e800020
[ 1139.416636][ T5301] ---[ end trace c44d1fb4ace04784 ]---
[ 1139.520686][ T5301]
[ 1140.520820][ T5301] Kernel panic - not syncing: Fatal exception

Patch fixes the issue with a more robust check for vbase to NULL.

Before patch, ls output for the debugfs imc directory

# ls /sys/kernel/debug/powerpc/imc/
imc_cmd_0    imc_cmd_251  imc_cmd_253  imc_cmd_255  imc_mode_0    imc_mode_251  imc_mode_253  imc_mode_255
imc_cmd_250  imc_cmd_252  imc_cmd_254  imc_cmd_8    imc_mode_250  imc_mode_252  imc_mode_254  imc_mode_8

After patch, ls output for the debugfs imc directory

# ls /sys/kernel/debug/powerpc/imc/
imc_cmd_0  imc_cmd_8  imc_mode_0  imc_mode_8

Fixes: 684d984038aa ('powerpc/powernv: Add debugfs interface for imc-mode and imc')
Reported-by: Qian Cai <cai@lca.pw>
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
Changelog v1:
- Modified the cpumask check.
  
 arch/powerpc/platforms/powernv/opal-imc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

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
@@ -53,9 +53,9 @@ static void export_imc_mode_and_cmd(struct device_node *node,
 				    struct imc_pmu *pmu_ptr)
 {
 	static u64 loc, *imc_mode_addr, *imc_cmd_addr;
-	int chip = 0, nid;
 	char mode[16], cmd[16];
 	u32 cb_offset;
+	struct imc_mem_info *ptr = pmu_ptr->mem_info;
 
 	imc_debugfs_parent = debugfs_create_dir("imc", powerpc_debugfs_root);
 
@@ -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) {
+		loc = (u64)(ptr->vbase) + cb_offset;
 		imc_mode_addr = (u64 *)(loc + IMC_CNTL_BLK_MODE_OFFSET);
-		sprintf(mode, "imc_mode_%d", nid);
+		sprintf(mode, "imc_mode_%d", (u32)(ptr->id));
 		if (!imc_debugfs_create_x64(mode, 0600, imc_debugfs_parent,
 					    imc_mode_addr))
 			goto err;
 
 		imc_cmd_addr = (u64 *)(loc + IMC_CNTL_BLK_CMD_OFFSET);
-		sprintf(cmd, "imc_cmd_%d", nid);
+		sprintf(cmd, "imc_cmd_%d", (u32)(ptr->id));
 		if (!imc_debugfs_create_x64(cmd, 0600, imc_debugfs_parent,
 					    imc_cmd_addr))
 			goto err;
-		chip++;
+		ptr++;
 	}
 	return;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes
  2019-07-02  9:21 [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes Madhavan Srinivasan
@ 2019-07-11  4:53 ` Michael Ellerman
  2019-07-15  6:22   ` maddy
  2019-07-15 18:41   ` Qian Cai
  0 siblings, 2 replies; 7+ messages in thread
From: Michael Ellerman @ 2019-07-11  4:53 UTC (permalink / raw)
  To: Madhavan Srinivasan; +Cc: Qian Cai, Madhavan Srinivasan, linuxppc-dev

Hi Maddy,

Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
> 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?


> +		loc = (u64)(ptr->vbase) + cb_offset;
>  		imc_mode_addr = (u64 *)(loc + IMC_CNTL_BLK_MODE_OFFSET);
> -		sprintf(mode, "imc_mode_%d", nid);
> +		sprintf(mode, "imc_mode_%d", (u32)(ptr->id));
>  		if (!imc_debugfs_create_x64(mode, 0600, imc_debugfs_parent,
>  					    imc_mode_addr))
>  			goto err;
>  
>  		imc_cmd_addr = (u64 *)(loc + IMC_CNTL_BLK_CMD_OFFSET);
> -		sprintf(cmd, "imc_cmd_%d", nid);
> +		sprintf(cmd, "imc_cmd_%d", (u32)(ptr->id));
>  		if (!imc_debugfs_create_x64(cmd, 0600, imc_debugfs_parent,
>  					    imc_cmd_addr))
>  			goto err;
> -		chip++;
> +		ptr++;
>  	}
>  	return;

cheers

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes
  2019-07-11  4:53 ` Michael Ellerman
@ 2019-07-15  6:22   ` maddy
  2019-07-15 18:41   ` Qian Cai
  1 sibling, 0 replies; 7+ messages in thread
From: maddy @ 2019-07-15  6:22 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Qian Cai, linuxppc-dev


On 7/11/19 10:23 AM, Michael Ellerman wrote:
> Hi Maddy,
>
> Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
>> 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.
Nice catch. Thanks for the review, will fix it.

Maddy

>
> So I think you want to keep the for loop, but continue if you see a NULL
> vbase?
>
>
>> +		loc = (u64)(ptr->vbase) + cb_offset;
>>   		imc_mode_addr = (u64 *)(loc + IMC_CNTL_BLK_MODE_OFFSET);
>> -		sprintf(mode, "imc_mode_%d", nid);
>> +		sprintf(mode, "imc_mode_%d", (u32)(ptr->id));
>>   		if (!imc_debugfs_create_x64(mode, 0600, imc_debugfs_parent,
>>   					    imc_mode_addr))
>>   			goto err;
>>   
>>   		imc_cmd_addr = (u64 *)(loc + IMC_CNTL_BLK_CMD_OFFSET);
>> -		sprintf(cmd, "imc_cmd_%d", nid);
>> +		sprintf(cmd, "imc_cmd_%d", (u32)(ptr->id));
>>   		if (!imc_debugfs_create_x64(cmd, 0600, imc_debugfs_parent,
>>   					    imc_cmd_addr))
>>   			goto err;
>> -		chip++;
>> +		ptr++;
>>   	}
>>   	return;
> cheers
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes
  2019-07-11  4:53 ` Michael Ellerman
  2019-07-15  6:22   ` maddy
@ 2019-07-15 18:41   ` Qian Cai
  2019-07-23 11:27     ` Anju T Sudhakar
  1 sibling, 1 reply; 7+ messages in thread
From: Qian Cai @ 2019-07-15 18:41 UTC (permalink / raw)
  To: Michael Ellerman, Madhavan Srinivasan; +Cc: linuxppc-dev

On Thu, 2019-07-11 at 14:53 +1000, Michael Ellerman wrote:
> Hi Maddy,
> 
> Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
> > 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!

where it has the following layout,
# numactl -H
available: 6 nodes (0,8,252-255)
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 50 51 52
53 54 55 56 57 58 59 60 61 62 63
node 0 size: 130197 MB
node 0 free: 127453 MB
node 8 cpus: 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 100 101 102 103 104 105 106 107 108
109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127
node 8 size: 130778 MB
node 8 free: 128119 MB
node 252 cpus:
node 252 size: 0 MB
node 252 free: 0 MB
node 253 cpus:
node 253 size: 0 MB
node 253 free: 0 MB
node 254 cpus:
node 254 size: 0 MB
node 254 free: 0 MB
node 255 cpus:
node 255 size: 0 MB
node 255 free: 0 MB
node distances:
node   0   8  252  253  254  255 
  0:  10  40  80  80  80  80 
  8:  40  10  80  80  80  80 
 252:  80  80  10  80  80  80 
 253:  80  80  80  10  80  80 
 254:  80  80  80  80  10  80 
 255:  80  80  80  80  80  10

> 
> 
> > +		loc = (u64)(ptr->vbase) + cb_offset;
> >  		imc_mode_addr = (u64 *)(loc + IMC_CNTL_BLK_MODE_OFFSET);
> > -		sprintf(mode, "imc_mode_%d", nid);
> > +		sprintf(mode, "imc_mode_%d", (u32)(ptr->id));
> >  		if (!imc_debugfs_create_x64(mode, 0600, imc_debugfs_parent,
> >  					    imc_mode_addr))
> >  			goto err;
> >  
> >  		imc_cmd_addr = (u64 *)(loc + IMC_CNTL_BLK_CMD_OFFSET);
> > -		sprintf(cmd, "imc_cmd_%d", nid);
> > +		sprintf(cmd, "imc_cmd_%d", (u32)(ptr->id));
> >  		if (!imc_debugfs_create_x64(cmd, 0600, imc_debugfs_parent,
> >  					    imc_cmd_addr))
> >  			goto err;
> > -		chip++;
> > +		ptr++;
> >  	}
> >  	return;
> 
> cheers

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes
  2019-07-15 18:41   ` Qian Cai
@ 2019-07-23 11:27     ` Anju T Sudhakar
  2019-07-29 12:36       ` Michael Ellerman
  2019-10-30 18:37       ` Qian Cai
  0 siblings, 2 replies; 7+ messages in thread
From: Anju T Sudhakar @ 2019-07-23 11:27 UTC (permalink / raw)
  To: Qian Cai, Michael Ellerman, Madhavan Srinivasan; +Cc: linuxppc-dev

Hi Qian,

On 7/16/19 12:11 AM, Qian Cai wrote:
> On Thu, 2019-07-11 at 14:53 +1000, Michael Ellerman wrote:
>> Hi Maddy,
>>
>> Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
>>> 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?


Thanks,

Anju





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes
  2019-07-23 11:27     ` Anju T Sudhakar
@ 2019-07-29 12:36       ` Michael Ellerman
  2019-10-30 18:37       ` Qian Cai
  1 sibling, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2019-07-29 12:36 UTC (permalink / raw)
  To: Anju T Sudhakar, Qian Cai, Madhavan Srinivasan; +Cc: linuxppc-dev

Anju T Sudhakar <anju@linux.vnet.ibm.com> writes:
> Hi Qian,
>
> On 7/16/19 12:11 AM, Qian Cai wrote:
>> On Thu, 2019-07-11 at 14:53 +1000, Michael Ellerman wrote:
>>> Hi Maddy,
>>>
>>> Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
>>>> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes
  2019-07-23 11:27     ` Anju T Sudhakar
  2019-07-29 12:36       ` Michael Ellerman
@ 2019-10-30 18:37       ` Qian Cai
  1 sibling, 0 replies; 7+ messages in thread
From: Qian Cai @ 2019-10-30 18:37 UTC (permalink / raw)
  To: Anju T Sudhakar, Michael Ellerman, Madhavan Srinivasan; +Cc: linuxppc-dev

On Tue, 2019-07-23 at 16:57 +0530, Anju T Sudhakar wrote:
> Hi Qian,
> 
> On 7/16/19 12:11 AM, Qian Cai wrote:
> > On Thu, 2019-07-11 at 14:53 +1000, Michael Ellerman wrote:
> > > Hi Maddy,
> > > 
> > > Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
> > > > 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.

Anju, do you still plan to fix those "Directory 'imc' with parent 'powerpc'
already present!" warnings as they are still there in the latest linux-next?

> 
> 
> 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?
> 
> 
> Thanks,
> 
> Anju
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-10-30 18:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-02  9:21 [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes Madhavan Srinivasan
2019-07-11  4:53 ` Michael Ellerman
2019-07-15  6:22   ` maddy
2019-07-15 18:41   ` Qian Cai
2019-07-23 11:27     ` Anju T Sudhakar
2019-07-29 12:36       ` Michael Ellerman
2019-10-30 18:37       ` Qian Cai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.