From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753703Ab3KYLLB (ORCPT ); Mon, 25 Nov 2013 06:11:01 -0500 Received: from v094114.home.net.pl ([79.96.170.134]:59258 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1752817Ab3KYLK6 convert rfc822-to-8bit (ORCPT ); Mon, 25 Nov 2013 06:10:58 -0500 From: "Rafael J. Wysocki" To: Yinghai Lu Cc: Bjorn Helgaas , Greg Kroah-Hartman , Tejun Heo , Mika Westerberg , "linux-kernel@vger.kernel.org" , Linux PCI Subject: Re: [PATCH] PCI: Move device_del() from pci_stop_dev() to pci_destroy_dev() Date: Mon, 25 Nov 2013 12:23:44 +0100 Message-ID: <1890016.dnpTzZzgDl@vostro.rjw.lan> User-Agent: KMail/4.10.5 (Linux/3.12.0-rc6+; KDE/4.10.5; x86_64; ; ) In-Reply-To: References: <1384866598-19716-1-git-send-email-mika.westerberg@linux.intel.com>

MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sunday, November 24, 2013 08:58:12 PM Yinghai Lu wrote: > On Sun, Nov 24, 2013 at 8:54 PM, Yinghai Lu wrote: > > On Sat, Nov 23, 2013 at 4:17 PM, Rafael J. Wysocki wrote: > >> From: Rafael J. Wysocki > >> > >> After commit bcdde7e221a8 (sysfs: make __sysfs_remove_dir() recursive) > >> I'm seeing traces analogous to the one below in Thunderbolt testing: > >> > >> WARNING: CPU: 3 PID: 76 at /scratch/rafael/work/linux-pm/fs/sysfs/group.c:214 sysfs_remove_group+0x59/0xe0() > >> sysfs group ffffffff81c6c500 not found for kobject '0000:08' > >> Modules linked in: fuse hidp af_packet xt_tcpudp xt_pkttype xt_LOG xt_limit ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_raw ipt_REJECT iptable_raw xt_CT iptable_filter ip6table_mangle nf_conntrack_netbios_ns nf_conntrack_broadcast nf_conntrack_ipv4 nf_defrag_ipv4 ip_tables xt_conntrack nf_conntrack rfcomm ip6table_filter bnep ip6_tables x_tables arc4 ath9k mac80211 x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul iTCO_wdt crc32c_intel iTCO_vendor_support ghash_clmulni_intel aesni_intel ablk_helper acer_wmi sparse_keymap ath9k_common ath9k_hw cryptd lrw gf128mul ath3k glue_helper aes_x86_64 btusb microcode ath pcspkr joydev uvcvideo serio_raw videobuf2_core i2c_i801 videodev snd_hda_codec_hdmi cfg80211 videobuf2_vmalloc tg3 videobuf2_memops sg ptp pps_core lpc_ich mfd_core snd_hda_codec_realtek bluetooth hid_logitech_dj rfkill snd_hda_intel snd_hda_codec shpchp battery ac wmi acpi_cpufreq edd snd_usb_audio snd_pcm snd_page_alloc snd_hwdep snd_usbmidi_lib > >> snd_rawmidi snd_seq snd_timer snd_seq_device snd soundcore autofs4 xhci_hcd processor scsi_dh_hp_sw scsi_dh_rdac scsi_dh_emc scsi_dh_alua scsi_dh > >> CPU: 3 PID: 76 Comm: kworker/u16:7 Not tainted 3.13.0-rc1+ #76 > >> Hardware name: Acer Aspire S5-391/Venus , BIOS V1.02 05/29/2012 > >> Workqueue: kacpi_hotplug acpi_hotplug_work_fn > >> 0000000000000009 ffff8801644b9ac8 ffffffff816b23bf 0000000000000007 > >> ffff8801644b9b18 ffff8801644b9b08 ffffffff81046607 ffff88016925b800 > >> 0000000000000000 ffffffff81c6c500 ffff88016924f928 ffff88016924f800 > >> Call Trace: > >> [] dump_stack+0x4e/0x71 > >> [] warn_slowpath_common+0x87/0xb0 > >> [] warn_slowpath_fmt+0x41/0x50 > >> [] ? sysfs_get_dirent_ns+0x6f/0x80 > >> [] sysfs_remove_group+0x59/0xe0 > >> [] dpm_sysfs_remove+0x3b/0x50 > >> [] device_del+0x58/0x1c0 > >> [] device_unregister+0x48/0x60 > >> [] pci_remove_bus+0x6e/0x80 > >> [] pci_remove_bus_device+0x38/0x110 > >> [] pci_remove_bus_device+0x4d/0x110 > >> [] pci_stop_and_remove_bus_device+0x19/0x20 > >> [] disable_slot+0x20/0xe0 > >> [] acpiphp_check_bridge+0xa8/0xd0 > >> [] hotplug_event+0x17d/0x220 > >> [] hotplug_event_work+0x30/0x70 > >> [] acpi_hotplug_work_fn+0x18/0x24 > >> [] process_one_work+0x261/0x450 > >> [] worker_thread+0x21e/0x370 > >> [] ? rescuer_thread+0x300/0x300 > >> [] kthread+0xd2/0xe0 > >> [] ? flush_kthread_worker+0x70/0x70 > >> [] ret_from_fork+0x7c/0xb0 > >> [] ? flush_kthread_worker+0x70/0x70 > >> > >> (Mika Westerberg sees them too in his tests). > >> > >> Some investigation documented in kernel bug #65281 lead me to the > >> conclusion that the source of the problem is the device_del() in > >> pci_stop_dev() as it now causes the sysfs directory of the device > >> to be removed recursively along with all of its subdirectories. > >> That includes the sysfs directory of the device's subordinate > >> bus (dev->subordinate) and its "power" group. > >> > >> Consequently, when pci_remove_bus() is called for dev->subordinate > >> in pci_remove_bus_device(), it calls device_unregister(&bus->dev), > >> but at this point the sysfs directory of bus->dev doesn't exist any > >> more and its "power" group doesn't exist either. Thus, when > >> dpm_sysfs_remove() called from device_del() tries to remove that > >> group, it triggers the above warning. > >> > >> That indicates a logical mistake in the design of > >> pci_stop_and_remove_bus_device(), which causes bus device objects > >> to be left behind their parents (bridge device objects) and can be > >> fixed by moving the device_del() from pci_stop_dev() into > >> pci_destroy_dev(), so pci_remove_bus() can be called for the > >> device's subordinate bus before the device itself is unregistered > >> from the hierarchy. Still, the driver, if any, should be detached > >> from the device in pci_stop_dev(), so use device_release_driver() > >> directly from there. > >> > >> References: https://bugzilla.kernel.org/show_bug.cgi?id=65281#c6 > >> Reported-by: Mika Westerberg > >> Signed-off-by: Rafael J. Wysocki > >> --- > >> drivers/pci/remove.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> Index: linux-pm/drivers/pci/remove.c > >> =================================================================== > >> --- linux-pm.orig/drivers/pci/remove.c > >> +++ linux-pm/drivers/pci/remove.c > >> @@ -24,7 +24,7 @@ static void pci_stop_dev(struct pci_dev > >> if (dev->is_added) { > >> pci_proc_detach_device(dev); > >> pci_remove_sysfs_dev_files(dev); > >> - device_del(&dev->dev); > >> + device_release_driver(&dev->dev); > >> dev->is_added = 0; > >> } > >> > >> @@ -34,6 +34,8 @@ static void pci_stop_dev(struct pci_dev > >> > >> static void pci_destroy_dev(struct pci_dev *dev) > >> { > >> + device_del(&dev->dev); > >> + > >> down_write(&pci_bus_sem); > >> list_del(&dev->bus_list); > >> up_write(&pci_bus_sem); > >> > > > > Maybe that is not enough. could still need add is_removed ... > > > > Please check > > > > https://lkml.org/lkml/2013/5/13/653 > > PATCH 3/7] PCI: Detach driver in pci_stop_device > > or you can check if > http://patchwork.ozlabs.org/patch/292622/ > [1/6] PCI: move back pci_proc_attach_devices calling > http://patchwork.ozlabs.org/patch/292623/ > [2/6] PCI: move resources and bus_list releasing to pci_release_dev > http://patchwork.ozlabs.org/patch/292624/ > [3/6] PCI: Destroy pci dev only once > > could help with you test setup. I honestly don't think that all of the additional changes are needed to fix this particular problem. Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.