From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
alan@lxorguk.ukuu.org.uk, Huang Ying <ying.huang@intel.com>,
Bjorn Helgaas <bhelgaas@google.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
Zhang Yanmin <yanmin.zhang@intel.com>
Subject: [ 02/38] PCI/PM: Fix deadlock when unbinding device if parent in D3cold
Date: Wed, 21 Nov 2012 16:39:45 -0800 [thread overview]
Message-ID: <20121122003904.516735571@linuxfoundation.org> (raw)
In-Reply-To: <20121122003904.262382971@linuxfoundation.org>
3.0-stable review patch. If anyone has any objections, please let me know.
------------------
From: Huang Ying <ying.huang@intel.com>
commit 90b5c1d7c45eeb622302680ff96ed30c1a2b6f0e upstream.
If a PCI device and its parents are put into D3cold, unbinding the
device will trigger deadlock as follow:
- driver_unbind
- device_release_driver
- device_lock(dev) <--- previous lock here
- __device_release_driver
- pm_runtime_get_sync
...
- rpm_resume(dev)
- rpm_resume(dev->parent)
...
- pci_pm_runtime_resume
...
- pci_set_power_state
- __pci_start_power_transition
- pci_wakeup_bus(dev->parent->subordinate)
- pci_walk_bus
- device_lock(dev) <--- deadlock here
If we do not do device_lock in pci_walk_bus, we can avoid deadlock.
Device_lock in pci_walk_bus is introduced in commit:
d71374dafbba7ec3f67371d3b7e9f6310a588808, corresponding email thread
is: https://lkml.org/lkml/2006/5/26/38. The patch author Zhang Yanmin
said device_lock is added to pci_walk_bus because:
Some error handling functions call pci_walk_bus. For example, PCIe
aer. Here we lock the device, so the driver wouldn't detach from the
device, as the cb might call driver's callback function.
So I fixed the deadlock as follows:
- remove device_lock from pci_walk_bus
- add device_lock into callback if callback will call driver's callback
I checked pci_walk_bus users one by one, and found only PCIe aer needs
device lock.
Signed-off-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
CC: Zhang Yanmin <yanmin.zhang@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/pci/bus.c | 3 ---
drivers/pci/pcie/aer/aerdrv_core.c | 20 ++++++++++++++++----
2 files changed, 16 insertions(+), 7 deletions(-)
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -284,10 +284,7 @@ void pci_walk_bus(struct pci_bus *top, i
} else
next = dev->bus_list.next;
- /* Run device routines with the device locked */
- device_lock(&dev->dev);
retval = cb(dev, userdata);
- device_unlock(&dev->dev);
if (retval)
break;
}
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -243,6 +243,7 @@ static int report_error_detected(struct
struct aer_broadcast_data *result_data;
result_data = (struct aer_broadcast_data *) data;
+ device_lock(&dev->dev);
dev->error_state = result_data->state;
if (!dev->driver ||
@@ -261,12 +262,14 @@ static int report_error_detected(struct
dev->driver ?
"no AER-aware driver" : "no driver");
}
- return 0;
+ goto out;
}
err_handler = dev->driver->err_handler;
vote = err_handler->error_detected(dev, result_data->state);
result_data->result = merge_result(result_data->result, vote);
+out:
+ device_unlock(&dev->dev);
return 0;
}
@@ -277,14 +280,17 @@ static int report_mmio_enabled(struct pc
struct aer_broadcast_data *result_data;
result_data = (struct aer_broadcast_data *) data;
+ device_lock(&dev->dev);
if (!dev->driver ||
!dev->driver->err_handler ||
!dev->driver->err_handler->mmio_enabled)
- return 0;
+ goto out;
err_handler = dev->driver->err_handler;
vote = err_handler->mmio_enabled(dev);
result_data->result = merge_result(result_data->result, vote);
+out:
+ device_unlock(&dev->dev);
return 0;
}
@@ -295,14 +301,17 @@ static int report_slot_reset(struct pci_
struct aer_broadcast_data *result_data;
result_data = (struct aer_broadcast_data *) data;
+ device_lock(&dev->dev);
if (!dev->driver ||
!dev->driver->err_handler ||
!dev->driver->err_handler->slot_reset)
- return 0;
+ goto out;
err_handler = dev->driver->err_handler;
vote = err_handler->slot_reset(dev);
result_data->result = merge_result(result_data->result, vote);
+out:
+ device_unlock(&dev->dev);
return 0;
}
@@ -310,15 +319,18 @@ static int report_resume(struct pci_dev
{
struct pci_error_handlers *err_handler;
+ device_lock(&dev->dev);
dev->error_state = pci_channel_io_normal;
if (!dev->driver ||
!dev->driver->err_handler ||
!dev->driver->err_handler->resume)
- return 0;
+ goto out;
err_handler = dev->driver->err_handler;
err_handler->resume(dev);
+out:
+ device_unlock(&dev->dev);
return 0;
}
next prev parent reply other threads:[~2012-11-22 21:37 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20121122003904.262382971@linuxfoundation.org>
2012-11-22 0:39 ` [ 01/38] mm: bugfix: set current->reclaim_state to NULL while returning from kswapd() Greg Kroah-Hartman
2012-11-22 0:39 ` Greg Kroah-Hartman [this message]
2012-11-23 2:35 ` [ 02/38] PCI/PM: Fix deadlock when unbinding device if parent in D3cold Ben Hutchings
2012-11-23 3:09 ` Huang Ying
2012-11-23 7:47 ` Huang Ying
2012-11-30 2:01 ` Greg Kroah-Hartman
2012-11-30 2:54 ` Huang Ying
2012-12-11 8:12 ` Huang Ying
2012-12-11 18:08 ` Greg Kroah-Hartman
2012-12-14 7:08 ` Huang Ying
2012-12-14 21:56 ` Greg Kroah-Hartman
2012-11-26 18:55 ` Greg Kroah-Hartman
2012-11-26 19:08 ` Greg Kroah-Hartman
2012-11-26 19:30 ` Greg Kroah-Hartman
2012-11-27 0:28 ` Huang Ying
2012-11-22 0:39 ` [ 03/38] fanotify: fix missing break Greg Kroah-Hartman
2012-11-22 0:39 ` [ 04/38] crypto: cryptd - disable softirqs in cryptd_queue_worker to prevent data corruption Greg Kroah-Hartman
2012-11-22 0:39 ` Greg Kroah-Hartman
2012-11-22 0:39 ` [ 05/38] ptp: update adjfreq callback description Greg Kroah-Hartman
2012-11-24 0:26 ` Herton Ronaldo Krzesinski
2012-11-26 18:46 ` Greg Kroah-Hartman
2012-11-26 21:19 ` Keller, Jacob E
2012-11-22 0:39 ` [ 06/38] ALSA: hda: Cirrus: Fix coefficient index for beep configuration Greg Kroah-Hartman
2012-11-22 0:39 ` [ 07/38] ALSA: hda - Force to reset IEC958 status bits for AD codecs Greg Kroah-Hartman
2012-11-22 0:39 ` [ 08/38] ASoC: wm8978: pll incorrectly configured when codec is master Greg Kroah-Hartman
2012-11-22 0:39 ` [ 09/38] ASoC: dapm: Use card_list during DAPM shutdown Greg Kroah-Hartman
2012-11-22 0:39 ` [ 10/38] UBIFS: fix mounting problems after power cuts Greg Kroah-Hartman
2012-11-22 0:39 ` [ 11/38] UBIFS: introduce categorized lprops counter Greg Kroah-Hartman
2012-11-22 0:39 ` [ 12/38] s390/gup: add missing TASK_SIZE check to get_user_pages_fast() Greg Kroah-Hartman
2012-11-22 0:39 ` [ 13/38] USB: option: add Novatel E362 and Dell Wireless 5800 USB IDs Greg Kroah-Hartman
2012-11-22 0:39 ` [ 14/38] USB: option: add Alcatel X220/X500D " Greg Kroah-Hartman
2012-11-22 0:39 ` [ 15/38] wireless: allow 40 MHz on world roaming channels 12/13 Greg Kroah-Hartman
2012-11-22 0:39 ` [ 16/38] m68k: fix sigset_t accessor functions Greg Kroah-Hartman
2012-11-22 0:40 ` [ 17/38] ipv4: avoid undefined behavior in do_ip_setsockopt() Greg Kroah-Hartman
2012-11-22 0:40 ` [ 18/38] ipv6: setsockopt(IPIPPROTO_IPV6, IPV6_MINHOPCOUNT) forgot to set return value Greg Kroah-Hartman
2012-11-22 0:40 ` [ 19/38] net: correct check in dev_addr_del() Greg Kroah-Hartman
2012-11-22 0:40 ` [ 20/38] net-rps: Fix brokeness causing OOO packets Greg Kroah-Hartman
2012-11-22 0:40 ` [ 21/38] r8169: use unlimited DMA burst for TX Greg Kroah-Hartman
2012-11-22 0:40 ` [ 22/38] kbuild: Fix gcc -x syntax Greg Kroah-Hartman
2012-11-22 0:40 ` [ 23/38] netfilter: Validate the sequence number of dataless ACK packets as well Greg Kroah-Hartman
2012-11-22 0:40 ` [ 24/38] netfilter: Mark SYN/ACK packets as invalid from original direction Greg Kroah-Hartman
2012-11-22 0:40 ` [ 25/38] netfilter: nf_nat: dont check for port change on ICMP tuples Greg Kroah-Hartman
2012-11-22 0:40 ` [ 26/38] usb: use usb_serial_put in usb_serial_probe errors Greg Kroah-Hartman
2012-11-22 0:40 ` [ 27/38] eCryptfs: Copy up POSIX ACL and read-only flags from lower mount Greg Kroah-Hartman
2012-11-22 0:40 ` [ 28/38] eCryptfs: check for eCryptfs cipher support at mount Greg Kroah-Hartman
2012-11-22 0:40 ` [ 29/38] sky2: Fix for interrupt handler Greg Kroah-Hartman
2012-11-22 0:40 ` [ 30/38] drm/i915: fix overlay on i830M Greg Kroah-Hartman
2012-11-22 0:40 ` [ 31/38] NFS: Wait for session recovery to finish before returning Greg Kroah-Hartman
2012-11-22 0:40 ` [ 32/38] reiserfs: Fix lock ordering during remount Greg Kroah-Hartman
2012-11-22 0:40 ` [ 33/38] reiserfs: Protect reiserfs_quota_on() with write lock Greg Kroah-Hartman
2012-11-22 0:40 ` [ 34/38] reiserfs: Move quota calls out of " Greg Kroah-Hartman
2012-11-22 0:40 ` [ 35/38] reiserfs: Protect reiserfs_quota_write() with " Greg Kroah-Hartman
2012-11-22 0:40 ` [ 36/38] selinux: fix sel_netnode_insert() suspicious rcu dereference Greg Kroah-Hartman
2012-11-22 0:40 ` [ 37/38] PCI : ability to relocate assigned pci-resources Greg Kroah-Hartman
2012-11-23 13:29 ` Herton Ronaldo Krzesinski
[not found] ` <20121124014141.GD2752@ram.oc3035372033.ibm.com>
2012-11-26 18:53 ` Greg Kroah-Hartman
2012-11-22 0:40 ` [ 38/38] PCI : Calculate right add_size Greg Kroah-Hartman
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=20121122003904.516735571@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=bhelgaas@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rafael.j.wysocki@intel.com \
--cc=stable@vger.kernel.org \
--cc=yanmin.zhang@intel.com \
--cc=ying.huang@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.