Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1265558 > unrolled thread
| Started by | Sanjeev Sharma <sanjeev_sharma@mentor.com> |
|---|---|
| First post | 2015-11-09 11:50 +0100 |
| Last post | 2015-11-16 10:40 +0100 |
| Articles | 6 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] PCI: imx6:don't sleep in atomic context Sanjeev Sharma <sanjeev_sharma@mentor.com> - 2015-11-09 11:50 +0100
Re: [PATCH] PCI: imx6:don't sleep in atomic context Lucas Stach <l.stach@pengutronix.de> - 2015-11-10 09:50 +0100
Re: [PATCH] PCI: imx6:don't sleep in atomic context Arnd Bergmann <arnd@arndb.de> - 2015-11-10 10:40 +0100
Re: [PATCH] PCI: imx6:don't sleep in atomic context Lucas Stach <l.stach@pengutronix.de> - 2015-11-10 10:40 +0100
Re: [PATCH] PCI: imx6:don't sleep in atomic context Arnd Bergmann <arnd@arndb.de> - 2015-11-10 10:50 +0100
RE: [PATCH] PCI: imx6:don't sleep in atomic context "Sharma, Sanjeev" <Sanjeev_Sharma@mentor.com> - 2015-11-16 10:40 +0100
| From | Sanjeev Sharma <sanjeev_sharma@mentor.com> |
|---|---|
| Date | 2015-11-09 11:50 +0100 |
| Subject | [PATCH] PCI: imx6:don't sleep in atomic context |
| Message-ID | <qsSbn-5gy-11@gated-at.bofh.it> |
If additional PCIe switch get connected between the host and the NIC,the kernel crashes with "BUG: scheduling while atomic". To handle this we need to call mdelay() instead of usleep_range(). For more detail please refer bugzilla.kernel.org, Bug 100031 Signed-off-by: Sanjeev Sharma <sanjeev_sharma@mentor.com> Signed-off-by: David Mueller <dave.mueller@gmx.ch> --- drivers/pci/host/pci-imx6.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c index 233a196..9769b13 100644 --- a/drivers/pci/host/pci-imx6.c +++ b/drivers/pci/host/pci-imx6.c @@ -499,7 +499,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); + mdelay(1000); } /* * From L0, initiate MAC entry to gen2 if EP/RC supports gen2. -- 1.7.11.7 -- 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/
[toc] | [next] | [standalone]
| From | Lucas Stach <l.stach@pengutronix.de> |
|---|---|
| Date | 2015-11-10 09:50 +0100 |
| Message-ID | <qtcMN-2IH-1@gated-at.bofh.it> |
| In reply to | #1265558 |
Am Montag, den 09.11.2015, 16:18 +0530 schrieb Sanjeev Sharma: > If additional PCIe switch get connected between the > host and the NIC,the kernel crashes with "BUG: > scheduling while atomic". To handle this we need to > call mdelay() instead of usleep_range(). > > For more detail please refer bugzilla.kernel.org, Bug > 100031 > > Signed-off-by: Sanjeev Sharma <sanjeev_sharma@mentor.com> This is wrong. You are not the author of this patch and this should be reflected both in the From: line as well as in the order of signoffs. > Signed-off-by: David Mueller <dave.mueller@gmx.ch> > --- > drivers/pci/host/pci-imx6.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index 233a196..9769b13 100644 > --- a/drivers/pci/host/pci-imx6.c > +++ b/drivers/pci/host/pci-imx6.c > @@ -499,7 +499,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); > + mdelay(1000); A mdelay(1000) is a whole different timescale than a usleep(1000). If this patch works for you with mdelay(1) or maybe mdelay(2) I would be fine with it. Regards, Lucas > } > /* > * From L0, initiate MAC entry to gen2 if EP/RC supports gen2. -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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/
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2015-11-10 10:40 +0100 |
| Message-ID | <qtdzb-3g9-1@gated-at.bofh.it> |
| In reply to | #1266327 |
On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote: > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > > index 233a196..9769b13 100644 > > --- a/drivers/pci/host/pci-imx6.c > > +++ b/drivers/pci/host/pci-imx6.c > > @@ -499,7 +499,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); > > + mdelay(1000); > > A mdelay(1000) is a whole different timescale than a usleep(1000). If > this patch works for you with mdelay(1) or maybe mdelay(2) I would be > fine with it. mdelay(1) is still a really long time to block the CPU for, on potentially every config space access. Everybody else just returns the link status here, which seems to be the better alternative. If you need to delay the startup, better have a msleep(1) loop in the initial probe function where you are allowed to sleep. Arnd -- 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/
[toc] | [prev] | [next] | [standalone]
| From | Lucas Stach <l.stach@pengutronix.de> |
|---|---|
| Date | 2015-11-10 10:40 +0100 |
| Message-ID | <qtdzb-3g9-3@gated-at.bofh.it> |
| In reply to | #1266353 |
Am Dienstag, den 10.11.2015, 10:28 +0100 schrieb Arnd Bergmann: > On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote: > > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > > > index 233a196..9769b13 100644 > > > --- a/drivers/pci/host/pci-imx6.c > > > +++ b/drivers/pci/host/pci-imx6.c > > > @@ -499,7 +499,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); > > > + mdelay(1000); > > > > A mdelay(1000) is a whole different timescale than a usleep(1000). If > > this patch works for you with mdelay(1) or maybe mdelay(2) I would be > > fine with it. > > mdelay(1) is still a really long time to block the CPU for, on potentially > every config space access. > > Everybody else just returns the link status here, which seems to be > the better alternative. If you need to delay the startup, better have > a msleep(1) loop in the initial probe function where you are allowed to > sleep. > Yes, it's somewhere on my TODO list to rework the link-up handling here, but as there are quite a few timing and ordering implications in that code, this needs a good thought and a good deal of testing. So I'm inclined to ACK the current patch to get rid of the obvious bug and sort things out properly in a follow on patchset. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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/
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2015-11-10 10:50 +0100 |
| Message-ID | <qtdIS-3jG-19@gated-at.bofh.it> |
| In reply to | #1266354 |
On Tuesday 10 November 2015 10:35:10 Lucas Stach wrote:
> Am Dienstag, den 10.11.2015, 10:28 +0100 schrieb Arnd Bergmann:
> > On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote:
> > > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > > > index 233a196..9769b13 100644
> > > > --- a/drivers/pci/host/pci-imx6.c
> > > > +++ b/drivers/pci/host/pci-imx6.c
> > > > @@ -499,7 +499,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);
> > > > + mdelay(1000);
> > >
> > > A mdelay(1000) is a whole different timescale than a usleep(1000). If
> > > this patch works for you with mdelay(1) or maybe mdelay(2) I would be
> > > fine with it.
> >
> > mdelay(1) is still a really long time to block the CPU for, on potentially
> > every config space access.
> >
> > Everybody else just returns the link status here, which seems to be
> > the better alternative. If you need to delay the startup, better have
> > a msleep(1) loop in the initial probe function where you are allowed to
> > sleep.
> >
> Yes, it's somewhere on my TODO list to rework the link-up handling here,
> but as there are quite a few timing and ordering implications in that
> code, this needs a good thought and a good deal of testing. So I'm
> inclined to ACK the current patch to get rid of the obvious bug and sort
> things out properly in a follow on patchset.
Maybe use that patch with some modifications then:
* add a comment to explain that this is currently called from possibly
atomic context through pci_config_{read,write} and that the link
state handling never belonged here.
* instead of looping five times for up to 2ms each, loop 100 times
around a udelay(20) to hopefully be done earlier. I was going to
suggest using time_before(timeout, jiffies) as the condition to
wait for, but that doesn't work if called with interrupts disabled.
Arnd
--
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/
[toc] | [prev] | [next] | [standalone]
| From | "Sharma, Sanjeev" <Sanjeev_Sharma@mentor.com> |
|---|---|
| Date | 2015-11-16 10:40 +0100 |
| Message-ID | <qvoqv-5zh-25@gated-at.bofh.it> |
| In reply to | #1266363 |
On Tuesday 10 November 2015 10:35:10 Lucas Stach wrote:
> Am Dienstag, den 10.11.2015, 10:28 +0100 schrieb Arnd Bergmann:
> > On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote:
> > > > diff --git a/drivers/pci/host/pci-imx6.c
> > > > b/drivers/pci/host/pci-imx6.c index 233a196..9769b13 100644
> > > > --- a/drivers/pci/host/pci-imx6.c
> > > > +++ b/drivers/pci/host/pci-imx6.c
> > > > @@ -499,7 +499,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);
> > > > + mdelay(1000);
> > >
> > > A mdelay(1000) is a whole different timescale than a usleep(1000).
> > > If this patch works for you with mdelay(1) or maybe mdelay(2) I
> > > would be fine with it.
> >
> > mdelay(1) is still a really long time to block the CPU for, on
> > potentially every config space access.
> >
> > Everybody else just returns the link status here, which seems to be
> > the better alternative. If you need to delay the startup, better
> > have a msleep(1) loop in the initial probe function where you are
> > allowed to sleep.
> >
> Yes, it's somewhere on my TODO list to rework the link-up handling
> here, but as there are quite a few timing and ordering implications in
> that code, this needs a good thought and a good deal of testing. So
> I'm inclined to ACK the current patch to get rid of the obvious bug
> and sort things out properly in a follow on patchset.
Maybe use that patch with some modifications then:
* add a comment to explain that this is currently called from possibly
atomic context through pci_config_{read,write} and that the link
state handling never belonged here.
* instead of looping five times for up to 2ms each, loop 100 times
around a udelay(20) to hopefully be done earlier. I was going to
suggest using time_before(timeout, jiffies) as the condition to
wait for, but that doesn't work if called with interrupts disabled.
Arnd
Shall I go ahead by changing only current patch to mdelay(1). I will also
Incorporate comment #1 given by Arnd above.
--
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/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web