kdump: No udev events for memory hotplug?

kdump: No udev events for memory hotplug?

[Michael Holzheu]

Hello Vivek and Eric,

I noticed that on my system kernel 3.1 *no* udev events for memory
hotplug are generated. Same on my RHEL6.1.

# udevadm monitor
# echo offline > /sys/devices/system/memory/memory4/state

-> No event

But we need the udev events in order to do a kdump reload for setting up
the ELF loads correctly.

In my /etc/udev/rules.d/98-kexec.rules there are rules for memory
hotplug:

SUBSYSTEM=="memory", ACTION=="add", PROGRAM="/etc/init.d/kdump restart"
SUBSYSTEM=="memory", ACTION=="remove", PROGRAM="/etc/init.d/kdump
restart"

Perhaps/probably I am missing something?

Michael

Re: kdump: No udev events for memory hotplug?

[Vivek Goyal]

On Wed, Oct 26, 2011 at 06:08:12PM +0200, Michael Holzheu wrote:
> Hello Vivek and Eric,
> 
> I noticed that on my system kernel 3.1 *no* udev events for memory
> hotplug are generated. Same on my RHEL6.1.
> 
> # udevadm monitor
> # echo offline > /sys/devices/system/memory/memory4/state
> 
> -> No event
> 
> But we need the udev events in order to do a kdump reload for setting up
> the ELF loads correctly.
> 
> In my /etc/udev/rules.d/98-kexec.rules there are rules for memory
> hotplug:
> 
> SUBSYSTEM=="memory", ACTION=="add", PROGRAM="/etc/init.d/kdump restart"
> SUBSYSTEM=="memory", ACTION=="remove", PROGRAM="/etc/init.d/kdump
> restart"
> 
> Perhaps/probably I am missing something?

I don't know. Sounds like a bug. I have never looked into it. CCing Kay,
if he has any thoughts.

Are any events generated for memory add?

Thanks
Vivek

Re: kdump: No udev events for memory hotplug?

[Heiko Carstens]

On Wed, Oct 26, 2011 at 03:24:49PM -0400, Vivek Goyal wrote:
> On Wed, Oct 26, 2011 at 06:08:12PM +0200, Michael Holzheu wrote:
> > Hello Vivek and Eric,
> > 
> > I noticed that on my system kernel 3.1 *no* udev events for memory
> > hotplug are generated. Same on my RHEL6.1.
> > 
> > # udevadm monitor
> > # echo offline > /sys/devices/system/memory/memory4/state
> > 
> > -> No event
> > 
> > But we need the udev events in order to do a kdump reload for setting up
> > the ELF loads correctly.
> > 
> > In my /etc/udev/rules.d/98-kexec.rules there are rules for memory
> > hotplug:
> > 
> > SUBSYSTEM=="memory", ACTION=="add", PROGRAM="/etc/init.d/kdump restart"
> > SUBSYSTEM=="memory", ACTION=="remove", PROGRAM="/etc/init.d/kdump
> > restart"
> > 
> > Perhaps/probably I am missing something?
> 
> I don't know. Sounds like a bug. I have never looked into it. CCing Kay,
> if he has any thoughts.
> 
> Are any events generated for memory add?

Looks like uevents are only genereted when memory gets registered and
unregistered, but not when when it gets set online or offline.
To achieve that you would need to add similar code to
store_mem_state()/memory_block_change_state() in drivers/base/memory.c
like we have it already in store_online() in drivers/base/cpu.c

[PATCH] kdump: Add udev events for memory online/offline

[Michael Holzheu]

From: Michael Holzheu <holzheu@linux.vnet.ibm.com>

Currently no udev events for memory hotplug "online" and "offline" are
generated:

# udevadm monitor
# echo offline > /sys/devices/system/memory/memory4/state
==> No event

When kdump is loaded, kexec detects the current memory configuration and
stores it in the pre-allocated ELF core header. Therefore, for kdump it is
necessary to reload the kdump kernel with kexec when the memory
configuration changes (e.g. for online/offline hotplug memory).

In order to do this automatically, udev rules should be used. This kernel
patch adds udev events for "online" and "offline". Together with this kernel
patch, the following udev rules for online/offline have to be added to
"/etc/udev/rules.d/98-kexec.rules":

SUBSYSTEM=="memory", ACTION=="online", PROGRAM="/etc/init.d/kdump restart"
SUBSYSTEM=="memory", ACTION=="offline", PROGRAM="/etc/init.d/kdump restart"

Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 drivers/base/memory.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -310,11 +310,15 @@ store_mem_state(struct sys_device *dev,
 
 	mem = container_of(dev, struct memory_block, sysdev);
 
-	if (!strncmp(buf, "online", min((int)count, 6)))
+	if (!strncmp(buf, "online", min((int)count, 6))) {
 		ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
-	else if(!strncmp(buf, "offline", min((int)count, 7)))
+		if (ret == 0)
+			kobject_uevent(&dev->kobj, KOBJ_ONLINE);
+	} else if (!strncmp(buf, "offline", min((int)count, 7))) {
 		ret = memory_block_change_state(mem, MEM_OFFLINE, MEM_ONLINE);
-
+		if (ret == 0)
+			kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
+	}
 	if (ret)
 		return ret;
 	return count;

Re: [PATCH] kdump: Add udev events for memory online/offline

[Andrew Morton]

On Thu, 27 Oct 2011 11:32:45 +0200
Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:

> From: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> 
> Currently no udev events for memory hotplug "online" and "offline" are
> generated:
> 
> # udevadm monitor
> # echo offline > /sys/devices/system/memory/memory4/state
> ==> No event
> 
> When kdump is loaded, kexec detects the current memory configuration and
> stores it in the pre-allocated ELF core header. Therefore, for kdump it is
> necessary to reload the kdump kernel with kexec when the memory
> configuration changes (e.g. for online/offline hotplug memory).
> 
> In order to do this automatically, udev rules should be used. This kernel
> patch adds udev events for "online" and "offline". Together with this kernel
> patch, the following udev rules for online/offline have to be added to
> "/etc/udev/rules.d/98-kexec.rules":
> 
> SUBSYSTEM=="memory", ACTION=="online", PROGRAM="/etc/init.d/kdump restart"
> SUBSYSTEM=="memory", ACTION=="offline", PROGRAM="/etc/init.d/kdump restart"
> 
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> ---
>  drivers/base/memory.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -310,11 +310,15 @@ store_mem_state(struct sys_device *dev,
>  
>  	mem = container_of(dev, struct memory_block, sysdev);
>  
> -	if (!strncmp(buf, "online", min((int)count, 6)))
> +	if (!strncmp(buf, "online", min((int)count, 6))) {
>  		ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
> -	else if(!strncmp(buf, "offline", min((int)count, 7)))
> +		if (ret == 0)
> +			kobject_uevent(&dev->kobj, KOBJ_ONLINE);
> +	} else if (!strncmp(buf, "offline", min((int)count, 7))) {
>  		ret = memory_block_change_state(mem, MEM_OFFLINE, MEM_ONLINE);
> -
> +		if (ret == 0)
> +			kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
> +	}

ot: what on earth is up with that min() thing which
Dave-who-doesn't-know-about-min_t added to the strncmp() calls?

Re: [PATCH] kdump: Add udev events for memory online/offline

[Dave Hansen]

On Fri, 2011-10-28 at 15:41 -0700, Andrew Morton wrote:
> > +     } else if (!strncmp(buf, "offline", min((int)count, 7))) {
> >               ret = memory_block_change_state(mem, MEM_OFFLINE, MEM_ONLINE);
> > -
> > +             if (ret == 0)
> > +                     kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
> > +     }
> 
> ot: what on earth is up with that min() thing which
> Dave-who-doesn't-know-about-min_t added to the strncmp() calls?

My first reaction was, "that's not my code!", but git seems to disagree
with me.  So I'll go and blame it on being young and stupid in 2005.

Reading it, I also think it's probably buggy.

	echo -n "o" > state
 
would be interesting.  I think it'll fall in to the 'online' case.
While probably not that common, it's certainly unexpected.  I'll send a
patch to fix it up.

-- Dave

Re: [PATCH] kdump: Add udev events for memory online/offline

[Andrew Morton]

On Thu, 27 Oct 2011 11:32:45 +0200
Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:

> From: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> 
> Currently no udev events for memory hotplug "online" and "offline" are
> generated:
> 
> # udevadm monitor
> # echo offline > /sys/devices/system/memory/memory4/state
> ==> No event
> 
> When kdump is loaded, kexec detects the current memory configuration and
> stores it in the pre-allocated ELF core header. Therefore, for kdump it is
> necessary to reload the kdump kernel with kexec when the memory
> configuration changes (e.g. for online/offline hotplug memory).
> 
> In order to do this automatically, udev rules should be used. This kernel
> patch adds udev events for "online" and "offline". Together with this kernel
> patch, the following udev rules for online/offline have to be added to
> "/etc/udev/rules.d/98-kexec.rules":
> 
> SUBSYSTEM=="memory", ACTION=="online", PROGRAM="/etc/init.d/kdump restart"
> SUBSYSTEM=="memory", ACTION=="offline", PROGRAM="/etc/init.d/kdump restart"
> 
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> ---
>  drivers/base/memory.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -310,11 +310,15 @@ store_mem_state(struct sys_device *dev,
>  
>  	mem = container_of(dev, struct memory_block, sysdev);
>  
> -	if (!strncmp(buf, "online", min((int)count, 6)))
> +	if (!strncmp(buf, "online", min((int)count, 6))) {
>  		ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
> -	else if(!strncmp(buf, "offline", min((int)count, 7)))
> +		if (ret == 0)
> +			kobject_uevent(&dev->kobj, KOBJ_ONLINE);
> +	} else if (!strncmp(buf, "offline", min((int)count, 7))) {
>  		ret = memory_block_change_state(mem, MEM_OFFLINE, MEM_ONLINE);
> -
> +		if (ret == 0)
> +			kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
> +	}
>  	if (ret)
>  		return ret;
>  	return count;

I think a safer place from which to send the uevent is
memory_block_change_state() or even memory_block_action().  Because if
either of those functions later gets new callers, those callers might
forget to send the uevent?

[PATCH v2] kdump: Add udev events for memory online/offline

[Michael Holzheu]

On Fri, 2011-10-28 at 15:46 -0700, Andrew Morton wrote:
> On Thu, 27 Oct 2011 11:32:45 +0200

[snip]

> I think a safer place from which to send the uevent is
> memory_block_change_state() or even memory_block_action().  Because if
> either of those functions later gets new callers, those callers might
> forget to send the uevent?

Ok fine. I put the code into memory_block_change_state(). This also has
the advantage that we are serialized by "mem->state_mutex". Not sure if
we need that, but for CPU hotplug the udev events are also serialized
with a lock. See  "drivers/base/cpu.c" -> cpu_hotplug_driver_lock()

Do you think the following patch is acceptable?
---
From: Michael Holzheu <holzheu@linux.vnet.ibm.com>

Currently no udev events for memory hotplug "online" and "offline" are
generated:

# udevadm monitor
# echo offline > /sys/devices/system/memory/memory4/state
==> No event

When kdump is loaded, kexec detects the current memory configuration and
stores it in the pre-allocated ELF core header. Therefore, for kdump it is
necessary to reload the kdump kernel with kexec when the memory
configuration changes (e.g. for online/offline hotplug memory).

In order to do this automatically, udev rules should be used. This kernel
patch adds udev events for "online" and "offline". Together with this kernel
patch, the following udev rules for online/offline have to be added to
"/etc/udev/rules.d/98-kexec.rules":

SUBSYSTEM=="memory", ACTION=="online", PROGRAM="/etc/init.d/kdump restart"
SUBSYSTEM=="memory", ACTION=="offline", PROGRAM="/etc/init.d/kdump restart"

Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 drivers/base/memory.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -291,11 +291,22 @@ static int memory_block_change_state(str
 
 	ret = memory_block_action(mem->start_section_nr, to_state);
 
-	if (ret)
+	if (ret) {
 		mem->state = from_state_req;
-	else
-		mem->state = to_state;
+		goto out;
+	}
 
+	mem->state = to_state;
+	switch (mem->state) {
+	case MEM_OFFLINE:
+		kobject_uevent(&mem->sysdev.kobj, KOBJ_OFFLINE);
+		break;
+	case MEM_ONLINE:
+		kobject_uevent(&mem->sysdev.kobj, KOBJ_ONLINE);
+		break;
+	default:
+		break;
+	}
 out:
 	mutex_unlock(&mem->state_mutex);
 	return ret;

Re: [PATCH v2] kdump: Add udev events for memory online/offline

[WANG Cong]

On Mon, 31 Oct 2011 14:21:46 +0100, Michael Holzheu wrote:

> On Fri, 2011-10-28 at 15:46 -0700, Andrew Morton wrote:
>> On Thu, 27 Oct 2011 11:32:45 +0200
> 
> [snip]
> 
>> I think a safer place from which to send the uevent is
>> memory_block_change_state() or even memory_block_action().  Because if
>> either of those functions later gets new callers, those callers might
>> forget to send the uevent?
> 
> Ok fine. I put the code into memory_block_change_state(). This also has
> the advantage that we are serialized by "mem->state_mutex". Not sure if
> we need that, but for CPU hotplug the udev events are also serialized
> with a lock. See  "drivers/base/cpu.c" -> cpu_hotplug_driver_lock()
> 
> Do you think the following patch is acceptable? 


Yes.

> ---
> From: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> 
> Currently no udev events for memory hotplug "online" and "offline" are
> generated:
> 
> # udevadm monitor
> # echo offline > /sys/devices/system/memory/memory4/state ==> No event
> 
> When kdump is loaded, kexec detects the current memory configuration and
> stores it in the pre-allocated ELF core header. Therefore, for kdump it
> is necessary to reload the kdump kernel with kexec when the memory
> configuration changes (e.g. for online/offline hotplug memory).
> 
> In order to do this automatically, udev rules should be used. This
> kernel patch adds udev events for "online" and "offline". Together with
> this kernel patch, the following udev rules for online/offline have to
> be added to "/etc/udev/rules.d/98-kexec.rules":
> 
> SUBSYSTEM=="memory", ACTION=="online", PROGRAM="/etc/init.d/kdump
> restart" SUBSYSTEM=="memory", ACTION=="offline",
> PROGRAM="/etc/init.d/kdump restart"
> 
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>


Acked-by: WANG Cong <xiyou.wangcong@gmail.com>

Thanks.

Re: kdump: No udev events for memory hotplug?

[Dave Hansen]

On Thu, 2011-10-27 at 09:30 +0200, Heiko Carstens wrote:
> > Are any events generated for memory add?
> 
> Looks like uevents are only genereted when memory gets registered and
> unregistered, but not when when it gets set online or offline.
> To achieve that you would need to add similar code to
> store_mem_state()/memory_block_change_state() in drivers/base/memory.c
> like we have it already in store_online() in drivers/base/cpu.c 

Instead of doing it at that low a level, perhaps we should be generating
events when 'totalram_pages' changes.  That way, we'll also trigger
events when things like ballooning happen.

-- Dave

Re: kdump: No udev events for memory hotplug?

[Américo Wang]

On Thu, Oct 27, 2011 at 8:45 PM, Dave Hansen <haveblue@us.ibm.com> wrote:
> On Thu, 2011-10-27 at 09:30 +0200, Heiko Carstens wrote:
>> > Are any events generated for memory add?
>>
>> Looks like uevents are only genereted when memory gets registered and
>> unregistered, but not when when it gets set online or offline.
>> To achieve that you would need to add similar code to
>> store_mem_state()/memory_block_change_state() in drivers/base/memory.c
>> like we have it already in store_online() in drivers/base/cpu.c
>
> Instead of doing it at that low a level, perhaps we should be generating
> events when 'totalram_pages' changes.  That way, we'll also trigger
> events when things like ballooning happen.
>

I think we should separate the memory hot-plug issues from ballooning issues,
so probably we need another uevent for this...

Re: kdump: No udev events for memory hotplug?

[Américo Wang]

On Thu, Oct 27, 2011 at 12:08 AM, Michael Holzheu
<holzheu@linux.vnet.ibm.com> wrote:
> Hello Vivek and Eric,
>

Hello,

> I noticed that on my system kernel 3.1 *no* udev events for memory
> hotplug are generated. Same on my RHEL6.1.
>
> # udevadm monitor
> # echo offline > /sys/devices/system/memory/memory4/state
>
> -> No event
>
> But we need the udev events in order to do a kdump reload for setting up
> the ELF loads correctly.
>
> In my /etc/udev/rules.d/98-kexec.rules there are rules for memory
> hotplug:
>
> SUBSYSTEM=="memory", ACTION=="add", PROGRAM="/etc/init.d/kdump restart"
> SUBSYSTEM=="memory", ACTION=="remove", PROGRAM="/etc/init.d/kdump
> restart"
>
> Perhaps/probably I am missing something?

As Heiko pointed out, probably the memory-hotplug subsystem doesn't implement
uevent, this is why you can't get uevent when you hot-remove the memory.

For kdump, the memory is reserved by "crashkernel=", it is determined during
boot (but you can shrink it after boot), so it is not aware of memory hotplug.
IOW, if you hot-remove the memory where the crash kernel stays, the
'kdump restart'
will fail. But yes, restarting kdump can rebuild the memmap and ELF headers.

Thanks.