linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lucas Stach <l.stach@pengutronix.de>
To: Richard Zhu <r65037@freescale.com>
Cc: linux-pci-owner@vger.kernel.org, linux-pci@vger.kernel.org,
	shawn.guo@freescale.com, festevam@gmail.com,
	tharvey@gateworks.com
Subject: Re: [PATCH v4 03/10] PCI: imx6: Fix possible dead lock
Date: Tue, 30 Sep 2014 16:57:03 +0200	[thread overview]
Message-ID: <1412089023.2473.3.camel@pengutronix.de> (raw)
In-Reply-To: <1412069804-17162-4-git-send-email-r65037@freescale.com>

Am Dienstag, den 30.09.2014, 17:36 +0800 schrieb Richard Zhu:
> kernel report one possible dead lock during imx6sx pcie
> suspend resume stress tests, after enable Lock Debugging.
> platform: imx6sx sdb board + xhci(pcie2usb3.0 ep)
> 
> reason: usleep_range used in imx6_pcie_link_up maybe scheduled
> out from dw_pcie_valid_config.isra...
> About details, please see the following logs.
> 
> solution: replace the usleep_range(1000, 2000) by udelay(10) and
> enlarge the loop counter.
> 
> logs:
> [   50.643062] xhci_hcd 0000:01:00.0: Refused to change power state, currently in D3
> [   50.653390]
> [   50.654898] =========================================================
> [   50.661343] [ INFO: possible irq lock inversion dependency detected ]
> [   50.667792] 3.17.0-rc2-01341-gfc43ff7-dirty #101 Not tainted
> [   50.673454] ---------------------------------------------------------
> [   50.679898] kworker/u2:2/48 just changed the state of lock:
> [   50.685477]  (pci_lock){+.....}, at: [<802d650c>] pci_bus_read_config_dword+0x44/0x94
> [   50.693394] but this lock was taken by another, HARDIRQ-safe lock in the past:
> [   50.700619]  (&irq_desc_lock_class){-.-...}
> 
> and interrupts could create inverse lock ordering between them.
> 
> [   50.710843]
> [   50.710843] other info that might help us debug this:
> [   50.717377]  Possible interrupt unsafe locking scenario:
> [   50.717377]
> [   50.724169]        CPU0                    CPU1
> [   50.728702]        ----                    ----
> [   50.733234]   lock(pci_lock);
> [   50.736232]                                local_irq_disable();
> [   50.742154]                                lock(&irq_desc_lock_class);
> [   50.748713]                                lock(pci_lock);
> [   50.754229]   <Interrupt>
> [   50.756852]     lock(&irq_desc_lock_class);
> [   50.761065]
> [   50.761065]  *** DEADLOCK ***
> ...
> 
> [   52.119515] [<806e8ad0>] (schedule_hrtimeout_range) from [<80077e90>] (usleep_range+0x50/0x58)
> [   52.128141] [<80077e40>] (usleep_range) from [<802f3694>] (imx6_pcie_link_up+0x48/0x16c)
> [   52.136242] [<802f364c>] (imx6_pcie_link_up) from [<802f1b74>] (dw_pcie_valid_config.isra.10+0x40/0x7c)
> [   52.145637]  r6:ae72606d r5:ae72606c r4:ae711228
> [   52.150311] [<802f1b34>] (dw_pcie_valid_config.isra.10) from [<802f1ca8>] (dw_pcie_rd_conf+0x4c/0x154)
> [   52.159620]  r7:00000000 r6:00000000 r5:ae711228 r4:ae726000
> [   52.165355] [<802f1c5c>] (dw_pcie_rd_conf) from [<802d6534>] (pci_bus_read_config_dword+0x6c/0x94)
> [   52.174316]  r9:adf4e910 r8:809bc51c r7:00000000 r6:60000153 r5:adc3fd5c r4:ae726000
> [   52.182152] [<802d64c8>] (pci_bus_read_config_dword) from [<802db588>] (pci_restore_config_dword+0x54/0xa4)
> [   52.191894]  r6:00000024 r5:0000000a r4:ae61e000
> [   52.196570] [<802db534>] (pci_restore_config_dword) from [<802dd150>] (pci_restore_state.part.37+0x7c/0x1f8)
> [   52.206399]  r8:802e086c r7:ae61dfe8 r6:519e2024 r5:ae61e000 r4:ae61dffc
> [   52.213187] [<802dd0d4>] (pci_restore_state.part.37) from [<802dd2e8>] (pci_restore_state+0x1c/0x20)
> [   52.222322]  r7:adc3fea8 r6:809d90e0 r5:ae61e000 r4:ae61e068
> [   52.228056] [<802dd2cc>] (pci_restore_state) from [<802e0894>] (pci_pm_resume_noirq+0x28/0xa4)
> [   52.236683] [<802e086c>] (pci_pm_resume_noirq) from [<8037ea1c>] (dpm_run_callback.isra.12+0x34/0x7c)
> 
> Signed-off-by: Richard Zhu <r65037@freescale.com>

NACK. I don't like this spinning with the CPU and you are now possibly
dumping 500 debug prints into the logs until you reach the timeout.
Please remove this patch from this series. It has nothing to do with the
imx6sx support and I'll try to post a (hopefully) better patch shortly.

> ---
>  drivers/pci/host/pci-imx6.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index c74e87d..4334de6 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -519,7 +519,7 @@ static void imx6_pcie_reset_phy(struct pcie_port *pp)
>  static int imx6_pcie_link_up(struct pcie_port *pp)
>  {
>  	u32 rc, debug_r0, rx_valid;
> -	int count = 5;
> +	int count = 500;
>  
>  	/*
>  	 * Test if the PHY reports that the link is up and also that the LTSSM
> @@ -550,7 +550,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
>  		 * Wait a little bit, then re-check if the link finished
>  		 * the training.
>  		 */
> -		usleep_range(1000, 2000);
> +		udelay(10);
>  	}
>  	/*
>  	 * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |


  reply	other threads:[~2014-09-30 14:57 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-30  9:36 [PATCH v4]PCI: imx6: enable pcie on imx6sx sdb and imx6qdl sabreauto Richard Zhu
2014-09-30  9:36 ` [PATCH v4 01/10] PCI: imx6: wait the clocks to stabilize after ref_en Richard Zhu
2014-09-30  9:36 ` [PATCH v4 02/10] PCI: imx6: add imx6sx pcie support Richard Zhu
2014-09-30 14:54   ` Lucas Stach
2014-10-02  2:38     ` Hong-Xing.Zhu
2014-10-08  7:30       ` Hong-Xing.Zhu
2014-09-30  9:36 ` [PATCH v4 03/10] PCI: imx6: Fix possible dead lock Richard Zhu
2014-09-30 14:57   ` Lucas Stach [this message]
2014-09-30  9:36 ` [PATCH v4 04/10] PCI: designware: refine setup_rc and add msi data restore Richard Zhu
2014-09-30 14:58   ` Lucas Stach
2014-10-08  6:45     ` Hong-Xing.Zhu
2014-09-30  9:36 ` [PATCH v4 05/10] PCI: designware: fix one potential assignment error of cfg start Richard Zhu
2014-09-30  9:36 ` [PATCH v4 06/10] ARM: imx6qdl: enable pcie on imx6qdl sabreauto Richard Zhu
2014-09-30  9:36 ` [PATCH v4 07/10] ARM: imx6: update dts and binding for imx6sx pcie Richard Zhu
2014-09-30  9:36 ` [PATCH v4 08/10] ARM: imx6sx: add syscon into gpc dts Richard Zhu
2014-09-30  9:36 ` [PATCH v4 09/10] ARM: imx6sx: add imx6sx pcie related gpr bits definitions Richard Zhu
2014-09-30  9:36 ` [PATCH v4 10/10] ARM: imx6sx: enable pcie on imx6sx sdb board Richard Zhu
2014-09-30 16:21   ` Fabio Estevam
2014-10-02  2:40     ` Hong-Xing.Zhu
2014-10-08  6:41       ` Hong-Xing.Zhu
2014-10-09  1:11         ` Fabio Estevam
2014-10-09  5:39           ` Hong-Xing.Zhu
  -- strict thread matches above, loose matches on Subject: below --
2014-09-30  9:19 [PATCH v4]PCI: imx6: enable pcie on imx6sx sdb and imx6qdl sabreauto Richard Zhu
2014-09-30  9:19 ` [PATCH v4 03/10] PCI: imx6: Fix possible dead lock Richard Zhu

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=1412089023.2473.3.camel@pengutronix.de \
    --to=l.stach@pengutronix.de \
    --cc=festevam@gmail.com \
    --cc=linux-pci-owner@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=r65037@freescale.com \
    --cc=shawn.guo@freescale.com \
    --cc=tharvey@gateworks.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;
as well as URLs for NNTP newsgroup(s).