* [PATCH v2] memory-hotplug: Fix kernel warning during memory hotplug on ppc64
@ 2015-11-03 17:21 John Allen
2015-11-03 21:13 ` Yasuaki Ishimatsu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: John Allen @ 2015-11-03 17:21 UTC (permalink / raw)
To: gregkh; +Cc: linux-kernel, linuxppc-dev, Nathan Fontenot
This patch fixes a bug where a kernel warning is triggered when performing
a memory hotplug on ppc64. This warning may also occur on any architecture
that has multiple sections per memory block.
[ 78.300767] ------------[ cut here ]------------
[ 78.300768] WARNING: at ../drivers/base/memory.c:210
[ 78.300769] Modules linked in: rpadlpar_io(X) rpaphp(X) tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag af_packet xfs libcrc32c ibmveth(X) rtc_generic btrfs xor raid6_pq xts gf128mul dm_crypt sd_mod sr_mod cdrom crc_t10dif ibmvscsi(X) scsi_transport_srp scsi_tgt dm_mod sg scsi_mod autofs4
[ 78.300789] Supported: Yes, External
[ 78.300791] CPU: 1 PID: 3090 Comm: systemd-udevd Tainted: G X 3.12.45-1-default #1
[ 78.300793] task: c0000004d7d1d970 ti: c0000004d7b90000 task.ti: c0000004d7b90000
[ 78.300794] NIP: c0000000004fcff8 LR: c0000000004fda84 CTR: 0000000000000000
[ 78.300795] REGS: c0000004d7b93930 TRAP: 0700 Tainted: G X (3.12.45-1-default)
[ 78.300796] MSR: 8000000000029033 <SF,EE,ME,IR,DR,RI,LE> CR: 24088848 XER: 00000000
[ 78.300800] CFAR: c0000000004fcf98 SOFTE: 1
GPR00: 0000000000000537 c0000004d7b93bb0 c000000000e7f200 0000000000053000
GPR04: 0000000000001000 0000000000000001 c000000000e0f200 0000000000000000
GPR08: 0000000000000000 0000000000000001 0000000000000537 00000000014dc000
GPR12: 0000000000054000 c00000000e7f0900 0000000010041040 0000000000000000
GPR16: 00000100206f0010 000000001003ff78 000000001006c824 00000000100410b0
GPR20: 000000001003ff90 000000001006c00c 000001002073cd20 00000100206f0760
GPR24: 00000100206f85a0 c00000000076d950 c0000004ef7c95e0 c0000004d7b93e00
GPR28: c0000004de601738 0000000000000001 c000000001218f80 00000000003fffff
[ 78.300818] NIP [c0000000004fcff8] memory_block_action+0x258/0x2e0
[ 78.300820] LR [c0000000004fda84] memory_subsys_online+0x54/0x100
[ 78.300821] Call Trace:
[ 78.300822] [c0000004d7b93bb0] [c000000009071ce0] 0xc000000009071ce0 (unreliable)
[ 78.300824] [c0000004d7b93c40] [c0000000004fda84] memory_subsys_online+0x54/0x100
[ 78.300826] [c0000004d7b93c70] [c0000000004df784] device_online+0xb4/0x120
[ 78.300828] [c0000004d7b93cb0] [c0000000004fd738] store_mem_state+0x88/0x220
[ 78.300830] [c0000004d7b93cf0] [c0000000004db448] dev_attr_store+0x68/0xa0
[ 78.300833] [c0000004d7b93d30] [c00000000031f938] sysfs_write_file+0xf8/0x1d0
[ 78.300835] [c0000004d7b93d90] [c00000000027d29c] vfs_write+0xec/0x250
[ 78.300837] [c0000004d7b93de0] [c00000000027dfdc] SyS_write+0x6c/0xf0
[ 78.300839] [c0000004d7b93e30] [c00000000000a17c] syscall_exit+0x0/0x7c
[ 78.300840] Instruction dump:
[ 78.300841] 780a0560 79482ea4 7ce94214 2fa70000 41de0014 7d09402a 396b4000 7907ffe3
[ 78.300844] 4082ff54 3cc2fff9 8926b83a 69290001 <0b090000> 2fa90000 40de006c 3860fff0
[ 78.300847] ---[ end trace dfec8da06ebbc762 ]---
The warning is triggered because there is a udev rule that automatically
tries to online memory after it has been added. The udev rule varies from
distro to distro, but will generally look something like:
SUBSYSTEM=="memory", ACTION=="add", ATTR{state}=="offline", ATTR{state}="online"
On any architecture that uses memory_probe_store to reserve memory,
this can interrupt the memory reservation process. This patch modifies
memory_probe_store to take the hotplug sysfs lock to prevent the online
of added memory before the completion of the probe.
Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
---
v2: Move call to unlock_device_hotplug under "out" label
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index bece691..7c50415 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -422,6 +422,10 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
return -EINVAL;
+ ret = lock_device_hotplug_sysfs();
+ if (ret)
+ return ret;
+
for (i = 0; i < sections_per_block; i++) {
nid = memory_add_physaddr_to_nid(phys_addr);
ret = add_memory(nid, phys_addr,
@@ -434,6 +438,7 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
ret = count;
out:
+ unlock_device_hotplug();
return ret;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] memory-hotplug: Fix kernel warning during memory hotplug on ppc64
2015-11-03 17:21 [PATCH v2] memory-hotplug: Fix kernel warning during memory hotplug on ppc64 John Allen
@ 2015-11-03 21:13 ` Yasuaki Ishimatsu
2015-11-09 16:35 ` Nathan Fontenot
2015-11-10 1:21 ` Michael Ellerman
2 siblings, 0 replies; 6+ messages in thread
From: Yasuaki Ishimatsu @ 2015-11-03 21:13 UTC (permalink / raw)
To: John Allen; +Cc: gregkh, linux-kernel, linuxppc-dev, Nathan Fontenot
On Tue, 3 Nov 2015 11:21:59 -0600
John Allen <jallen@linux.vnet.ibm.com> wrote:
> This patch fixes a bug where a kernel warning is triggered when performing
> a memory hotplug on ppc64. This warning may also occur on any architecture
> that has multiple sections per memory block.
>
> [ 78.300767] ------------[ cut here ]------------
> [ 78.300768] WARNING: at ../drivers/base/memory.c:210
> [ 78.300769] Modules linked in: rpadlpar_io(X) rpaphp(X) tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag af_packet xfs libcrc32c ibmveth(X) rtc_generic btrfs xor raid6_pq xts gf128mul dm_crypt sd_mod sr_mod cdrom crc_t10dif ibmvscsi(X) scsi_transport_srp scsi_tgt dm_mod sg scsi_mod autofs4
> [ 78.300789] Supported: Yes, External
> [ 78.300791] CPU: 1 PID: 3090 Comm: systemd-udevd Tainted: G X 3.12.45-1-default #1
> [ 78.300793] task: c0000004d7d1d970 ti: c0000004d7b90000 task.ti: c0000004d7b90000
> [ 78.300794] NIP: c0000000004fcff8 LR: c0000000004fda84 CTR: 0000000000000000
> [ 78.300795] REGS: c0000004d7b93930 TRAP: 0700 Tainted: G X (3.12.45-1-default)
> [ 78.300796] MSR: 8000000000029033 <SF,EE,ME,IR,DR,RI,LE> CR: 24088848 XER: 00000000
> [ 78.300800] CFAR: c0000000004fcf98 SOFTE: 1
> GPR00: 0000000000000537 c0000004d7b93bb0 c000000000e7f200 0000000000053000
> GPR04: 0000000000001000 0000000000000001 c000000000e0f200 0000000000000000
> GPR08: 0000000000000000 0000000000000001 0000000000000537 00000000014dc000
> GPR12: 0000000000054000 c00000000e7f0900 0000000010041040 0000000000000000
> GPR16: 00000100206f0010 000000001003ff78 000000001006c824 00000000100410b0
> GPR20: 000000001003ff90 000000001006c00c 000001002073cd20 00000100206f0760
> GPR24: 00000100206f85a0 c00000000076d950 c0000004ef7c95e0 c0000004d7b93e00
> GPR28: c0000004de601738 0000000000000001 c000000001218f80 00000000003fffff
> [ 78.300818] NIP [c0000000004fcff8] memory_block_action+0x258/0x2e0
> [ 78.300820] LR [c0000000004fda84] memory_subsys_online+0x54/0x100
> [ 78.300821] Call Trace:
> [ 78.300822] [c0000004d7b93bb0] [c000000009071ce0] 0xc000000009071ce0 (unreliable)
> [ 78.300824] [c0000004d7b93c40] [c0000000004fda84] memory_subsys_online+0x54/0x100
> [ 78.300826] [c0000004d7b93c70] [c0000000004df784] device_online+0xb4/0x120
> [ 78.300828] [c0000004d7b93cb0] [c0000000004fd738] store_mem_state+0x88/0x220
> [ 78.300830] [c0000004d7b93cf0] [c0000000004db448] dev_attr_store+0x68/0xa0
> [ 78.300833] [c0000004d7b93d30] [c00000000031f938] sysfs_write_file+0xf8/0x1d0
> [ 78.300835] [c0000004d7b93d90] [c00000000027d29c] vfs_write+0xec/0x250
> [ 78.300837] [c0000004d7b93de0] [c00000000027dfdc] SyS_write+0x6c/0xf0
> [ 78.300839] [c0000004d7b93e30] [c00000000000a17c] syscall_exit+0x0/0x7c
> [ 78.300840] Instruction dump:
> [ 78.300841] 780a0560 79482ea4 7ce94214 2fa70000 41de0014 7d09402a 396b4000 7907ffe3
> [ 78.300844] 4082ff54 3cc2fff9 8926b83a 69290001 <0b090000> 2fa90000 40de006c 3860fff0
> [ 78.300847] ---[ end trace dfec8da06ebbc762 ]---
>
> The warning is triggered because there is a udev rule that automatically
> tries to online memory after it has been added. The udev rule varies from
> distro to distro, but will generally look something like:
>
> SUBSYSTEM=="memory", ACTION=="add", ATTR{state}=="offline", ATTR{state}="online"
>
> On any architecture that uses memory_probe_store to reserve memory,
> this can interrupt the memory reservation process. This patch modifies
> memory_probe_store to take the hotplug sysfs lock to prevent the online
> of added memory before the completion of the probe.
>
> Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
> ---
Looks good to me.
Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Thanks,
Yasuaki Ishimatsu
> v2: Move call to unlock_device_hotplug under "out" label
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index bece691..7c50415 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -422,6 +422,10 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
> if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
> return -EINVAL;
>
> + ret = lock_device_hotplug_sysfs();
> + if (ret)
> + return ret;
> +
> for (i = 0; i < sections_per_block; i++) {
> nid = memory_add_physaddr_to_nid(phys_addr);
> ret = add_memory(nid, phys_addr,
> @@ -434,6 +438,7 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
>
> ret = count;
> out:
> + unlock_device_hotplug();
> return ret;
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] memory-hotplug: Fix kernel warning during memory hotplug on ppc64
2015-11-03 17:21 [PATCH v2] memory-hotplug: Fix kernel warning during memory hotplug on ppc64 John Allen
2015-11-03 21:13 ` Yasuaki Ishimatsu
@ 2015-11-09 16:35 ` Nathan Fontenot
2015-11-10 1:21 ` Michael Ellerman
2 siblings, 0 replies; 6+ messages in thread
From: Nathan Fontenot @ 2015-11-09 16:35 UTC (permalink / raw)
To: John Allen, gregkh; +Cc: linux-kernel, linuxppc-dev
On 11/03/2015 11:21 AM, John Allen wrote:
> This patch fixes a bug where a kernel warning is triggered when performing
> a memory hotplug on ppc64. This warning may also occur on any architecture
> that has multiple sections per memory block.
>
> [ 78.300767] ------------[ cut here ]------------
> [ 78.300768] WARNING: at ../drivers/base/memory.c:210
> [ 78.300769] Modules linked in: rpadlpar_io(X) rpaphp(X) tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag af_packet xfs libcrc32c ibmveth(X) rtc_generic btrfs xor raid6_pq xts gf128mul dm_crypt sd_mod sr_mod cdrom crc_t10dif ibmvscsi(X) scsi_transport_srp scsi_tgt dm_mod sg scsi_mod autofs4
> [ 78.300789] Supported: Yes, External
> [ 78.300791] CPU: 1 PID: 3090 Comm: systemd-udevd Tainted: G X 3.12.45-1-default #1
> [ 78.300793] task: c0000004d7d1d970 ti: c0000004d7b90000 task.ti: c0000004d7b90000
> [ 78.300794] NIP: c0000000004fcff8 LR: c0000000004fda84 CTR: 0000000000000000
> [ 78.300795] REGS: c0000004d7b93930 TRAP: 0700 Tainted: G X (3.12.45-1-default)
> [ 78.300796] MSR: 8000000000029033 <SF,EE,ME,IR,DR,RI,LE> CR: 24088848 XER: 00000000
> [ 78.300800] CFAR: c0000000004fcf98 SOFTE: 1
> GPR00: 0000000000000537 c0000004d7b93bb0 c000000000e7f200 0000000000053000
> GPR04: 0000000000001000 0000000000000001 c000000000e0f200 0000000000000000
> GPR08: 0000000000000000 0000000000000001 0000000000000537 00000000014dc000
> GPR12: 0000000000054000 c00000000e7f0900 0000000010041040 0000000000000000
> GPR16: 00000100206f0010 000000001003ff78 000000001006c824 00000000100410b0
> GPR20: 000000001003ff90 000000001006c00c 000001002073cd20 00000100206f0760
> GPR24: 00000100206f85a0 c00000000076d950 c0000004ef7c95e0 c0000004d7b93e00
> GPR28: c0000004de601738 0000000000000001 c000000001218f80 00000000003fffff
> [ 78.300818] NIP [c0000000004fcff8] memory_block_action+0x258/0x2e0
> [ 78.300820] LR [c0000000004fda84] memory_subsys_online+0x54/0x100
> [ 78.300821] Call Trace:
> [ 78.300822] [c0000004d7b93bb0] [c000000009071ce0] 0xc000000009071ce0 (unreliable)
> [ 78.300824] [c0000004d7b93c40] [c0000000004fda84] memory_subsys_online+0x54/0x100
> [ 78.300826] [c0000004d7b93c70] [c0000000004df784] device_online+0xb4/0x120
> [ 78.300828] [c0000004d7b93cb0] [c0000000004fd738] store_mem_state+0x88/0x220
> [ 78.300830] [c0000004d7b93cf0] [c0000000004db448] dev_attr_store+0x68/0xa0
> [ 78.300833] [c0000004d7b93d30] [c00000000031f938] sysfs_write_file+0xf8/0x1d0
> [ 78.300835] [c0000004d7b93d90] [c00000000027d29c] vfs_write+0xec/0x250
> [ 78.300837] [c0000004d7b93de0] [c00000000027dfdc] SyS_write+0x6c/0xf0
> [ 78.300839] [c0000004d7b93e30] [c00000000000a17c] syscall_exit+0x0/0x7c
> [ 78.300840] Instruction dump:
> [ 78.300841] 780a0560 79482ea4 7ce94214 2fa70000 41de0014 7d09402a 396b4000 7907ffe3
> [ 78.300844] 4082ff54 3cc2fff9 8926b83a 69290001 <0b090000> 2fa90000 40de006c 3860fff0
> [ 78.300847] ---[ end trace dfec8da06ebbc762 ]---
>
> The warning is triggered because there is a udev rule that automatically
> tries to online memory after it has been added. The udev rule varies from
> distro to distro, but will generally look something like:
>
> SUBSYSTEM=="memory", ACTION=="add", ATTR{state}=="offline", ATTR{state}="online"
>
> On any architecture that uses memory_probe_store to reserve memory,
> this can interrupt the memory reservation process. This patch modifies
> memory_probe_store to take the hotplug sysfs lock to prevent the online
> of added memory before the completion of the probe.
>
> Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
Reviewed-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> ---
> v2: Move call to unlock_device_hotplug under "out" label
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index bece691..7c50415 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -422,6 +422,10 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
> if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
> return -EINVAL;
>
> + ret = lock_device_hotplug_sysfs();
> + if (ret)
> + return ret;
> +
> for (i = 0; i < sections_per_block; i++) {
> nid = memory_add_physaddr_to_nid(phys_addr);
> ret = add_memory(nid, phys_addr,
> @@ -434,6 +438,7 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
>
> ret = count;
> out:
> + unlock_device_hotplug();
> return ret;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] memory-hotplug: Fix kernel warning during memory hotplug on ppc64
2015-11-03 17:21 [PATCH v2] memory-hotplug: Fix kernel warning during memory hotplug on ppc64 John Allen
@ 2015-11-10 1:21 ` Michael Ellerman
2015-11-09 16:35 ` Nathan Fontenot
2015-11-10 1:21 ` Michael Ellerman
2 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2015-11-10 1:21 UTC (permalink / raw)
To: John Allen, gregkh; +Cc: Nathan Fontenot, linuxppc-dev, linux-kernel
Hi John,
On Tue, 2015-11-03 at 11:21 -0600, John Allen wrote:
> This patch fixes a bug where a kernel warning is triggered when performing
> a memory hotplug on ppc64. This warning may also occur on any architecture
> that has multiple sections per memory block.
So it looks like the only arches that enable this code at all are powerpc, sh
and x86 (via CONFIG_ARCH_MEMORY_PROBE). And it sounds like on x86 it's just
there for debugging, ACPI is meant to notify you of memory hotplug by the
sounds.
Do any of sh or x86 have "multiple sections per memory block" ?
If not then this bug would only apply to powerpc, which would be useful to
know.
And what is the actual warning? ie. what does the code look like. My line 210
of memory.c is a printk() not a WARN.
> [ 78.300767] ------------[ cut here ]------------
> [ 78.300768] WARNING: at ../drivers/base/memory.c:210
> [ 78.300769] Modules linked in: rpadlpar_io(X) rpaphp(X) tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag af_packet xfs libcrc32c ibmveth(X) rtc_generic btrfs xor raid6_pq xts
> gf128mul dm_crypt sd_mod sr_mod cdrom crc_t10dif ibmvscsi(X) scsi_transport_srp scsi_tgt dm_mod sg scsi_mod autofs4
> [ 78.300789] Supported: Yes, External
> [ 78.300791] CPU: 1 PID: 3090 Comm: systemd-udevd Tainted: G X 3.12.45-1-default #1
> [ 78.300793] task: c0000004d7d1d970 ti: c0000004d7b90000 task.ti: c0000004d7b90000
> [ 78.300794] NIP: c0000000004fcff8 LR: c0000000004fda84 CTR: 0000000000000000
> [ 78.300795] REGS: c0000004d7b93930 TRAP: 0700 Tainted: G X (3.12.45-1-default)
> [ 78.300796] MSR: 8000000000029033 <SF,EE,ME,IR,DR,RI,LE> CR: 24088848 XER: 00000000
> [ 78.300800] CFAR: c0000000004fcf98 SOFTE: 1
> GPR00: 0000000000000537 c0000004d7b93bb0 c000000000e7f200 0000000000053000
> GPR04: 0000000000001000 0000000000000001 c000000000e0f200 0000000000000000
> GPR08: 0000000000000000 0000000000000001 0000000000000537 00000000014dc000
> GPR12: 0000000000054000 c00000000e7f0900 0000000010041040 0000000000000000
> GPR16: 00000100206f0010 000000001003ff78 000000001006c824 00000000100410b0
> GPR20: 000000001003ff90 000000001006c00c 000001002073cd20 00000100206f0760
> GPR24: 00000100206f85a0 c00000000076d950 c0000004ef7c95e0 c0000004d7b93e00
> GPR28: c0000004de601738 0000000000000001 c000000001218f80 00000000003fffff
> [ 78.300818] NIP [c0000000004fcff8] memory_block_action+0x258/0x2e0
> [ 78.300820] LR [c0000000004fda84] memory_subsys_online+0x54/0x100
> [ 78.300821] Call Trace:
> [ 78.300822] [c0000004d7b93bb0] [c000000009071ce0] 0xc000000009071ce0 (unreliable)
> [ 78.300824] [c0000004d7b93c40] [c0000000004fda84] memory_subsys_online+0x54/0x100
> [ 78.300826] [c0000004d7b93c70] [c0000000004df784] device_online+0xb4/0x120
> [ 78.300828] [c0000004d7b93cb0] [c0000000004fd738] store_mem_state+0x88/0x220
> [ 78.300830] [c0000004d7b93cf0] [c0000000004db448] dev_attr_store+0x68/0xa0
> [ 78.300833] [c0000004d7b93d30] [c00000000031f938] sysfs_write_file+0xf8/0x1d0
> [ 78.300835] [c0000004d7b93d90] [c00000000027d29c] vfs_write+0xec/0x250
> [ 78.300837] [c0000004d7b93de0] [c00000000027dfdc] SyS_write+0x6c/0xf0
> [ 78.300839] [c0000004d7b93e30] [c00000000000a17c] syscall_exit+0x0/0x7c
> [ 78.300840] Instruction dump:
> [ 78.300841] 780a0560 79482ea4 7ce94214 2fa70000 41de0014 7d09402a 396b4000 7907ffe3
> [ 78.300844] 4082ff54 3cc2fff9 8926b83a 69290001 <0b090000> 2fa90000 40de006c 3860fff0
> [ 78.300847] ---[ end trace dfec8da06ebbc762 ]---
For change logs I think it's nice to trim the oops a bit. Others probably have
different opinions but I'd remove the printk timestamp, the GPRs and some of
the other regs and the instruction dump, so more like:
WARNING: at ../drivers/base/memory.c:210
CPU: 1 PID: 3090 Comm: systemd-udevd Tainted: G X 3.12.45-1-default #1
NIP [c0000000004fcff8] memory_block_action+0x258/0x2e0
LR [c0000000004fda84] memory_subsys_online+0x54/0x100
Call Trace:
[c0000004d7b93bb0] [c000000009071ce0] 0xc000000009071ce0 (unreliable)
[c0000004d7b93c40] [c0000000004fda84] memory_subsys_online+0x54/0x100
[c0000004d7b93c70] [c0000000004df784] device_online+0xb4/0x120
[c0000004d7b93cb0] [c0000000004fd738] store_mem_state+0x88/0x220
[c0000004d7b93cf0] [c0000000004db448] dev_attr_store+0x68/0xa0
[c0000004d7b93d30] [c00000000031f938] sysfs_write_file+0xf8/0x1d0
[c0000004d7b93d90] [c00000000027d29c] vfs_write+0xec/0x250
[c0000004d7b93de0] [c00000000027dfdc] SyS_write+0x6c/0xf0
[c0000004d7b93e30] [c00000000000a17c] syscall_exit+0x0/0x7c
Also looking at the trace, it's from 3.12.45-1, which is pretty old. Have you
also tested on mainline?
> The warning is triggered because there is a udev rule that automatically
> tries to online memory after it has been added. The udev rule varies from
> distro to distro, but will generally look something like:
>
> SUBSYSTEM=="memory", ACTION=="add", ATTR{state}=="offline", ATTR{state}="online"
>
> On any architecture that uses memory_probe_store to reserve memory,
> this can interrupt the memory reservation process. This patch modifies
> memory_probe_store to take the hotplug sysfs lock to prevent the online
> of added memory before the completion of the probe.
So presumably it's add_memory() that is causing a uevent to fire? I can't see
where that happens but I guess it's in there somewhere.
And that's a problem because we've only added one (or some) of the sections, so
the memory block is not fully populated, and then the add path hits the warning
because of that. Am I right?
> Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
> ---
> v2: Move call to unlock_device_hotplug under "out" label
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index bece691..7c50415 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -422,6 +422,10 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
> if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
> return -EINVAL;
>
> + ret = lock_device_hotplug_sysfs();
> + if (ret)
> + return ret;
> +
> for (i = 0; i < sections_per_block; i++) {
> nid = memory_add_physaddr_to_nid(phys_addr);
> ret = add_memory(nid, phys_addr,
> @@ -434,6 +438,7 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
>
> ret = count;
> out:
> + unlock_device_hotplug();
> return ret;
> }
>
This looks OK. What I don't know is what the locking rules in add_memory() are.
It takes the mem_hotplug lock, which is also a mutex. I suspect it's fine to
nest the mem_hotplug lock within the sysfs one, but it would be good if you can
try and confirm. Also if you run with lockdep enabled it should tell you if
you've got it wrong.
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] memory-hotplug: Fix kernel warning during memory hotplug on ppc64
@ 2015-11-10 1:21 ` Michael Ellerman
0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2015-11-10 1:21 UTC (permalink / raw)
To: John Allen, gregkh; +Cc: Nathan Fontenot, linuxppc-dev, linux-kernel
Hi John,
On Tue, 2015-11-03 at 11:21 -0600, John Allen wrote:
> This patch fixes a bug where a kernel warning is triggered when performing
> a memory hotplug on ppc64. This warning may also occur on any architecture
> that has multiple sections per memory block.
So it looks like the only arches that enable this code at all are powerpc, sh
and x86 (via CONFIG_ARCH_MEMORY_PROBE). And it sounds like on x86 it's just
there for debugging, ACPI is meant to notify you of memory hotplug by the
sounds.
Do any of sh or x86 have "multiple sections per memory block" ?
If not then this bug would only apply to powerpc, which would be useful to
know.
And what is the actual warning? ie. what does the code look like. My line 210
of memory.c is a printk() not a WARN.
> [ 78.300767] ------------[ cut here ]------------
> [ 78.300768] WARNING: at ../drivers/base/memory.c:210
> [ 78.300769] Modules linked in: rpadlpar_io(X) rpaphp(X) tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag af_packet xfs libcrc32c ibmveth(X) rtc_generic btrfs xor raid6_pq xts
> gf128mul dm_crypt sd_mod sr_mod cdrom crc_t10dif ibmvscsi(X) scsi_transport_srp scsi_tgt dm_mod sg scsi_mod autofs4
> [ 78.300789] Supported: Yes, External
> [ 78.300791] CPU: 1 PID: 3090 Comm: systemd-udevd Tainted: G X 3.12.45-1-default #1
> [ 78.300793] task: c0000004d7d1d970 ti: c0000004d7b90000 task.ti: c0000004d7b90000
> [ 78.300794] NIP: c0000000004fcff8 LR: c0000000004fda84 CTR: 0000000000000000
> [ 78.300795] REGS: c0000004d7b93930 TRAP: 0700 Tainted: G X (3.12.45-1-default)
> [ 78.300796] MSR: 8000000000029033 <SF,EE,ME,IR,DR,RI,LE> CR: 24088848 XER: 00000000
> [ 78.300800] CFAR: c0000000004fcf98 SOFTE: 1
> GPR00: 0000000000000537 c0000004d7b93bb0 c000000000e7f200 0000000000053000
> GPR04: 0000000000001000 0000000000000001 c000000000e0f200 0000000000000000
> GPR08: 0000000000000000 0000000000000001 0000000000000537 00000000014dc000
> GPR12: 0000000000054000 c00000000e7f0900 0000000010041040 0000000000000000
> GPR16: 00000100206f0010 000000001003ff78 000000001006c824 00000000100410b0
> GPR20: 000000001003ff90 000000001006c00c 000001002073cd20 00000100206f0760
> GPR24: 00000100206f85a0 c00000000076d950 c0000004ef7c95e0 c0000004d7b93e00
> GPR28: c0000004de601738 0000000000000001 c000000001218f80 00000000003fffff
> [ 78.300818] NIP [c0000000004fcff8] memory_block_action+0x258/0x2e0
> [ 78.300820] LR [c0000000004fda84] memory_subsys_online+0x54/0x100
> [ 78.300821] Call Trace:
> [ 78.300822] [c0000004d7b93bb0] [c000000009071ce0] 0xc000000009071ce0 (unreliable)
> [ 78.300824] [c0000004d7b93c40] [c0000000004fda84] memory_subsys_online+0x54/0x100
> [ 78.300826] [c0000004d7b93c70] [c0000000004df784] device_online+0xb4/0x120
> [ 78.300828] [c0000004d7b93cb0] [c0000000004fd738] store_mem_state+0x88/0x220
> [ 78.300830] [c0000004d7b93cf0] [c0000000004db448] dev_attr_store+0x68/0xa0
> [ 78.300833] [c0000004d7b93d30] [c00000000031f938] sysfs_write_file+0xf8/0x1d0
> [ 78.300835] [c0000004d7b93d90] [c00000000027d29c] vfs_write+0xec/0x250
> [ 78.300837] [c0000004d7b93de0] [c00000000027dfdc] SyS_write+0x6c/0xf0
> [ 78.300839] [c0000004d7b93e30] [c00000000000a17c] syscall_exit+0x0/0x7c
> [ 78.300840] Instruction dump:
> [ 78.300841] 780a0560 79482ea4 7ce94214 2fa70000 41de0014 7d09402a 396b4000 7907ffe3
> [ 78.300844] 4082ff54 3cc2fff9 8926b83a 69290001 <0b090000> 2fa90000 40de006c 3860fff0
> [ 78.300847] ---[ end trace dfec8da06ebbc762 ]---
For change logs I think it's nice to trim the oops a bit. Others probably have
different opinions but I'd remove the printk timestamp, the GPRs and some of
the other regs and the instruction dump, so more like:
WARNING: at ../drivers/base/memory.c:210
CPU: 1 PID: 3090 Comm: systemd-udevd Tainted: G X 3.12.45-1-default #1
NIP [c0000000004fcff8] memory_block_action+0x258/0x2e0
LR [c0000000004fda84] memory_subsys_online+0x54/0x100
Call Trace:
[c0000004d7b93bb0] [c000000009071ce0] 0xc000000009071ce0 (unreliable)
[c0000004d7b93c40] [c0000000004fda84] memory_subsys_online+0x54/0x100
[c0000004d7b93c70] [c0000000004df784] device_online+0xb4/0x120
[c0000004d7b93cb0] [c0000000004fd738] store_mem_state+0x88/0x220
[c0000004d7b93cf0] [c0000000004db448] dev_attr_store+0x68/0xa0
[c0000004d7b93d30] [c00000000031f938] sysfs_write_file+0xf8/0x1d0
[c0000004d7b93d90] [c00000000027d29c] vfs_write+0xec/0x250
[c0000004d7b93de0] [c00000000027dfdc] SyS_write+0x6c/0xf0
[c0000004d7b93e30] [c00000000000a17c] syscall_exit+0x0/0x7c
Also looking at the trace, it's from 3.12.45-1, which is pretty old. Have you
also tested on mainline?
> The warning is triggered because there is a udev rule that automatically
> tries to online memory after it has been added. The udev rule varies from
> distro to distro, but will generally look something like:
>
> SUBSYSTEM=="memory", ACTION=="add", ATTR{state}=="offline", ATTR{state}="online"
>
> On any architecture that uses memory_probe_store to reserve memory,
> this can interrupt the memory reservation process. This patch modifies
> memory_probe_store to take the hotplug sysfs lock to prevent the online
> of added memory before the completion of the probe.
So presumably it's add_memory() that is causing a uevent to fire? I can't see
where that happens but I guess it's in there somewhere.
And that's a problem because we've only added one (or some) of the sections, so
the memory block is not fully populated, and then the add path hits the warning
because of that. Am I right?
> Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
> ---
> v2: Move call to unlock_device_hotplug under "out" label
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index bece691..7c50415 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -422,6 +422,10 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
> if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
> return -EINVAL;
>
> + ret = lock_device_hotplug_sysfs();
> + if (ret)
> + return ret;
> +
> for (i = 0; i < sections_per_block; i++) {
> nid = memory_add_physaddr_to_nid(phys_addr);
> ret = add_memory(nid, phys_addr,
> @@ -434,6 +438,7 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
>
> ret = count;
> out:
> + unlock_device_hotplug();
> return ret;
> }
>
This looks OK. What I don't know is what the locking rules in add_memory() are.
It takes the mem_hotplug lock, which is also a mutex. I suspect it's fine to
nest the mem_hotplug lock within the sysfs one, but it would be good if you can
try and confirm. Also if you run with lockdep enabled it should tell you if
you've got it wrong.
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] memory-hotplug: Fix kernel warning during memory hotplug on ppc64
2015-11-10 1:21 ` Michael Ellerman
(?)
@ 2015-11-24 20:15 ` John Allen
-1 siblings, 0 replies; 6+ messages in thread
From: John Allen @ 2015-11-24 20:15 UTC (permalink / raw)
To: Michael Ellerman, gregkh; +Cc: Nathan Fontenot, linuxppc-dev, linux-kernel
Hi Michael,
On 11/09/2015 07:21 PM, Michael Ellerman wrote:
> Hi John,
>
> On Tue, 2015-11-03 at 11:21 -0600, John Allen wrote:
>> This patch fixes a bug where a kernel warning is triggered when performing
>> a memory hotplug on ppc64. This warning may also occur on any architecture
>> that has multiple sections per memory block.
>
> So it looks like the only arches that enable this code at all are powerpc, sh
> and x86 (via CONFIG_ARCH_MEMORY_PROBE). And it sounds like on x86 it's just
> there for debugging, ACPI is meant to notify you of memory hotplug by the
> sounds.
>
> Do any of sh or x86 have "multiple sections per memory block" ?
>
> If not then this bug would only apply to powerpc, which would be useful to
> know.
Sorry for the delayed response. It looks like your reply got buried in my
lkml folder instead of going straight to my inbox.
My understanding is that x86 may use multiple sections per block. I
think we would have to assume that any architecture that uses this code
could potentially hit this issue.
>
> And what is the actual warning? ie. what does the code look like. My line 210
> of memory.c is a printk() not a WARN.
In mainline, the warning is at line 200:
if (WARN_ON_ONCE(!pfn_valid(pfn)))
return false;
>
>
>> [ 78.300767] ------------[ cut here ]------------
>> [ 78.300768] WARNING: at ../drivers/base/memory.c:210
>> [ 78.300769] Modules linked in: rpadlpar_io(X) rpaphp(X) tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag af_packet xfs libcrc32c ibmveth(X) rtc_generic btrfs xor raid6_pq xts
>> gf128mul dm_crypt sd_mod sr_mod cdrom crc_t10dif ibmvscsi(X) scsi_transport_srp scsi_tgt dm_mod sg scsi_mod autofs4
>> [ 78.300789] Supported: Yes, External
>> [ 78.300791] CPU: 1 PID: 3090 Comm: systemd-udevd Tainted: G X 3.12.45-1-default #1
>> [ 78.300793] task: c0000004d7d1d970 ti: c0000004d7b90000 task.ti: c0000004d7b90000
>> [ 78.300794] NIP: c0000000004fcff8 LR: c0000000004fda84 CTR: 0000000000000000
>> [ 78.300795] REGS: c0000004d7b93930 TRAP: 0700 Tainted: G X (3.12.45-1-default)
>> [ 78.300796] MSR: 8000000000029033 <SF,EE,ME,IR,DR,RI,LE> CR: 24088848 XER: 00000000
>> [ 78.300800] CFAR: c0000000004fcf98 SOFTE: 1
>> GPR00: 0000000000000537 c0000004d7b93bb0 c000000000e7f200 0000000000053000
>> GPR04: 0000000000001000 0000000000000001 c000000000e0f200 0000000000000000
>> GPR08: 0000000000000000 0000000000000001 0000000000000537 00000000014dc000
>> GPR12: 0000000000054000 c00000000e7f0900 0000000010041040 0000000000000000
>> GPR16: 00000100206f0010 000000001003ff78 000000001006c824 00000000100410b0
>> GPR20: 000000001003ff90 000000001006c00c 000001002073cd20 00000100206f0760
>> GPR24: 00000100206f85a0 c00000000076d950 c0000004ef7c95e0 c0000004d7b93e00
>> GPR28: c0000004de601738 0000000000000001 c000000001218f80 00000000003fffff
>> [ 78.300818] NIP [c0000000004fcff8] memory_block_action+0x258/0x2e0
>> [ 78.300820] LR [c0000000004fda84] memory_subsys_online+0x54/0x100
>> [ 78.300821] Call Trace:
>> [ 78.300822] [c0000004d7b93bb0] [c000000009071ce0] 0xc000000009071ce0 (unreliable)
>> [ 78.300824] [c0000004d7b93c40] [c0000000004fda84] memory_subsys_online+0x54/0x100
>> [ 78.300826] [c0000004d7b93c70] [c0000000004df784] device_online+0xb4/0x120
>> [ 78.300828] [c0000004d7b93cb0] [c0000000004fd738] store_mem_state+0x88/0x220
>> [ 78.300830] [c0000004d7b93cf0] [c0000000004db448] dev_attr_store+0x68/0xa0
>> [ 78.300833] [c0000004d7b93d30] [c00000000031f938] sysfs_write_file+0xf8/0x1d0
>> [ 78.300835] [c0000004d7b93d90] [c00000000027d29c] vfs_write+0xec/0x250
>> [ 78.300837] [c0000004d7b93de0] [c00000000027dfdc] SyS_write+0x6c/0xf0
>> [ 78.300839] [c0000004d7b93e30] [c00000000000a17c] syscall_exit+0x0/0x7c
>> [ 78.300840] Instruction dump:
>> [ 78.300841] 780a0560 79482ea4 7ce94214 2fa70000 41de0014 7d09402a 396b4000 7907ffe3
>> [ 78.300844] 4082ff54 3cc2fff9 8926b83a 69290001 <0b090000> 2fa90000 40de006c 3860fff0
>> [ 78.300847] ---[ end trace dfec8da06ebbc762 ]---
>
> For change logs I think it's nice to trim the oops a bit. Others probably have
> different opinions but I'd remove the printk timestamp, the GPRs and some of
> the other regs and the instruction dump, so more like:
>
> WARNING: at ../drivers/base/memory.c:210
> CPU: 1 PID: 3090 Comm: systemd-udevd Tainted: G X 3.12.45-1-default #1
> NIP [c0000000004fcff8] memory_block_action+0x258/0x2e0
> LR [c0000000004fda84] memory_subsys_online+0x54/0x100
> Call Trace:
> [c0000004d7b93bb0] [c000000009071ce0] 0xc000000009071ce0 (unreliable)
> [c0000004d7b93c40] [c0000000004fda84] memory_subsys_online+0x54/0x100
> [c0000004d7b93c70] [c0000000004df784] device_online+0xb4/0x120
> [c0000004d7b93cb0] [c0000000004fd738] store_mem_state+0x88/0x220
> [c0000004d7b93cf0] [c0000000004db448] dev_attr_store+0x68/0xa0
> [c0000004d7b93d30] [c00000000031f938] sysfs_write_file+0xf8/0x1d0
> [c0000004d7b93d90] [c00000000027d29c] vfs_write+0xec/0x250
> [c0000004d7b93de0] [c00000000027dfdc] SyS_write+0x6c/0xf0
> [c0000004d7b93e30] [c00000000000a17c] syscall_exit+0x0/0x7c
>
>
> Also looking at the trace, it's from 3.12.45-1, which is pretty old. Have you
> also tested on mainline?
All testing and debugging was done on a mainline kernel. The traces with
the 3.12 kernel were just what was submitted in the initial bug report.
>
>> The warning is triggered because there is a udev rule that automatically
>> tries to online memory after it has been added. The udev rule varies from
>> distro to distro, but will generally look something like:
>>
>> SUBSYSTEM=="memory", ACTION=="add", ATTR{state}=="offline", ATTR{state}="online"
>>
>> On any architecture that uses memory_probe_store to reserve memory,
>> this can interrupt the memory reservation process. This patch modifies
>> memory_probe_store to take the hotplug sysfs lock to prevent the online
>> of added memory before the completion of the probe.
>
> So presumably it's add_memory() that is causing a uevent to fire? I can't see
> where that happens but I guess it's in there somewhere.
>
> And that's a problem because we've only added one (or some) of the sections, so
> the memory block is not fully populated, and then the add path hits the warning
> because of that. Am I right?
Yes, that's right. While we are in the process of reserving the sections of the
block,the uevent gets triggered and calls store_mem_state(). This then calls
pages_correctly_reserved() which finds that the block has only been partially
reserved, triggering the warning.
>
>> Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
>> ---
>> v2: Move call to unlock_device_hotplug under "out" label
>>
>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>> index bece691..7c50415 100644
>> --- a/drivers/base/memory.c
>> +++ b/drivers/base/memory.c
>> @@ -422,6 +422,10 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
>> if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
>> return -EINVAL;
>>
>> + ret = lock_device_hotplug_sysfs();
>> + if (ret)
>> + return ret;
>> +
>> for (i = 0; i < sections_per_block; i++) {
>> nid = memory_add_physaddr_to_nid(phys_addr);
>> ret = add_memory(nid, phys_addr,
>> @@ -434,6 +438,7 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
>>
>> ret = count;
>> out:
>> + unlock_device_hotplug();
>> return ret;
>> }
>>
>
> This looks OK. What I don't know is what the locking rules in add_memory() are.
> It takes the mem_hotplug lock, which is also a mutex. I suspect it's fine to
> nest the mem_hotplug lock within the sysfs one, but it would be good if you can
> try and confirm. Also if you run with lockdep enabled it should tell you if
> you've got it wrong.
My assumption is that nesting the locks in this way is alright. We can see this
same nesting structure used in store_mem_state(). I never ran into any locking
issues in my testing, but I can test with lockdep enabled if you think that's
necessary.
-John
>
> cheers
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-24 20:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-03 17:21 [PATCH v2] memory-hotplug: Fix kernel warning during memory hotplug on ppc64 John Allen
2015-11-03 21:13 ` Yasuaki Ishimatsu
2015-11-09 16:35 ` Nathan Fontenot
2015-11-10 1:21 ` Michael Ellerman
2015-11-10 1:21 ` Michael Ellerman
2015-11-24 20:15 ` John Allen
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.