Re: [PATCH 1/11] mpt2sas: mpt2sas_base sources

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Wed, 2009-02-25 at 14:12 -0700, Matthew Wilcox wrote:
> On Wed, Feb 25, 2009 at 01:27:00PM -0700, Moore, Eric wrote:
> > > > +static void
> > > > +_base_mask_interrupts(struct MPT2SAS_ADAPTER *ioc)
> > > > +{
> > > > + ? ? ? u32 him_register;
> > > > +
> > > > + ? ? ? ioc->mask_interrupts = 1;
> > > > + ? ? ? him_register = readl(&ioc->chip->HostInterruptMask);
> > > > + ? ? ? him_register |= MPI2_HIM_DIM + MPI2_HIM_RIM + 
> > > MPI2_HIM_RESET_IRQ_MASK;
> > > > + ? ? ? writel(him_register, &ioc->chip->HostInterruptMask);
> > > 
> > > This is a posted write. Does it need to be flushed?
> > > ie do other parts of the driver require this take effect immediately
> > > or will they tolerate some late arriving interrupt?
> > 
> > What are you suggesting?   Calling wmb()?
> 
> PCI writes can be posted, so the him_register writel() may not complete
> for a few thousand cycles.  wmb() isn't going to solve the problem.  A
> readl() from the device would, as would your current solution of
> ignoring subsequent interrupts.

This seems to be the most serious issue with the current patch set.  If
you repost with that fixed, I can put it into scsi-misc and we can
address the other issues later.

James