On Thu, May 24, 2007 at 01:42:58PM -0600, Eric W. Biederman wrote:
> Andrew Morton <[email protected]> writes:
>
> > On Thu, 24 May 2007 11:07:56 -0500
> > "Mike Miller (OS Dev)" <[email protected]> wrote:
> >
> >> So I guess I found the answer to my own question. msi_free_irqs was apparently
> > added
> >> in 2.6.22-something. I don't find it in 2.6.21.2 or anywhere else. So somebody
> > broke a
> >> couple of things.
> >> The most noticable is cciss hangs after turning on interrupts. The reason for
> > that is
> >> the kernel now looks at my array of MSI-X vectors in reverse order. We have 4
> > ways of
> >> generating an interrupt on Smart Array hw. They are:
> >>
> >> # define DOORBELL_INT 0
> >> # define PERF_MODE_INT 1
> >> # define SIMPLE_MODE_INT 2
> >> # define MEMQ_MODE_INT 3
> >>
> >> For INTx these four lines are OR'ed together and run to one interrupt
> > pin. MSI-X
> >> breaks this hardware OR'ing so we must register either all 4 or at the least
> > the
> >> correct interrupt. When I first submitted the MSI/X support I was registering
> > all 4.
> >> Someone changed that to only register a single MSI-X vector. That worked fine
> > until
> >> 2.6.22-something.
> >> Now it appears that the kernel is looking at the array in reverse order. IOW,
> > I must
> >> register PERF_MODE_INT in order for cciss to work. That's messed up. Anybody
> > want to
> >> `fess up to making these changes? :)
> >> I'll keep working this, but I'm going to undo someones change when I figure
> > out where
> >> it's broke.
> >>
> >
> > I'd guess that you're referring to Michael's changes. If you can identify
> > the offending code in a less vague fashion, more confident answers can
> > be given ;)
> >
> > I canot find any sign of anyone altering the IRQ handling in cciss.c after
> > your initial MSI-support merge. But that's perhaps isn't what you meant.
> > it's all rather foggy. Please either quote file-n-line, or grab a copy
> > of git-blame.
>
> Or perhaps git-bisect to find the offending patch.
>
> I don't recall seeing anything that looked to bad but there was a fair
> amount of change needed to get the last bit of portability into the msi
> code, so it is possible something slipped through.
>
> Possibly someone changed the default enable or disable state?
>
> ....
>
> Which reminds me. Now that we have a reasonable list, we really need
> to reduce pci_enable_msix:
>
> - int pci_enable_msix(struct pci_dev* dev, struct msix_entry *entries, int nvec);
> + int pci_enable_msix(struct pci_dev* dev, int nvec);
>
> And just have drivers that use more the one irq walk the list off of pci_dev
> of all of the msi irqs. I did a little review a while ago and only
> 0-(nvec -1) are allocated and the are always in order in entries so it
> shouldn't be to bad to generate a patch for that case, and not having
> to worry about out of order or holes in the irq allocator would be
> good.
>
> Eric
Found what seems the problem with our vectors being listed backward. In
drivers/pci/msi.c we should be using list_add_tail rather than list_add to preserve
the ordering across various kernels. Please consider this for inclusion.
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 0e67723..d74975d 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -333,7 +333,7 @@ static int msi_capability_init(struct pci_dev *dev)
msi_mask_bits_reg(pos, is_64bit_address(control)),
maskbits);
}
- list_add(&entry->list, &dev->msi_list);
+ list_add_tail(&entry->list, &dev->msi_list);
/* Configure MSI capability structure */
ret = arch_setup_msi_irqs(dev, 1, PCI_CAP_ID_MSI);
@@ -404,7 +404,7 @@ static int msix_capability_init(struct pci_dev *dev,
entry->dev = dev;
entry->mask_base = base;
- list_add(&entry->list, &dev->msi_list);
+ list_add_tail(&entry->list, &dev->msi_list);
}
ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
--------------------------------------------------------------------------------
This patch undoes my dirty little hack.
diff --git a/drivers/block/cciss.h b/drivers/block/cciss.h
index 26b5866..b70988d 100644
--- a/drivers/block/cciss.h
+++ b/drivers/block/cciss.h
@@ -70,8 +70,8 @@ struct ctlr_info
int highest_lun;
int usage_count; /* number of opens all all minor devices */
# define DOORBELL_INT 0
-# define PERF_MODE_INT 2
-# define SIMPLE_MODE_INT 1
+# define PERF_MODE_INT 1
+# define SIMPLE_MODE_INT 2
# define MEMQ_MODE_INT 3
unsigned int intr[4];
unsigned int msix_vector;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
