Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754461Ab2JEBB1 (ORCPT ); Thu, 4 Oct 2012 21:01:27 -0400 Received: from fgwmail6.fujitsu.co.jp ([192.51.44.36]:55611 "EHLO fgwmail6.fujitsu.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752498Ab2JEBBZ (ORCPT ); Thu, 4 Oct 2012 21:01:25 -0400 X-SecurityPolicyCheck: OK by SHieldMailChecker v1.7.4 Message-ID: <506E313B.5010303@jp.fujitsu.com> Date: Fri, 5 Oct 2012 10:00:43 +0900 From: Yasuaki Ishimatsu User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/20120907 Thunderbird/15.0.1 MIME-Version: 1.0 To: KOSAKI Motohiro CC: , , , , , , , , , Subject: Re: [PATCH 2/4] memory-hotplug: add node_device_release References: <1348724705-23779-1-git-send-email-wency@cn.fujitsu.com> <1348724705-23779-3-git-send-email-wency@cn.fujitsu.com> <5064EA5A.3080905@jp.fujitsu.com> <5064FDCA.1020504@jp.fujitsu.com> <5065740A.2000502@jp.fujitsu.com> <50693E30.3010006@jp.fujitsu.com> In-Reply-To: Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 11011 Lines: 213 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 (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] [] 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] 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] [] warn_slowpath_common+0x7f/0xc0 [ 378.578843] [] warn_slowpath_fmt+0x46/0x50 [ 378.647658] [] ? mark_held_locks+0x8d/0x140 [ 378.717504] [] kobject_add_internal+0x1b3/0x260 [ 378.791507] [] kobject_add_varg+0x38/0x60 [ 378.859187] [] kobject_add+0x44/0x70 [ 378.921769] [] ? __init_waitqueue_head+0x46/0x60 [ 378.996747] [] device_add+0xd4/0x570 [ 379.059342] [] device_register+0x1e/0x30 [ 379.126042] [] register_node+0x25/0xe0 [ 379.190652] [] register_one_node+0x67/0x140 [ 379.260532] [] add_memory+0x100/0x1f0 [ 379.324175] [] acpi_memory_enable_device+0x92/0xdf [acpi_memhotplug] [ 379.419903] [] acpi_memory_device_add+0x10d/0x116 [acpi_memhotplug] [ 379.514628] [] acpi_device_probe+0x50/0x18a [ 379.584489] [] ? sysfs_create_link+0x13/0x20 [ 379.655351] [] really_probe+0x6c/0x320 [ 379.720047] [] driver_probe_device+0x47/0xa0 [ 379.790863] [] ? __driver_attach+0xb0/0xb0 [ 379.859667] [] ? __driver_attach+0xb0/0xb0 [ 379.928526] [] __device_attach+0x53/0x60 [ 379.995218] [] bus_for_each_drv+0x6c/0xa0 [ 380.063010] [] device_attach+0xa8/0xc0 [ 380.127664] [] bus_probe_device+0xb0/0xe0 [ 380.195449] [] device_add+0x301/0x570 [ 380.259081] [] device_register+0x1e/0x30 [ 380.325840] [] acpi_device_register+0x1af/0x2bf [ 380.399852] [] acpi_add_single_object+0x1df/0x2b9 [ 380.475874] [] ? acpi_ut_release_mutex+0xac/0xb5 [ 380.550884] [] acpi_bus_check_add+0x10b/0x166 [ 380.622761] [] ? trace_hardirqs_on+0xd/0x10 [ 380.692529] [] ? up+0x2f/0x50 [ 380.747792] [] ? acpi_os_signal_semaphore+0x6b/0x74 [ 380.825974] [] acpi_ns_walk_namespace+0xbe/0x17d [ 380.901027] [] ? acpi_add_single_object+0x2b9/0x2b9 [ 380.979208] [] ? acpi_add_single_object+0x2b9/0x2b9 [ 381.057328] [] acpi_walk_namespace+0x8a/0xc4 [ 381.128253] [] acpi_bus_scan+0x5b/0x7c [ 381.192922] [] acpi_bus_add+0x2a/0x2c [ 381.256577] [] container_notify_cb+0x103/0x18d [ 381.329551] [] acpi_ev_notify_dispatch+0x41/0x5f [ 381.404612] [] acpi_os_execute_deferred+0x27/0x34 [ 381.480709] [] process_one_work+0x219/0x680 [ 381.550600] [] ? process_one_work+0x1b8/0x680 [ 381.622535] [] ? acpi_os_wait_events_complete+0x23/0x23 [ 381.704882] [] worker_thread+0x12e/0x320 [ 381.771655] [] ? manage_workers+0x190/0x190 [ 381.841554] [] kthread+0xc6/0xd0 [ 381.899995] [] kernel_thread_helper+0x4/0x10 [ 381.970815] [] ? retint_restore_args+0x13/0x13 [ 382.043793] [] ? __init_kthread_worker+0x70/0x70 [ 382.118880] [] ? 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/ > -- 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/