LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Bernhard Walle <bwalle@suse.de>
To: Michael Chan <mchan@broadcom.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Fix crash in tg3 when using irqpoll
Date: Fri, 23 Mar 2007 00:20:48 +0100	[thread overview]
Message-ID: <20070322232048.GC3333@sykes.suse.de> (raw)
In-Reply-To: <1174601067.9753.42.camel@dell>

Hello Michael,

* Michael Chan <mchan@broadcom.com> [2007-03-22 23:04]:
> On Thu, 2007-03-22 at 21:46 +0100, Bernhard Walle wrote:
> 
> > This patch makes sure that even the tr32() instruction in the interrupt handler
> > is not executed which accesses PCI memory. Accessing PCI memory when
> > pci_restore_state() is called is a bad idea because that function modifies
> > the BARs of the PCI device.
> 
> It is not caused by the BAR as it doesn't get changed in this case.  The
> pci_restore_state() call is to restore the memory enable bit in the PCI
> command register.  The tr32() call in tg3_interrupt() will cause a
> master abort if it is called before the memory enable bit has been
> restored.

Ok, thanks for the explanation. I wondered why you call
pci_restore_state() here, normally that's only called from .resume
handlers.

> > --- mainline-msi-init.orig/drivers/net/tg3.c
> > +++ mainline-msi-init/drivers/net/tg3.c
> > @@ -3561,7 +3561,10 @@ static irqreturn_t tg3_interrupt(int irq
> >  	struct net_device *dev = dev_id;
> >  	struct tg3 *tp = netdev_priv(dev);
> >  	struct tg3_hw_status *sblk = tp->hw_status;
> > -	unsigned int handled = 1;
> > +	unsigned int handled = 0;
> > +
> > +	if (tg3_irq_sync(tp))
> > +		goto out;
> 
> 
> This will break other things.  When we disable interrupts, we set the
> irq_sync flag but allow one more interrupt to be generated.  The irq
> handler will simply mask off the interrupt when it sees the irq_sync
> flag.  With the above change, the irq handler can no longer mask off
> this trailing interrupt and you may get screaming interrupts as a
> result.

You're right, I had only the case in mind where the network device
doesn't generate any interrupts (in initialisation phase and in
shutdown phase) because it's disabled in the device, and the interrupt
handler is only called because of IRQ-sharing/irqpoll.

> Thanks for reporting this.  I'll try to come up with a good solution.

Could you please CC me, I'd like to test it here.


Thanks,
   Bernhard

      reply	other threads:[~2007-03-22 23:20 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-22 20:46 Bernhard Walle
2007-03-22 22:04 ` Michael Chan
2007-03-22 23:20   ` Bernhard Walle [this message]

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=20070322232048.GC3333@sykes.suse.de \
    --to=bwalle@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [PATCH] Fix crash in tg3 when using irqpoll' \
    /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).