Re: [PATCH 2/4] memory-hotplug: add node_device_release
> I have the reason to have to fill the node struct with 0 by memset. > The node is a part of node struct array (node_devices[]). > If we add empty release function for suppressing warning, > some data remains in the node struct after hot removing memory. > So if we re-hot adds the memory, the node struct is reused by > register_onde_node(). But the node struct has some data, because > it was not initialized with 0. As a result, more waning is shown > by the remained data at hot addinig memory as follows: Even though you call memset(0) at offline. It doesn't guarantee the memory keep 0 until online. E.g. physical memory exchange during offline, bit corruption by cosmic ray, etc. So, you should fill zero at online phase explicitly if need. The basic hotplug design is: you should forget everything at offline and you shouldn't assume any initialized data at online. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
I have the reason to have to fill the node struct with 0 by memset. The node is a part of node struct array (node_devices[]). If we add empty release function for suppressing warning, some data remains in the node struct after hot removing memory. So if we re-hot adds the memory, the node struct is reused by register_onde_node(). But the node struct has some data, because it was not initialized with 0. As a result, more waning is shown by the remained data at hot addinig memory as follows: Even though you call memset(0) at offline. It doesn't guarantee the memory keep 0 until online. E.g. physical memory exchange during offline, bit corruption by cosmic ray, etc. So, you should fill zero at online phase explicitly if need. The basic hotplug design is: you should forget everything at offline and you shouldn't assume any initialized data at online. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Hi Kosaki-san, 2012/10/02 3:12, KOSAKI Motohiro wrote: On Mon, Oct 1, 2012 at 2:54 AM, Yasuaki Ishimatsu wrote: Hi Kosaki-san, 2012/09/29 7:19, KOSAKI Motohiro wrote: I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. If we implement a function like cpu_device_release(), the warning disappears. But the comment says in the function "Never copy this way...". So I think it is illegal way. What does "illegal" mean? The "illegal" means the code should not be mimicked. You still haven't explain any benefit of your code. If there is zero benefit, just kill it. I believe everybody think so. Again, Which benefit do you have? The patch has a benefit to delets a warning message. Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. What we want to release is a part of array, not a pointer. Therefore, there is only this way instead of kfree(). Why? Before your patch, we don't have memset() and did work it. If we does not apply the patch, a warning message is shown. So I think it did not work well. I can't understand what mean "only way". For deleting a warning message, I created a node_device_release(). In the manner of releasing kobject, the function frees a object related to the kobject. So most functions calls kfree() for releasing it. In node_device_release(), we need to free a node struct. If the node struct is pointer, I can free it by kfree. But the node struct is a part of node_devices[] array. I cannot free it. So I filled the node struct with 0. But you think it is not good. Do you have a good solution? Do nothing. just add empty release function and kill a warning. Obviously do nothing can't make any performance drop nor any side effect. meaningless memset() is just silly from point of cache pollution view. I have the reason to have to fill the node struct with 0 by memset. The node is a part of node struct array (node_devices[]). If we add empty release function for suppressing warning, some data remains in the node struct after hot removing memory. So if we re-hot adds the memory, the node struct is reused by register_onde_node(). But the node struct has some data, because it was not initialized with 0. As a result, more waning is shown by the remained data at hot addinig memory as follows: [ 374.037710] kobject (82c15718): tried to init an initialized object, something is seriously wrong. [ 374.153169] Pid: 4, comm: kworker/0:0 Tainted: GW3.6.0 #5 [ 374.230279] Call Trace: [ 374.259647] [] kobject_init+0x89/0xa0 [ 374.323286] [] device_initialize+0x2c/0xc0 [ 374.392086] [] device_register+0x16/0x30 [ 374.458856] [] register_node+0x25/0xe0 [ 374.523434] [] register_one_node+0x67/0x140 [ 374.593306] [] add_memory+0x100/0x1f0 [ 374.656961] [] acpi_memory_enable_device+0x92/0xdf [acpi_memhotplug] [ 374.752811] [] acpi_memory_device_add+0x10d/0x116 [acpi_memhotplug] [ 374.847622] [] acpi_device_probe+0x50/0x18a [ 374.917504] [] ? sysfs_create_link+0x13/0x20 [ 374.988426] [] really_probe+0x6c/0x320 [ 375.053061] [] driver_probe_device+0x47/0xa0 [ 375.123922] [] ? __driver_attach+0xb0/0xb0 [ 375.192709] [] ? __driver_attach+0xb0/0xb0 [ 375.261494] [] __device_attach+0x53/0x60 [ 375.328206] [] bus_for_each_drv+0x6c/0xa0 [ 375.395950] [] device_attach+0xa8/0xc0 [ 375.460578] [] bus_probe_device+0xb0/0xe0 [ 375.528318] [] device_add+0x301/0x570 [ 375.591883] [] device_register+0x1e/0x30 [ 375.658568] [] acpi_device_register+0x1af/0x2bf [ 375.732590] [] acpi_add_single_object+0x1df/0x2b9 [ 375.808640] [] ? acpi_ut_release_mutex+0xac/0xb5 [ 375.883646] [] acpi_bus_check_add+0x10b/0x166 [ 375.955529] [] ? trace_hardirqs_on+0xd/0x10 [ 376.025327] [] ? up+0x2f/0x50 [ 376.080639] [] ? acpi_os_signal_semaphore+0x6b/0x74 [ 376.158792] [] acpi_ns_walk_namespace+0xbe/0x17d [ 376.233854] [] ? acpi_add_single_object+0x2b9/0x2b9 [ 376.312012] [] ? acpi_add_single_object+0x2b9/0x2b9 [ 376.390162] [] acpi_walk_namespace+0x8a/0xc4 [ 376.461051] [] acpi_bus_scan+0x5b/0x7c [ 376.525707] [] acpi_bus_add+0x2a/0x2c [ 376.589344] [] container_notify_cb+0x103/0x18d [ 376.662309] [] acpi_ev_notify_dispatch+0x41/0x5f [ 376.737386] [] acpi_os_execute_deferred+0x27/0x34 [ 376.813507] [] process_one_work+0x219/0x680 [ 376.883357] [] ? process_one_work+0x1b8/0x680 [ 376.955312] [] ? acpi_os_wait_events_complete+0x23/0x23 [ 377.037615] [] worker_thread+0x12e/0x320 [ 377.104365] [] ? manage_workers+0x190/0x190 [ 377.174274] [] kthread+0xc6/0xd0 [ 377.232697] [] kernel_thread_helper+0x4/0x10 [ 377.303588] [] ? retint_restore_args+0x13/0x13 [ 377.376550] [] ? __init_kthread_worker+0x70/0x70 [ 377.451591] [] ? gs_change+0x13/0x13 [ 377.514247] [ cut here ] [ 377.569481]
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Hi Kosaki-san, 2012/10/02 3:12, KOSAKI Motohiro wrote: On Mon, Oct 1, 2012 at 2:54 AM, Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com wrote: Hi Kosaki-san, 2012/09/29 7:19, KOSAKI Motohiro wrote: I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. If we implement a function like cpu_device_release(), the warning disappears. But the comment says in the function Never copy this way So I think it is illegal way. What does illegal mean? The illegal means the code should not be mimicked. You still haven't explain any benefit of your code. If there is zero benefit, just kill it. I believe everybody think so. Again, Which benefit do you have? The patch has a benefit to delets a warning message. Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. What we want to release is a part of array, not a pointer. Therefore, there is only this way instead of kfree(). Why? Before your patch, we don't have memset() and did work it. If we does not apply the patch, a warning message is shown. So I think it did not work well. I can't understand what mean only way. For deleting a warning message, I created a node_device_release(). In the manner of releasing kobject, the function frees a object related to the kobject. So most functions calls kfree() for releasing it. In node_device_release(), we need to free a node struct. If the node struct is pointer, I can free it by kfree. But the node struct is a part of node_devices[] array. I cannot free it. So I filled the node struct with 0. But you think it is not good. Do you have a good solution? Do nothing. just add empty release function and kill a warning. Obviously do nothing can't make any performance drop nor any side effect. meaningless memset() is just silly from point of cache pollution view. I have the reason to have to fill the node struct with 0 by memset. The node is a part of node struct array (node_devices[]). If we add empty release function for suppressing warning, some data remains in the node struct after hot removing memory. So if we re-hot adds the memory, the node struct is reused by register_onde_node(). But the node struct has some data, because it was not initialized with 0. As a result, more waning is shown by the remained data at hot addinig memory as follows: [ 374.037710] kobject (82c15718): tried to init an initialized object, something is seriously wrong. [ 374.153169] Pid: 4, comm: kworker/0:0 Tainted: GW3.6.0 #5 [ 374.230279] Call Trace: [ 374.259647] [8133cf39] kobject_init+0x89/0xa0 [ 374.323286] [8143632c] device_initialize+0x2c/0xc0 [ 374.392086] [814376a6] device_register+0x16/0x30 [ 374.458856] [81449b15] register_node+0x25/0xe0 [ 374.523434] [8144a057] register_one_node+0x67/0x140 [ 374.593306] [81652e40] add_memory+0x100/0x1f0 [ 374.656961] [a00a31c6] acpi_memory_enable_device+0x92/0xdf [acpi_memhotplug] [ 374.752811] [a00a3753] acpi_memory_device_add+0x10d/0x116 [acpi_memhotplug] [ 374.847622] [813a4376] acpi_device_probe+0x50/0x18a [ 374.917504] [8124b053] ? sysfs_create_link+0x13/0x20 [ 374.988426] [81439d7c] really_probe+0x6c/0x320 [ 375.053061] [8143a077] driver_probe_device+0x47/0xa0 [ 375.123922] [8143a180] ? __driver_attach+0xb0/0xb0 [ 375.192709] [8143a180] ? __driver_attach+0xb0/0xb0 [ 375.261494] [8143a1d3] __device_attach+0x53/0x60 [ 375.328206] [81437dac] bus_for_each_drv+0x6c/0xa0 [ 375.395950] [81439cf8] device_attach+0xa8/0xc0 [ 375.460578] [814389d0] bus_probe_device+0xb0/0xe0 [ 375.528318] [81437421] device_add+0x301/0x570 [ 375.591883] [814376ae] device_register+0x1e/0x30 [ 375.658568] [813a56bf] acpi_device_register+0x1af/0x2bf [ 375.732590] [813a59ae] acpi_add_single_object+0x1df/0x2b9 [ 375.808640] [813ce320] ? acpi_ut_release_mutex+0xac/0xb5 [ 375.883646] [813a5b93] acpi_bus_check_add+0x10b/0x166 [ 375.955529] [810db4ad] ? trace_hardirqs_on+0xd/0x10 [ 376.025327] [8109fa4f] ? up+0x2f/0x50 [ 376.080639] [813a0cc1] ? acpi_os_signal_semaphore+0x6b/0x74 [ 376.158792] [813c3f1d] acpi_ns_walk_namespace+0xbe/0x17d [ 376.233854] [813a5a88] ? acpi_add_single_object+0x2b9/0x2b9 [ 376.312012] [813a5a88] ? acpi_add_single_object+0x2b9/0x2b9 [ 376.390162] [813c43b3] acpi_walk_namespace+0x8a/0xc4 [ 376.461051] [813a5c49] acpi_bus_scan+0x5b/0x7c [ 376.525707] [813a5cd6] acpi_bus_add+0x2a/0x2c [ 376.589344] [813d5dc6] container_notify_cb+0x103/0x18d [ 376.662309] [813b3946] acpi_ev_notify_dispatch+0x41/0x5f [ 376.737386]
Re: [PATCH 2/4] memory-hotplug: add node_device_release
On Mon, Oct 1, 2012 at 2:54 AM, Yasuaki Ishimatsu wrote: > Hi Kosaki-san, > > > 2012/09/29 7:19, KOSAKI Motohiro wrote: > > I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. >>> >>> >>> If we implement a function like cpu_device_release(), the warning >>> disappears. But the comment says in the function "Never copy this >>> way...". >>> So I think it is illegal way. >> >> >> What does "illegal" mean? > > > The "illegal" means the code should not be mimicked. > > >> You still haven't explain any benefit of your code. If there is zero >> benefit, just kill it. >> I believe everybody think so. >> >> Again, Which benefit do you have? > > > The patch has a benefit to delets a warning message. > > >> >> Why do we need this node_device_release() implementation? > > > I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. >>> >>> >>> What we want to release is a part of array, not a pointer. >>> Therefore, there is only this way instead of kfree(). >> >> >> Why? Before your patch, we don't have memset() and did work it. > > > If we does not apply the patch, a warning message is shown. > So I think it did not work well. > > >> I can't understand what mean "only way". > > > For deleting a warning message, I created a node_device_release(). > In the manner of releasing kobject, the function frees a object related > to the kobject. So most functions calls kfree() for releasing it. > In node_device_release(), we need to free a node struct. If the node > struct is pointer, I can free it by kfree. But the node struct is a part > of node_devices[] array. I cannot free it. So I filled the node struct > with 0. > > But you think it is not good. Do you have a good solution? Do nothing. just add empty release function and kill a warning. Obviously do nothing can't make any performance drop nor any side effect. meaningless memset() is just silly from point of cache pollution view. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Hi Kosaki-san, 2012/09/29 7:19, KOSAKI Motohiro wrote: I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. If we implement a function like cpu_device_release(), the warning disappears. But the comment says in the function "Never copy this way...". So I think it is illegal way. What does "illegal" mean? The "illegal" means the code should not be mimicked. You still haven't explain any benefit of your code. If there is zero benefit, just kill it. I believe everybody think so. Again, Which benefit do you have? The patch has a benefit to delets a warning message. Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. What we want to release is a part of array, not a pointer. Therefore, there is only this way instead of kfree(). Why? Before your patch, we don't have memset() and did work it. If we does not apply the patch, a warning message is shown. So I think it did not work well. I can't understand what mean "only way". For deleting a warning message, I created a node_device_release(). In the manner of releasing kobject, the function frees a object related to the kobject. So most functions calls kfree() for releasing it. In node_device_release(), we need to free a node struct. If the node struct is pointer, I can free it by kfree. But the node struct is a part of node_devices[] array. I cannot free it. So I filled the node struct with 0. But you think it is not good. Do you have a good solution? Thanks, Yasuaki Ishimatsu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Hi Kosaki-san, 2012/09/29 7:19, KOSAKI Motohiro wrote: I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. If we implement a function like cpu_device_release(), the warning disappears. But the comment says in the function Never copy this way So I think it is illegal way. What does illegal mean? The illegal means the code should not be mimicked. You still haven't explain any benefit of your code. If there is zero benefit, just kill it. I believe everybody think so. Again, Which benefit do you have? The patch has a benefit to delets a warning message. Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. What we want to release is a part of array, not a pointer. Therefore, there is only this way instead of kfree(). Why? Before your patch, we don't have memset() and did work it. If we does not apply the patch, a warning message is shown. So I think it did not work well. I can't understand what mean only way. For deleting a warning message, I created a node_device_release(). In the manner of releasing kobject, the function frees a object related to the kobject. So most functions calls kfree() for releasing it. In node_device_release(), we need to free a node struct. If the node struct is pointer, I can free it by kfree. But the node struct is a part of node_devices[] array. I cannot free it. So I filled the node struct with 0. But you think it is not good. Do you have a good solution? Thanks, Yasuaki Ishimatsu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
On Mon, Oct 1, 2012 at 2:54 AM, Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com wrote: Hi Kosaki-san, 2012/09/29 7:19, KOSAKI Motohiro wrote: I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. If we implement a function like cpu_device_release(), the warning disappears. But the comment says in the function Never copy this way So I think it is illegal way. What does illegal mean? The illegal means the code should not be mimicked. You still haven't explain any benefit of your code. If there is zero benefit, just kill it. I believe everybody think so. Again, Which benefit do you have? The patch has a benefit to delets a warning message. Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. What we want to release is a part of array, not a pointer. Therefore, there is only this way instead of kfree(). Why? Before your patch, we don't have memset() and did work it. If we does not apply the patch, a warning message is shown. So I think it did not work well. I can't understand what mean only way. For deleting a warning message, I created a node_device_release(). In the manner of releasing kobject, the function frees a object related to the kobject. So most functions calls kfree() for releasing it. In node_device_release(), we need to free a node struct. If the node struct is pointer, I can free it by kfree. But the node struct is a part of node_devices[] array. I cannot free it. So I filled the node struct with 0. But you think it is not good. Do you have a good solution? Do nothing. just add empty release function and kill a warning. Obviously do nothing can't make any performance drop nor any side effect. meaningless memset() is just silly from point of cache pollution view. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
>>> I don't understand it. How can we get rid of the warning? >> >> See cpu_device_release() for example. > > If we implement a function like cpu_device_release(), the warning > disappears. But the comment says in the function "Never copy this way...". > So I think it is illegal way. What does "illegal" mean? You still haven't explain any benefit of your code. If there is zero benefit, just kill it. I believe everybody think so. Again, Which benefit do you have? Why do we need this node_device_release() implementation? >>> >>> I think that this is a manner of releasing object related kobject. >> >> No. Usually we never call memset() from release callback. > > What we want to release is a part of array, not a pointer. > Therefore, there is only this way instead of kfree(). Why? Before your patch, we don't have memset() and did work it. I can't understand what mean "only way". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Hi Kosaki-san, 2012/09/28 10:37, KOSAKI Motohiro wrote: Moreover, your explanation is still insufficient. Even if node_device_release() is empty function, we can get rid of the warning. I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. If we implement a function like cpu_device_release(), the warning disappears. But the comment says in the function "Never copy this way...". So I think it is illegal way. Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. What we want to release is a part of array, not a pointer. Therefore, there is only this way instead of kfree(). Thanks, Yasuaki Ishimatsu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Hi Kosaki-san, 2012/09/28 10:37, KOSAKI Motohiro wrote: Moreover, your explanation is still insufficient. Even if node_device_release() is empty function, we can get rid of the warning. I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. If we implement a function like cpu_device_release(), the warning disappears. But the comment says in the function Never copy this way So I think it is illegal way. Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. What we want to release is a part of array, not a pointer. Therefore, there is only this way instead of kfree(). Thanks, Yasuaki Ishimatsu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. If we implement a function like cpu_device_release(), the warning disappears. But the comment says in the function Never copy this way So I think it is illegal way. What does illegal mean? You still haven't explain any benefit of your code. If there is zero benefit, just kill it. I believe everybody think so. Again, Which benefit do you have? Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. What we want to release is a part of array, not a pointer. Therefore, there is only this way instead of kfree(). Why? Before your patch, we don't have memset() and did work it. I can't understand what mean only way. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
>> Moreover, your explanation is still insufficient. Even if >> node_device_release() is empty function, we can get rid of the >> warning. > > > I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. >> Why do we need this node_device_release() implementation? > > I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Hi Kosaki-san, 2012/09/28 10:13, KOSAKI Motohiro wrote: On Thu, Sep 27, 2012 at 8:07 PM, Yasuaki Ishimatsu wrote: Hi Kosaki-san, 2012/09/28 5:13, KOSAKI Motohiro wrote: On Thu, Sep 27, 2012 at 1:45 AM, wrote: From: Yasuaki Ishimatsu When calling unregister_node(), the function shows following message at device_release(). This description doesn't have the "following message". Device 'node2' does not have a release() function, it is broken and must be fixed. This is the messages. The message is shown by kobject_cleanup(), when calling unregister_node(). If so, you should quote the message. and don't mix it with your subject. Moreover your patch title is too silly. "add node_device_release() function" is a way. you should describe the effect of the patch. e.g. suppress "Device 'nodeXX' does not have a release() function" warning. What you say is correct. We should update subject and changelog. Moreover, your explanation is still insufficient. Even if node_device_release() is empty function, we can get rid of the warning. I don't understand it. How can we get rid of the warning? Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. Thanks, Yasuaki Ishimatsu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
On Thu, Sep 27, 2012 at 8:07 PM, Yasuaki Ishimatsu wrote: > Hi Kosaki-san, > > > 2012/09/28 5:13, KOSAKI Motohiro wrote: >> >> On Thu, Sep 27, 2012 at 1:45 AM, wrote: >>> >>> From: Yasuaki Ishimatsu >>> >>> When calling unregister_node(), the function shows following message at >>> device_release(). >> >> >> This description doesn't have the "following message". >> >> > >>> Device 'node2' does not have a release() function, it is broken and must >>> be >>> fixed. > > > This is the messages. The message is shown by kobject_cleanup(), when > calling > unregister_node(). If so, you should quote the message. and don't mix it with your subject. Moreover your patch title is too silly. "add node_device_release() function" is a way. you should describe the effect of the patch. e.g. suppress "Device 'nodeXX' does not have a release() function" warning. Moreover, your explanation is still insufficient. Even if node_device_release() is empty function, we can get rid of the warning. Why do we need this node_device_release() implementation? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Hi Kosaki-san, 2012/09/28 5:13, KOSAKI Motohiro wrote: On Thu, Sep 27, 2012 at 1:45 AM, wrote: From: Yasuaki Ishimatsu When calling unregister_node(), the function shows following message at device_release(). This description doesn't have the "following message". Device 'node2' does not have a release() function, it is broken and must be fixed. This is the messages. The message is shown by kobject_cleanup(), when calling unregister_node(). Thanks, Yasuaki Ishimatsu So the patch implements node_device_release() -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
On Thu, Sep 27, 2012 at 1:45 AM, wrote: > From: Yasuaki Ishimatsu > > When calling unregister_node(), the function shows following message at > device_release(). This description doesn't have the "following message". > Device 'node2' does not have a release() function, it is broken and must be > fixed. > > So the patch implements node_device_release() > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
On 09/27/2012 01:45 PM, we...@cn.fujitsu.com wrote:
From: Yasuaki Ishimatsu
When calling unregister_node(), the function shows following message at
device_release().
Device 'node2' does not have a release() function, it is broken and must be
fixed.
So the patch implements node_device_release()
looks reasonable to me.
CC: David Rientjes
CC: Jiang Liu
CC: Len Brown
CC: Benjamin Herrenschmidt
CC: Paul Mackerras
Cc: Minchan Kim
CC: Andrew Morton
CC: KOSAKI Motohiro
Signed-off-by: Yasuaki Ishimatsu
Signed-off-by: Wen Congyang
---
drivers/base/node.c | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index af1a177..07523fb 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -252,6 +252,16 @@ static inline void hugetlb_register_node(struct node
*node) {}
static inline void hugetlb_unregister_node(struct node *node) {}
#endif
+static void node_device_release(struct device *dev)
+{
+ struct node *node_dev = to_node(dev);
+
+#if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_HUGETLBFS)
+ flush_work(_dev->node_work);
+#endif
+
+ memset(node_dev, 0, sizeof(struct node));
+}
/*
* register_node - Setup a sysfs device for a node.
@@ -265,6 +275,7 @@ int register_node(struct node *node, int num, struct node
*parent)
node->dev.id = num;
node->dev.bus = _subsys;
+ node->dev.release = node_device_release;
error = device_register(>dev);
if (!error){
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
On 09/27/2012 01:45 PM, we...@cn.fujitsu.com wrote:
From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
When calling unregister_node(), the function shows following message at
device_release().
Device 'node2' does not have a release() function, it is broken and must be
fixed.
So the patch implements node_device_release()
looks reasonable to me.
CC: David Rientjes rient...@google.com
CC: Jiang Liu liu...@gmail.com
CC: Len Brown len.br...@intel.com
CC: Benjamin Herrenschmidt b...@kernel.crashing.org
CC: Paul Mackerras pau...@samba.org
Cc: Minchan Kim minchan@gmail.com
CC: Andrew Morton a...@linux-foundation.org
CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
Signed-off-by: Wen Congyang we...@cn.fujitsu.com
---
drivers/base/node.c | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index af1a177..07523fb 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -252,6 +252,16 @@ static inline void hugetlb_register_node(struct node
*node) {}
static inline void hugetlb_unregister_node(struct node *node) {}
#endif
+static void node_device_release(struct device *dev)
+{
+ struct node *node_dev = to_node(dev);
+
+#if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) defined(CONFIG_HUGETLBFS)
+ flush_work(node_dev-node_work);
+#endif
+
+ memset(node_dev, 0, sizeof(struct node));
+}
/*
* register_node - Setup a sysfs device for a node.
@@ -265,6 +275,7 @@ int register_node(struct node *node, int num, struct node
*parent)
node-dev.id = num;
node-dev.bus = node_subsys;
+ node-dev.release = node_device_release;
error = device_register(node-dev);
if (!error){
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
On Thu, Sep 27, 2012 at 1:45 AM, we...@cn.fujitsu.com wrote: From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com When calling unregister_node(), the function shows following message at device_release(). This description doesn't have the following message. Device 'node2' does not have a release() function, it is broken and must be fixed. So the patch implements node_device_release() -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Hi Kosaki-san, 2012/09/28 5:13, KOSAKI Motohiro wrote: On Thu, Sep 27, 2012 at 1:45 AM, we...@cn.fujitsu.com wrote: From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com When calling unregister_node(), the function shows following message at device_release(). This description doesn't have the following message. Device 'node2' does not have a release() function, it is broken and must be fixed. This is the messages. The message is shown by kobject_cleanup(), when calling unregister_node(). Thanks, Yasuaki Ishimatsu So the patch implements node_device_release() -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
On Thu, Sep 27, 2012 at 8:07 PM, Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com wrote: Hi Kosaki-san, 2012/09/28 5:13, KOSAKI Motohiro wrote: On Thu, Sep 27, 2012 at 1:45 AM, we...@cn.fujitsu.com wrote: From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com When calling unregister_node(), the function shows following message at device_release(). This description doesn't have the following message. Device 'node2' does not have a release() function, it is broken and must be fixed. This is the messages. The message is shown by kobject_cleanup(), when calling unregister_node(). If so, you should quote the message. and don't mix it with your subject. Moreover your patch title is too silly. add node_device_release() function is a way. you should describe the effect of the patch. e.g. suppress Device 'nodeXX' does not have a release() function warning. Moreover, your explanation is still insufficient. Even if node_device_release() is empty function, we can get rid of the warning. Why do we need this node_device_release() implementation? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Hi Kosaki-san, 2012/09/28 10:13, KOSAKI Motohiro wrote: On Thu, Sep 27, 2012 at 8:07 PM, Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com wrote: Hi Kosaki-san, 2012/09/28 5:13, KOSAKI Motohiro wrote: On Thu, Sep 27, 2012 at 1:45 AM, we...@cn.fujitsu.com wrote: From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com When calling unregister_node(), the function shows following message at device_release(). This description doesn't have the following message. Device 'node2' does not have a release() function, it is broken and must be fixed. This is the messages. The message is shown by kobject_cleanup(), when calling unregister_node(). If so, you should quote the message. and don't mix it with your subject. Moreover your patch title is too silly. add node_device_release() function is a way. you should describe the effect of the patch. e.g. suppress Device 'nodeXX' does not have a release() function warning. What you say is correct. We should update subject and changelog. Moreover, your explanation is still insufficient. Even if node_device_release() is empty function, we can get rid of the warning. I don't understand it. How can we get rid of the warning? Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. Thanks, Yasuaki Ishimatsu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] memory-hotplug: add node_device_release
Moreover, your explanation is still insufficient. Even if node_device_release() is empty function, we can get rid of the warning. I don't understand it. How can we get rid of the warning? See cpu_device_release() for example. Why do we need this node_device_release() implementation? I think that this is a manner of releasing object related kobject. No. Usually we never call memset() from release callback. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
