All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Adrian Bunk <bunk@stusta.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Thomas Meyer <thomas@m3y3r.de>,
	Frederic Riss <frederic.riss@gmail.com>,
	Marcus Better <marcus@better.se>, Len Brown <lenb@kernel.org>
Subject: [patch] MSI-X: fix resume crash
Date: Wed, 28 Mar 2007 15:06:30 +0200	[thread overview]
Message-ID: <20070328130630.GA28108@elte.hu> (raw)
In-Reply-To: <20070328130326.GA27126@elte.hu>


* Ingo Molnar <mingo@elte.hu> wrote:

> [...] I'll now re-test Eric's MSI patch.

Eric's patch seems to have done the trick on my T60: i've done 10 
suspend+resumes and each worked fine. I've tidied up the description 
part of Eric's patch a bit for upstream application - find it below.

	Ingo

---------------------->
Subject: [patch] MSI-X: fix resume crash
From: Eric W. Biederman <ebiederm@xmission.com>

I think the right solution is to simply make pci_enable_device just flip 
enable bits and move the rest of the work someplace else.

However a thorough cleanup is a little extreme for this point in the 
release cycle, so I think a quick hack that makes the code not stomp the 
irq when msi irq's are enabled should be the first fix.  Then we can 
later make the code not change the irqs at all.

Tony, Len the way pci_disable_device is being used in a suspend/resume 
path by a few drivers is completely incompatible with the way irqs are 
allocated on ia64.  In particular people the following sequence occurs 
in several drivers.

probe:
  pci_enable_device(pdev);
  request_irq(pdev->irq);
suspend:
  pci_disable_device(pdev);
resume:
  pci_enable_device(pdev);
remove:
  free_irq(pdev->irq);
  pci_disable_device(pdev);

What I'm proposing we do is move the irq allocation code out of 
pci_enable_device and the irq freeing code out of pci_disable_device in 
the future.  If we move ia64 to a model where the irq number equal the 
gsi like we have for x86_64 and are in the middle of for i386 that 
should be pretty straight forward.  It would even be relatively simple 
to delay vector allocation in that context until request_irq, if we 
needed the delayed allocation benefit.  Do you two have any problems 
with moving in that direction?

If fixing the arch code is unacceptable for some reason I'm not aware of 
we need to audit the 10-20 drivers that call pci_disable_device in their 
suspend/resume processing and ensure that they have freed all of the 
irqs before that point.  Given that I have bug reports on the msi path I 
know that isn't true.

From: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/cris/arch-v32/drivers/pci/bios.c |    4 +++-
 arch/frv/mb93090-mb00/pci-vdk.c       |    3 ++-
 arch/i386/pci/common.c                |    6 ++++--
 arch/ia64/pci/pci.c                   |    8 ++++++--
 4 files changed, 15 insertions(+), 6 deletions(-)

Index: linux/arch/cris/arch-v32/drivers/pci/bios.c
===================================================================
--- linux.orig/arch/cris/arch-v32/drivers/pci/bios.c
+++ linux/arch/cris/arch-v32/drivers/pci/bios.c
@@ -100,7 +100,9 @@ int pcibios_enable_device(struct pci_dev
 	if ((err = pcibios_enable_resources(dev, mask)) < 0)
 		return err;
 
-	return pcibios_enable_irq(dev);
+	if (!dev->msi_enabled)
+		pcibios_enable_irq(dev);
+	return 0;
 }
 
 int pcibios_assign_resources(void)
Index: linux/arch/frv/mb93090-mb00/pci-vdk.c
===================================================================
--- linux.orig/arch/frv/mb93090-mb00/pci-vdk.c
+++ linux/arch/frv/mb93090-mb00/pci-vdk.c
@@ -466,6 +466,7 @@ int pcibios_enable_device(struct pci_dev
 
 	if ((err = pcibios_enable_resources(dev, mask)) < 0)
 		return err;
-	pcibios_enable_irq(dev);
+	if (!dev->msi_enabled)
+		pcibios_enable_irq(dev);
 	return 0;
 }
Index: linux/arch/i386/pci/common.c
===================================================================
--- linux.orig/arch/i386/pci/common.c
+++ linux/arch/i386/pci/common.c
@@ -434,11 +434,13 @@ int pcibios_enable_device(struct pci_dev
 	if ((err = pcibios_enable_resources(dev, mask)) < 0)
 		return err;
 
-	return pcibios_enable_irq(dev);
+	if (!dev->msi_enabled)
+		return pcibios_enable_irq(dev);
+	return 0;
 }
 
 void pcibios_disable_device (struct pci_dev *dev)
 {
-	if (pcibios_disable_irq)
+	if (!dev->msi_enabled && pcibios_disable_irq)
 		pcibios_disable_irq(dev);
 }
Index: linux/arch/ia64/pci/pci.c
===================================================================
--- linux.orig/arch/ia64/pci/pci.c
+++ linux/arch/ia64/pci/pci.c
@@ -557,14 +557,18 @@ pcibios_enable_device (struct pci_dev *d
 	if (ret < 0)
 		return ret;
 
-	return acpi_pci_irq_enable(dev);
+	if (!dev->msi_enabled)
+		return acpi_pci_irq_enable(dev);
+	return 0;
 }
 
 void
 pcibios_disable_device (struct pci_dev *dev)
 {
 	BUG_ON(atomic_read(&dev->enable_cnt));
-	acpi_pci_irq_disable(dev);
+	if (!dev->msi_enabled)
+		acpi_pci_irq_disable(dev);
+	return 0;
 }
 
 void

  reply	other threads:[~2007-03-28 13:08 UTC|newest]

Thread overview: 128+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-25 23:08 Linux 2.6.21-rc5 Linus Torvalds
2007-03-26  8:31 ` Ingo Molnar
2007-03-26  8:17   ` Ayaz Abdulla
2007-03-26  8:39   ` Ingo Molnar
2007-03-26  8:58     ` [patch] forcedeth: work around NULL skb dereference crash Ingo Molnar
2007-04-02 11:56       ` [patch] forcedeth: improve NAPI logic Ingo Molnar
2007-03-26  8:55 ` Linux 2.6.21-rc5 Thomas Gleixner
2007-03-26 12:25   ` Bob Tracy
2007-03-26 12:30     ` Thomas Gleixner
2007-03-26  9:04 ` 2.6.21-rc5: maxcpus=1 crash in cpufreq: kernel BUG at drivers/cpufreq/cpufreq.c:82! Ingo Molnar
2007-03-26 18:12   ` Venki Pallipadi
2007-03-26 19:03     ` Venki Pallipadi
2007-03-27  7:11       ` Ingo Molnar
2007-03-26  9:21 ` [PATCH] clockevents: remove bad designed sysfs support for now Thomas Gleixner
2007-03-26  9:25   ` Ingo Molnar
2007-03-26 18:57     ` Greg KH
2007-03-26 12:51   ` Pavel Machek
2007-03-27  7:08   ` [PATCH] i386: Fix bogus return value in hpet_next_event() Thomas Gleixner
2007-03-26 10:11 ` -rc5: e1000 resume weirdness Ingo Molnar
2007-03-26 15:39   ` Kok, Auke
2007-03-26 15:50   ` Jesse Brandeburg
2007-03-26 15:55     ` Kok, Auke
2007-03-26 17:39     ` Ingo Molnar
2007-03-27  1:59 ` [1/5] 2.6.21-rc5: known regressions Adrian Bunk
2007-03-28 18:54   ` Kok, Auke
2007-03-28 19:23     ` Ingo Molnar
2007-03-30 18:04     ` Adrian Bunk
2007-03-30 12:04   ` [bug] hung bootup in various drivers, was: "2.6.21-rc5: known regressions" Ingo Molnar
2007-03-30 12:06     ` [bug] fixed_init(): BUG: at drivers/base/core.c:120 device_release(), " Ingo Molnar
2007-03-30 14:18       ` Greg KH
2007-03-30 14:25         ` Ingo Molnar
2007-03-30 16:31           ` Vitaly Bordug
2007-03-30 14:16     ` [bug] hung bootup in various drivers, " Greg KH
2007-03-30 17:46       ` Ingo Molnar
2007-03-30 19:32         ` Greg KH
2007-03-31  2:32           ` Kay Sievers
2007-03-31 16:51             ` [patch] driver core: fix built-in drivers sysfs links Ingo Molnar
2007-03-31 16:31           ` [bug] hung bootup in various drivers, was: "2.6.21-rc5: known regressions" Ingo Molnar
2007-04-01  7:49     ` Pavel Machek
2007-04-01 17:17       ` Linus Torvalds
2007-04-01 17:35         ` [patch] driver core: if built-in, do not wait in driver_unregister() Ingo Molnar
2007-04-02  1:47           ` Greg KH
2007-03-27  1:59 ` [2/5] 2.6.21-rc5: known regressions Adrian Bunk
2007-03-27  1:59   ` Adrian Bunk
2007-03-27  1:59   ` Adrian Bunk
2007-03-28 19:46   ` Laurent Riffard
2007-03-29 19:02     ` Fabio Comolli
2007-03-27  1:59 ` [3/5] " Adrian Bunk
2007-03-27  1:59 ` [4/5] " Adrian Bunk
2007-03-27  1:59   ` Adrian Bunk
2007-03-27  8:00   ` Marcus Better
2007-03-27 13:25     ` Eric W. Biederman
2007-03-27 16:53       ` Marcus Better
2007-03-27 20:50         ` Eric W. Biederman
2007-03-27 10:09   ` Rafael J. Wysocki
2007-03-27 10:09     ` Rafael J. Wysocki
2007-03-27 22:29     ` Adrian Bunk
2007-03-27 22:29       ` Adrian Bunk
2007-03-27 22:45       ` Thomas Meyer
2007-03-27 22:45         ` Thomas Meyer
2007-03-28 12:19   ` Ingo Molnar
2007-03-28 12:41     ` Ingo Molnar
2007-03-28 13:03       ` Ingo Molnar
2007-03-28 13:06         ` Ingo Molnar [this message]
2007-03-28 13:31           ` [patch] MSI-X: fix resume crash Eric W. Biederman
2007-03-28 13:36             ` Ingo Molnar
2007-03-29  4:30           ` Len Brown
2007-03-29  4:57             ` Eric W. Biederman
2007-03-27  1:59 ` [5/5] 2.6.21-rc5: known regressions Adrian Bunk
2007-03-27  1:59   ` Adrian Bunk
2007-03-27  5:51 ` ATA ACPI (was Re: Linux 2.6.21-rc5) Jeff Garzik
2007-03-27  5:54   ` Tejun Heo
2007-03-27 21:32     ` Pavel Machek
2007-03-28  9:51       ` Tejun Heo
2007-03-27 17:07   ` Linus Torvalds
2007-03-27 18:48     ` Jeff Garzik
2007-03-27  6:17 ` Linux 2.6.21-rc5 Andrew Morton
2007-03-27  6:20   ` Greg KH
2007-03-27 16:49     ` Jesse Barnes
2007-03-27  9:49   ` Takashi Iwai
2007-03-27 12:25   ` Andi Kleen
2007-03-27 16:33     ` Andrew Morton
2007-03-27 12:43   ` Dmitry Torokhov
2007-03-28 22:32   ` Tilman Schmidt
2007-03-27 18:34 ` Michal Piotrowski
2007-03-27 22:29   ` Pavel Machek
2007-03-27 22:55     ` Michal Piotrowski
2007-03-27 18:53 ` Michal Piotrowski
2007-03-28 14:30   ` Andi Kleen
2007-03-28 14:56     ` Michal Piotrowski
2007-03-28 16:12       ` Jiri Kosina
2007-03-28 16:51         ` Michal Piotrowski
2007-03-28 17:56     ` Linus Torvalds
     [not found] ` <20070327230024.GJ16477@stusta.de>
2007-03-27 23:10   ` 2.6.21-rc5: known regressions with patches Rafael J. Wysocki
2007-03-28  0:50   ` Jay Cliburn
2007-03-30 21:32 ` [1/4] 2.6.21-rc5: known regressions (v2) Adrian Bunk
2007-03-30 21:32   ` Adrian Bunk
2007-03-30 21:38   ` Greg KH
2007-03-31  0:23   ` Michal Jaegermann
2007-03-31 15:01     ` Adrian Bunk
2007-03-31 16:42       ` Michal Jaegermann
2007-03-30 21:32 ` [2/4] " Adrian Bunk
2007-03-30 21:32 ` [3/4] " Adrian Bunk
2007-03-30 21:32   ` Adrian Bunk
2007-03-31  2:52   ` Jeff Chua
2007-03-31  2:52     ` Jeff Chua
2007-03-31  2:52     ` Jeff Chua
2007-03-31  3:16     ` Adrian Bunk
2007-03-31 11:08       ` Jens Axboe
2007-04-01  5:39   ` Jeremy Fitzhardinge
2007-04-01  5:39     ` Jeremy Fitzhardinge
2007-04-13 16:32   ` Michal Piotrowski
2007-04-13 16:32     ` Michal Piotrowski
2007-03-30 21:49 ` [4/4] " Adrian Bunk
2007-03-30 21:49   ` Adrian Bunk
2007-03-31  2:41   ` Jeff Chua
2007-03-31  2:41     ` Jeff Chua
2007-03-31  6:44   ` Frédéric Riss
2007-04-01  7:04   ` Michael S. Tsirkin
2007-04-01  7:04     ` Michael S. Tsirkin
2007-04-01 20:37   ` Michael S. Tsirkin
2007-04-01 20:37     ` Michael S. Tsirkin
2007-03-31 18:19 ` 2.6.21-rc5: known regressions with patches (v2) Adrian Bunk
2007-03-31 18:19   ` Adrian Bunk
2007-04-03  4:05   ` [PATCH] libata: add NCQ blacklist entries from Silicon Image Windows driver (v2) Robert Hancock
2007-04-03  4:13     ` Tejun Heo
2007-04-04  6:09     ` Jeff Garzik
2007-04-04 14:26       ` Robert Hancock

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=20070328130630.GA28108@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=bunk@stusta.de \
    --cc=ebiederm@xmission.com \
    --cc=frederic.riss@gmail.com \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcus@better.se \
    --cc=thomas@m3y3r.de \
    --cc=torvalds@linux-foundation.org \
    /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.