From: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
To: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: <wency@cn.fujitsu.com>, <linux-mm@kvack.org>,
<linux-kernel@vger.kernel.org>, <rientjes@google.com>,
<liuj97@gmail.com>, <len.brown@intel.com>,
<benh@kernel.crashing.org>, <paulus@samba.org>,
<minchan.kim@gmail.com>, <akpm@linux-foundation.org>
Subject: Re: [PATCH 2/4] memory-hotplug: add node_device_release
Date: Fri, 5 Oct 2012 10:00:43 +0900 [thread overview]
Message-ID: <506E313B.5010303@jp.fujitsu.com> (raw)
In-Reply-To: <CAHGf_=qZVe_KfThZa5yEm+4w3MMREs1xqya5HmKWsWjyTcjkzA@mail.gmail.com>
Hi Kosaki-san,
2012/10/02 3:12, KOSAKI Motohiro wrote:
> On Mon, Oct 1, 2012 at 2:54 AM, Yasuaki Ishimatsu
> <isimatu.yasuaki@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 (ffffffff82c15718): tried to init an initialized object, something is seriously wrong.
[ 374.153169] Pid: 4, comm: kworker/0:0 Tainted: G W 3.6.0 #5
[ 374.230279] Call Trace:
[ 374.259647] [<ffffffff8133cf39>] kobject_init+0x89/0xa0
[ 374.323286] [<ffffffff8143632c>] device_initialize+0x2c/0xc0
[ 374.392086] [<ffffffff814376a6>] device_register+0x16/0x30
[ 374.458856] [<ffffffff81449b15>] register_node+0x25/0xe0
[ 374.523434] [<ffffffff8144a057>] register_one_node+0x67/0x140
[ 374.593306] [<ffffffff81652e40>] add_memory+0x100/0x1f0
[ 374.656961] [<ffffffffa00a31c6>] acpi_memory_enable_device+0x92/0xdf [acpi_memhotplug]
[ 374.752811] [<ffffffffa00a3753>] acpi_memory_device_add+0x10d/0x116 [acpi_memhotplug]
[ 374.847622] [<ffffffff813a4376>] acpi_device_probe+0x50/0x18a
[ 374.917504] [<ffffffff8124b053>] ? sysfs_create_link+0x13/0x20
[ 374.988426] [<ffffffff81439d7c>] really_probe+0x6c/0x320
[ 375.053061] [<ffffffff8143a077>] driver_probe_device+0x47/0xa0
[ 375.123922] [<ffffffff8143a180>] ? __driver_attach+0xb0/0xb0
[ 375.192709] [<ffffffff8143a180>] ? __driver_attach+0xb0/0xb0
[ 375.261494] [<ffffffff8143a1d3>] __device_attach+0x53/0x60
[ 375.328206] [<ffffffff81437dac>] bus_for_each_drv+0x6c/0xa0
[ 375.395950] [<ffffffff81439cf8>] device_attach+0xa8/0xc0
[ 375.460578] [<ffffffff814389d0>] bus_probe_device+0xb0/0xe0
[ 375.528318] [<ffffffff81437421>] device_add+0x301/0x570
[ 375.591883] [<ffffffff814376ae>] device_register+0x1e/0x30
[ 375.658568] [<ffffffff813a56bf>] acpi_device_register+0x1af/0x2bf
[ 375.732590] [<ffffffff813a59ae>] acpi_add_single_object+0x1df/0x2b9
[ 375.808640] [<ffffffff813ce320>] ? acpi_ut_release_mutex+0xac/0xb5
[ 375.883646] [<ffffffff813a5b93>] acpi_bus_check_add+0x10b/0x166
[ 375.955529] [<ffffffff810db4ad>] ? trace_hardirqs_on+0xd/0x10
[ 376.025327] [<ffffffff8109fa4f>] ? up+0x2f/0x50
[ 376.080639] [<ffffffff813a0cc1>] ? acpi_os_signal_semaphore+0x6b/0x74
[ 376.158792] [<ffffffff813c3f1d>] acpi_ns_walk_namespace+0xbe/0x17d
[ 376.233854] [<ffffffff813a5a88>] ? acpi_add_single_object+0x2b9/0x2b9
[ 376.312012] [<ffffffff813a5a88>] ? acpi_add_single_object+0x2b9/0x2b9
[ 376.390162] [<ffffffff813c43b3>] acpi_walk_namespace+0x8a/0xc4
[ 376.461051] [<ffffffff813a5c49>] acpi_bus_scan+0x5b/0x7c
[ 376.525707] [<ffffffff813a5cd6>] acpi_bus_add+0x2a/0x2c
[ 376.589344] [<ffffffff813d5dc6>] container_notify_cb+0x103/0x18d
[ 376.662309] [<ffffffff813b3946>] acpi_ev_notify_dispatch+0x41/0x5f
[ 376.737386] [<ffffffff813a0f47>] acpi_os_execute_deferred+0x27/0x34
[ 376.813507] [<ffffffff81090279>] process_one_work+0x219/0x680
[ 376.883357] [<ffffffff81090218>] ? process_one_work+0x1b8/0x680
[ 376.955312] [<ffffffff813a0f20>] ? acpi_os_wait_events_complete+0x23/0x23
[ 377.037615] [<ffffffff8109212e>] worker_thread+0x12e/0x320
[ 377.104365] [<ffffffff81092000>] ? manage_workers+0x190/0x190
[ 377.174274] [<ffffffff81098106>] kthread+0xc6/0xd0
[ 377.232697] [<ffffffff81678b04>] kernel_thread_helper+0x4/0x10
[ 377.303588] [<ffffffff8166e570>] ? retint_restore_args+0x13/0x13
[ 377.376550] [<ffffffff81098040>] ? __init_kthread_worker+0x70/0x70
[ 377.451591] [<ffffffff81678b00>] ? gs_change+0x13/0x13
[ 377.514247] ------------[ cut here ]------------
[ 377.569481] WARNING: at lib/kobject.c:166 kobject_add_internal+0x1b3/0x260()
[ 377.653796] Hardware name: PRIMEQUEST 1800E
[ 377.703865] kobject: (ffffffff82c15718): attempted to be registered with empty name!
[ 377.796584] Modules linked in: bridge stp llc sunrpc ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables binfmt_misc vfat fat dm_mirror dm_region_hash dm_log dm_mod uinput iTCO_wdt iTCO_vendor_support coretemp kvm_intel kvm crc32c_intel microcode pcspkr lpc_ich mfd_core i2c_i801 i2c_core ioatdma i7core_edac edac_core sg acpi_memhotplug e1000e igb dca sd_mod crc_t10dif lpfc scsi_transport_fc scsi_tgt mptsas mptscsih mptbase scsi_transport_sas scsi_mod
[ 378.400511] Pid: 4, comm: kworker/0:0 Tainted: G W 3.6.0 #5
[ 378.477614] Call Trace:
[ 378.506916] [<ffffffff8106c10f>] warn_slowpath_common+0x7f/0xc0
[ 378.578843] [<ffffffff8106c206>] warn_slowpath_fmt+0x46/0x50
[ 378.647658] [<ffffffff810db09d>] ? mark_held_locks+0x8d/0x140
[ 378.717504] [<ffffffff8133cda3>] kobject_add_internal+0x1b3/0x260
[ 378.791507] [<ffffffff8133cff8>] kobject_add_varg+0x38/0x60
[ 378.859187] [<ffffffff8133d0d4>] kobject_add+0x44/0x70
[ 378.921769] [<ffffffff81098d76>] ? __init_waitqueue_head+0x46/0x60
[ 378.996747] [<ffffffff814371f4>] device_add+0xd4/0x570
[ 379.059342] [<ffffffff814376ae>] device_register+0x1e/0x30
[ 379.126042] [<ffffffff81449b15>] register_node+0x25/0xe0
[ 379.190652] [<ffffffff8144a057>] register_one_node+0x67/0x140
[ 379.260532] [<ffffffff81652e40>] add_memory+0x100/0x1f0
[ 379.324175] [<ffffffffa00a31c6>] acpi_memory_enable_device+0x92/0xdf [acpi_memhotplug]
[ 379.419903] [<ffffffffa00a3753>] acpi_memory_device_add+0x10d/0x116 [acpi_memhotplug]
[ 379.514628] [<ffffffff813a4376>] acpi_device_probe+0x50/0x18a
[ 379.584489] [<ffffffff8124b053>] ? sysfs_create_link+0x13/0x20
[ 379.655351] [<ffffffff81439d7c>] really_probe+0x6c/0x320
[ 379.720047] [<ffffffff8143a077>] driver_probe_device+0x47/0xa0
[ 379.790863] [<ffffffff8143a180>] ? __driver_attach+0xb0/0xb0
[ 379.859667] [<ffffffff8143a180>] ? __driver_attach+0xb0/0xb0
[ 379.928526] [<ffffffff8143a1d3>] __device_attach+0x53/0x60
[ 379.995218] [<ffffffff81437dac>] bus_for_each_drv+0x6c/0xa0
[ 380.063010] [<ffffffff81439cf8>] device_attach+0xa8/0xc0
[ 380.127664] [<ffffffff814389d0>] bus_probe_device+0xb0/0xe0
[ 380.195449] [<ffffffff81437421>] device_add+0x301/0x570
[ 380.259081] [<ffffffff814376ae>] device_register+0x1e/0x30
[ 380.325840] [<ffffffff813a56bf>] acpi_device_register+0x1af/0x2bf
[ 380.399852] [<ffffffff813a59ae>] acpi_add_single_object+0x1df/0x2b9
[ 380.475874] [<ffffffff813ce320>] ? acpi_ut_release_mutex+0xac/0xb5
[ 380.550884] [<ffffffff813a5b93>] acpi_bus_check_add+0x10b/0x166
[ 380.622761] [<ffffffff810db4ad>] ? trace_hardirqs_on+0xd/0x10
[ 380.692529] [<ffffffff8109fa4f>] ? up+0x2f/0x50
[ 380.747792] [<ffffffff813a0cc1>] ? acpi_os_signal_semaphore+0x6b/0x74
[ 380.825974] [<ffffffff813c3f1d>] acpi_ns_walk_namespace+0xbe/0x17d
[ 380.901027] [<ffffffff813a5a88>] ? acpi_add_single_object+0x2b9/0x2b9
[ 380.979208] [<ffffffff813a5a88>] ? acpi_add_single_object+0x2b9/0x2b9
[ 381.057328] [<ffffffff813c43b3>] acpi_walk_namespace+0x8a/0xc4
[ 381.128253] [<ffffffff813a5c49>] acpi_bus_scan+0x5b/0x7c
[ 381.192922] [<ffffffff813a5cd6>] acpi_bus_add+0x2a/0x2c
[ 381.256577] [<ffffffff813d5dc6>] container_notify_cb+0x103/0x18d
[ 381.329551] [<ffffffff813b3946>] acpi_ev_notify_dispatch+0x41/0x5f
[ 381.404612] [<ffffffff813a0f47>] acpi_os_execute_deferred+0x27/0x34
[ 381.480709] [<ffffffff81090279>] process_one_work+0x219/0x680
[ 381.550600] [<ffffffff81090218>] ? process_one_work+0x1b8/0x680
[ 381.622535] [<ffffffff813a0f20>] ? acpi_os_wait_events_complete+0x23/0x23
[ 381.704882] [<ffffffff8109212e>] worker_thread+0x12e/0x320
[ 381.771655] [<ffffffff81092000>] ? manage_workers+0x190/0x190
[ 381.841554] [<ffffffff81098106>] kthread+0xc6/0xd0
[ 381.899995] [<ffffffff81678b04>] kernel_thread_helper+0x4/0x10
[ 381.970815] [<ffffffff8166e570>] ? retint_restore_args+0x13/0x13
[ 382.043793] [<ffffffff81098040>] ? __init_kthread_worker+0x70/0x70
[ 382.118880] [<ffffffff81678b00>] ? gs_change+0x13/0x13
[ 382.181437] ---[ end trace a3ee526778d7b765 ]---
Thanks,
Yasuaki Ishimatsu
> --
> 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/
>
next prev parent reply other threads:[~2012-10-05 1:01 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-27 5:45 [PATCH 0/4] bugfix for memory hotplug wency
2012-09-27 5:45 ` [PATCH 1/4] memory-hotplug: add memory_block_release wency
2012-09-27 20:11 ` KOSAKI Motohiro
[not found] ` <CAEkdkmVW5wwG4_cy0yHFNVmk2bzAqzo2adRsMn1yHOW9Ex98_g@mail.gmail.com>
2012-09-28 0:24 ` Yasuaki Ishimatsu
2012-09-28 1:35 ` KOSAKI Motohiro
2012-09-28 3:45 ` Yasuaki Ishimatsu
2012-09-28 6:04 ` Ni zhan Chen
2012-09-28 6:11 ` Yasuaki Ishimatsu
2012-09-28 6:14 ` Ni zhan Chen
2012-09-28 22:30 ` KOSAKI Motohiro
2012-09-27 5:45 ` [PATCH 2/4] memory-hotplug: add node_device_release wency
2012-09-27 10:38 ` Ni zhan Chen
2012-09-27 20:13 ` KOSAKI Motohiro
2012-09-28 0:07 ` Yasuaki Ishimatsu
2012-09-28 1:13 ` KOSAKI Motohiro
2012-09-28 1:30 ` Yasuaki Ishimatsu
2012-09-28 1:37 ` KOSAKI Motohiro
2012-09-28 9:55 ` Yasuaki Ishimatsu
2012-09-28 22:19 ` KOSAKI Motohiro
2012-10-01 6:54 ` Yasuaki Ishimatsu
2012-10-01 18:12 ` KOSAKI Motohiro
2012-10-05 1:00 ` Yasuaki Ishimatsu [this message]
2012-10-05 18:39 ` KOSAKI Motohiro
2012-09-27 5:45 ` [PATCH 3/4] memory-hotplug: clear hwpoisoned flag when onlining pages wency
2012-09-27 12:27 ` Ni zhan Chen
2012-09-27 20:17 ` KOSAKI Motohiro
2012-09-28 1:53 ` Wen Congyang
2012-09-27 5:45 ` [PATCH 4/4] memory-hotplug: auto offline page_cgroup when onlining memory block failed wency
2012-09-27 12:44 ` Ni zhan Chen
2012-09-27 20:19 ` KOSAKI Motohiro
2012-09-27 21:18 ` [PATCH 0/4] bugfix for memory hotplug Andrew Morton
2012-09-29 2:31 ` Ni zhan Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=506E313B.5010303@jp.fujitsu.com \
--to=isimatu.yasuaki@jp.fujitsu.com \
--cc=akpm@linux-foundation.org \
--cc=benh@kernel.crashing.org \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=liuj97@gmail.com \
--cc=minchan.kim@gmail.com \
--cc=paulus@samba.org \
--cc=rientjes@google.com \
--cc=wency@cn.fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome