From: Grant Grundler <grundler@parisc-linux.org>
To: Jeff Garzik <jgarzik@pobox.com>
Cc: Grant Grundler <grundler@parisc-linux.org>,
Valerie Henson <val_henson@linux.intel.com>,
Andrew Morton <akpm@osdl.org>,
netdev@vger.kernel.org
Subject: Re: PATCHv3 2.6.17-rc5 tulip free_irq() called too late
Date: Wed, 14 Jun 2006 12:14:19 -0600 [thread overview]
Message-ID: <20060614181419.GA10365@colo.lackof.org> (raw)
In-Reply-To: <44902554.7010703@pobox.com>
On Wed, Jun 14, 2006 at 11:03:48AM -0400, Jeff Garzik wrote:
> Grant Grundler wrote:
> >Switching the order to be:
> > tulip_stop_rxtx(tp); /* Stop DMA */
> > free_irq (dev->irq, dev); /* no more races after this */
> >
> >still leaves us open to IRQs being delivered _after_ we've stopped DMA.
>
> Correct. And that is the preferred, natural, logical, obvious order:
>
> 1) Turn things off.
> 2) Wait for activity to cease.
Patch v3 does this in two stages:
1) turn off tulip interrupts
2) free_irq() calls syncronize_irq() to handle pending IRQs
then calls tulip_stop_rxtx() which:
1) tells tulip to stop DMA
2) poll until DMA completes
After this we can free remaining resources.
> >That in turn allows the interrupt handler to re-enable DMA again.
>
> Then that would be a problem to solve... Some interrupt handlers will
> test netif_running() or a driver-specific shutting-down flag,
> specifically to avoid such behaviors.
I'm not keen on adding more code to tulip_interrupt() routine
for something that rarely happens (compared to IRQs) and is handled
outside the interrupt routine. I'm pretty sure stopping interrupts
before stopping DMA is sufficient.
Can you show an example where it doesn't work?
This is important since I'm going to propose a new Documentation/pci.txt
based on this experience.
thanks,
grant
next prev parent reply other threads:[~2006-06-14 18:14 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-05-31 19:52 PATCH 2.6.17-rc5 tulip free_irq() called too late Grant Grundler
2006-06-08 14:43 ` Jeff Garzik
2006-06-08 15:22 ` Grant Grundler
2006-06-08 15:32 ` Grant Grundler
2006-06-08 15:38 ` Jeff Garzik
2006-06-08 15:47 ` Grant Grundler
2006-06-08 15:32 ` Jeff Garzik
2006-06-08 15:36 ` Grant Grundler
2006-06-08 17:01 ` Grant Grundler
2006-06-13 23:55 ` PATCHv3 " Grant Grundler
2006-06-14 0:06 ` Valerie Henson
2006-06-14 0:33 ` Jeff Garzik
2006-06-14 4:44 ` Grant Grundler
2006-06-14 13:05 ` Kyle McMartin
2006-06-14 14:54 ` Grant Grundler
2006-06-14 15:03 ` Jeff Garzik
2006-06-14 18:14 ` Grant Grundler [this message]
2006-06-14 19:51 ` Jeff Garzik
2006-06-14 22:25 ` Grant Grundler
2006-06-14 20:47 ` Francois Romieu
2006-06-14 22:30 ` Grant Grundler
2006-06-15 20:30 ` Francois Romieu
2006-06-16 5:47 ` Grant Grundler
2006-06-16 7:32 ` Jeff Garzik
2006-06-16 15:25 ` Grant Grundler
[not found] ` <20060616152400.GA7868@colo.lackof.org>
[not found] ` <4492CE98.50900@pobox.com>
2006-06-16 16:06 ` Grant Grundler
2006-06-16 16:16 ` Jeff Garzik
2006-06-22 0:43 ` Valerie Henson
2006-06-23 5:00 ` Grant Grundler
2006-06-26 22:31 ` [PATCH] Fix tulip shutdown DMA/irq race Valerie Henson
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=20060614181419.GA10365@colo.lackof.org \
--to=grundler@parisc-linux.org \
--cc=akpm@osdl.org \
--cc=jgarzik@pobox.com \
--cc=netdev@vger.kernel.org \
--cc=val_henson@linux.intel.com \
/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 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).