LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Auke Kok <auke-jan.h.kok@intel.com>
Cc: linux-pci maillist <linux-pci@atrey.karlin.mff.cuni.cz>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	ebiederm@xmission.com, Andrew Morton <akpm@osdl.org>,
	Linus Torvalds <torvalds@osdl.org>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>
Subject: Re: Possible regression: MSI vector leakage since 2.6.18-rc5ish (Unable to repeatedly allocate/free MSI interrupt)
Date: Sat, 27 Jan 2007 02:28:58 -0700	[thread overview]
Message-ID: <m1bqkku8bp.fsf@ebiederm.dsl.xmission.com> (raw)
In-Reply-To: <45BA82DA.6050201@intel.com> (Auke Kok's message of "Fri, 26 Jan 2007 14:38:18 -0800")

Auke Kok <auke-jan.h.kok@intel.com> writes:

> Hi,
>
> I've established a regression in the MSI vector/irq allocation routine for both
> i386 and x86_64. Our test labs repeatedly modprobe/rmmod the e1000 driver for
> serveral minutes which allocates msi vectors and frees them. These tests have
> been running fine until 2.6.19.
>
> git-bisecting I've established that in between commit
> 04b9267b15206fc902a18de1f78de6c82ca47716 "Eric W. Biederman -- genirq: x86_64
> irq: Remove the msi assumption that irq == vector" and commit
> f29bd1ba68c8c6a0f50bd678bbd5a26674018f7c "Ingo Molnar -- genirq: convert the
> x86_64 architecture to irq-chips" the behaviour broke.
>
> The revisions in between seem to be dependent and give all sorts of other
> issues, so it's rather hard for me to bisect that and give trustworthy results.
>
> the e1000 driver hits the 256-mark cycle (I think - it consistently refuses to
> do 500 msi irq/vector allocations which is my test case) and throws:
>
> e1000: eth4: e1000_request_irq: Unable to allocate MSI interrupt Error: -16
>
> which is caused by a `if ((err = pci_enable_msi(adapter->pdev))) {` call from
> the e1000 driver. It's rather easy to hit this mark with the new 4-port e1000
> adapters :).
>
> as for the e1000 code, I can say that even our oldest msi-enabled e1000 driver
> works fine with 2.6.18 and under. All kernels from 2.6.19 fail consistently.
>
> I mostly suspect commit 7bd007e480672c99d8656c7b7b12ef0549432c37 at the
> moment. Perhaps Eric Biederman can help?
>
> Cheers,
>
> Auke
>

Does this patch fix it for you?  It looks like i386 vector allocate
did not have logic to look through the set of vectors more than once.

The code in this patch is a simplified version of what we have
on x86_64.

Eric


diff --git a/arch/i386/kernel/io_apic.c b/arch/i386/kernel/io_apic.c
index 2424cc9..6a3875f 100644
--- a/arch/i386/kernel/io_apic.c
+++ b/arch/i386/kernel/io_apic.c
@@ -1227,26 +1227,32 @@ static u8 irq_vector[NR_IRQ_VECTORS] __read_mostly = { FIRST_DEVICE_VECTOR , 0 }
 
 static int __assign_irq_vector(int irq)
 {
-	static int current_vector = FIRST_DEVICE_VECTOR, offset = 0;
-	int vector;
+	static int current_vector = FIRST_DEVICE_VECTOR, current_offset = 0;
+	int vector, offset, i;
 
 	BUG_ON((unsigned)irq >= NR_IRQ_VECTORS);
 
 	if (irq_vector[irq] > 0)
 		return irq_vector[irq];
 
-	current_vector += 8;
-	if (current_vector == SYSCALL_VECTOR)
-		current_vector += 8;
-
-	if (current_vector >= FIRST_SYSTEM_VECTOR) {
-		offset++;
-		if (!(offset % 8))
-			return -ENOSPC;
-		current_vector = FIRST_DEVICE_VECTOR + offset;
-	}
-
 	vector = current_vector;
+	offset = current_offset;
+next:
+	vector += 8;
+	if (vector >= FIRST_SYSTEM_VECTOR) {
+		offset = (offset + 1) % 8;
+		vector = FIRST_DEVICE_VECTOR + offset;
+	}
+	if (vector == current_vector)
+		return -ENOSPC;
+	if (vector == SYSCALL_VECTOR)
+		goto next;
+	for (i = 0; i < NR_IRQ_VECTORS; i++)
+		if (irq_vector[i] == vector)
+			goto next;
+
+	current_vector = vector;
+	current_offset = offset;
 	irq_vector[irq] = vector;
 
 	return vector;

  parent reply	other threads:[~2007-01-27  9:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-26 22:38 Auke Kok
2007-01-26 22:50 ` Auke Kok
2007-01-27  9:28 ` Eric W. Biederman [this message]
2007-01-27 19:07   ` Auke Kok
2007-01-27 19:28     ` Eric W. Biederman
2007-01-27 19:39       ` Auke Kok
2007-01-29 19:00   ` Auke Kok
2007-01-29 19:36     ` Linus Torvalds
2007-01-29 19:42       ` Eric W. Biederman
2007-01-29 20:19       ` [PATCH] i386: In assign_irq_vector look at all vectors before giving up Eric W. Biederman

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=m1bqkku8bp.fsf@ebiederm.dsl.xmission.com \
    --to=ebiederm@xmission.com \
    --cc=akpm@osdl.org \
    --cc=auke-jan.h.kok@intel.com \
    --cc=jesse.brandeburg@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@atrey.karlin.mff.cuni.cz \
    --cc=torvalds@osdl.org \
    --subject='Re: Possible regression: MSI vector leakage since 2.6.18-rc5ish (Unable to repeatedly allocate/free MSI interrupt)' \
    /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

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).