On 10/15/2019 3:00 PM, Thierry Reding wrote:
On Mon, Oct 14, 2019 at 01:21:31PM -0700, Sinan Kaya wrote:
On 10/14/2019 1:20 AM, Thierry Reding wrote:
I think this makes sense, so:
Reviewed-by: Thierry Reding <treding@xxxxxxxxxx>
However, it looks like Sinan has researched this extensively in the past
and gave a presentation on this at Plumbers in 2017:
https://blog.linuxplumbersconf.org/2017/ocw/system/presentations/4732/original/crs.pdf
Adding Sinan to see if he has any concerns about this, since resume time
is explicitly mentioned in the above slides.
Thanks for including me. Let me catch up here.
pci_dev_wait() is supposed to handle this case via pci_pm_reset().
/**
* pci_pm_reset - Put device into PCI_D3 and back into PCI_D0.
* @dev: Device to reset.
* @probe: If set, only check if the device can be reset this way.
*/
Do you know if your execution path hits this function? We might have
missed a use case.
I see only a couple of callers of pci_device_is_present() in the tree,
this being from next-20191015:
$ git grep -n pci_device_is_present
drivers/net/ethernet/broadcom/tg3.c:9070: if (!pci_device_is_present(tp->pdev))drivers/net/ethernet/broadcom/tg3.c:11785: if (pci_device_is_present(tp->pdev)) {
drivers/net/ethernet/intel/igb/igb_main.c:8838: if (!pci_device_is_present(pdev))
drivers/nvme/host/pci.c:2866: if (!pci_device_is_present(pdev)) {
drivers/pci/hotplug/acpiphp_glue.c:650: alive = pci_device_is_present(dev);
drivers/pci/pci.c:935: !pci_device_is_present(dev)) {
drivers/pci/pci.c:5902:bool pci_device_is_present(struct pci_dev *pdev)
drivers/pci/pci.c:5910:EXPORT_SYMBOL_GPL(pci_device_is_present);
drivers/thunderbolt/nhi.c:939: if (!pci_device_is_present(pdev)) {
include/linux/pci.h:1206:bool pci_device_is_present(struct pci_dev *pdev);
I think the important one is the following which is called from inside
pci_update_current_state() function.
drivers/pci/pci.c:935: !pci_device_is_present(dev)) {
I've put a dump_stack() to see how this is called and following is the trace
[ 36.380726] Call trace:
[ 36.383270] dump_backtrace+0x0/0x158
[ 36.386802] show_stack+0x14/0x20
[ 36.389749] dump_stack+0xb0/0xf8
[ 36.393451] pci_update_current_state+0x58/0xe0
[ 36.398178] pci_power_up+0x60/0x70
[ 36.401672] pci_pm_resume_noirq+0x6c/0x130
[ 36.405669] dpm_run_callback.isra.16+0x20/0x70
[ 36.410248] device_resume_noirq+0x120/0x238
[ 36.414364] async_resume_noirq+0x24/0x58
[ 36.418364] async_run_entry_fn+0x40/0x148
[ 36.422418] process_one_work+0x1e8/0x360
[ 36.426525] worker_thread+0x40/0x488
[ 36.430201] kthread+0x118/0x120
[ 36.433843] ret_from_fork+0x10/0x1c
The NVME driver has the call in the ->remove() callback, so I don't
think it's relevant here. Both TG3 and IGB ethernet drivers seem to call
this during resume and so does Thunderbolt.
Vidya, can you clarify for which device you're seeing the issues? Sounds
like adding a call to pci_pm_reset() (via pci_reset_function()) at some
point.
With 0 sec wait, I see issue with Intel 750 NVMe card. As I mentioned above,
it is called from the PCI-PM subsystem which is where the timeout is required.
Sinan, it looks as if pci_pm_reset() (or any of its callers) is not used
very widely. Is that just because most drivers haven't had a need for it
yet? Or am I missing some core functionality that calls this for every
device anyway?
Thierry
