Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

bge: use BUS_DMA_NOWIAT in functions called from the timeout

2 views
Skip to first unread message

Mike Belopuhov

unread,
May 21, 2013, 11:11:49 AM5/21/13
to te...@openbsd.org, d...@openbsd.org
prevent the system from spitting loads of splasserts when bge_watchdog
fires. ok?


diff --git sys/dev/pci/if_bge.c sys/dev/pci/if_bge.c
index 055ceec..ffad959 100644
--- sys/dev/pci/if_bge.c
+++ sys/dev/pci/if_bge.c
@@ -1226,7 +1226,7 @@ bge_init_rx_ring_std(struct bge_softc *sc)

for (i = 0; i < BGE_STD_RX_RING_CNT; i++) {
if (bus_dmamap_create(sc->bge_dmatag, MCLBYTES, 1, MCLBYTES, 0,
- BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW,
+ BUS_DMA_NOWAIT | BUS_DMA_ALLOCNOW,
&sc->bge_cdata.bge_rx_std_map[i]) != 0) {
printf("%s: unable to create dmamap for slot %d\n",
sc->bge_dev.dv_xname, i);
@@ -1336,7 +1336,7 @@ bge_init_rx_ring_jumbo(struct bge_softc *sc)

for (i = 0; i < BGE_JUMBO_RX_RING_CNT; i++) {
if (bus_dmamap_create(sc->bge_dmatag, BGE_JLEN, 4, BGE_JLEN, 0,
- BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW,
+ BUS_DMA_NOWAIT | BUS_DMA_ALLOCNOW,
&sc->bge_cdata.bge_rx_jumbo_map[i]) != 0) {
printf("%s: unable to create dmamap for slot %d\n",
sc->bge_dev.dv_xname, i);

Mark Kettenis

unread,
May 21, 2013, 11:19:49 AM5/21/13
to mi...@belopuhov.com, te...@openbsd.org, d...@openbsd.org
> Date: Tue, 21 May 2013 17:10:57 +0200
> From: Mike Belopuhov <mi...@belopuhov.com>
>
> prevent the system from spitting loads of splasserts when bge_watchdog
> fires. ok?

I'd say no. Why is the driver tearing down and reinitializing the dma
maps when a watchdog timeout happens? That's just wrong.

Theo de Raadt

unread,
May 21, 2013, 11:24:43 AM5/21/13
to Mark Kettenis, mi...@belopuhov.com, te...@openbsd.org, d...@openbsd.org
> I'd say no. Why is the driver tearing down and reinitializing the dma
> maps when a watchdog timeout happens? That's just wrong.

Because bge_watchdog() simply calls bge_init() to redo everything. It
is simply the way the driver was written.

During autoconfig, NOWAIT will be OK. bge is not a hotplug capable
chipset either, so the non-watchdog bge_init() code path will get the
resources it wants.

The change seems sane enough relative to the alternative of rewriting
everything.

Mike Belopuhov

unread,
May 21, 2013, 11:28:05 AM5/21/13
to Mark Kettenis, tech, d...@openbsd.org
On 21 May 2013 17:18, Mark Kettenis <mark.k...@xs4all.nl> wrote:
>> Date: Tue, 21 May 2013 17:10:57 +0200
>> From: Mike Belopuhov <mi...@belopuhov.com>
>>
>> prevent the system from spitting loads of splasserts when bge_watchdog
>> fires. ok?
>
> I'd say no. Why is the driver tearing down and reinitializing the dma
> maps when a watchdog timeout happens? That's just wrong.
>

well this is what most(?) of our drivers have been doing for ages.
i agree that this is probably not the nicest thing to do, but we're
not going to redesign bge right now, are we?

Mark Kettenis

unread,
May 21, 2013, 12:08:27 PM5/21/13
to der...@cvs.openbsd.org, mi...@belopuhov.com, te...@openbsd.org, d...@openbsd.org
> From: Theo de Raadt <der...@cvs.openbsd.org>
> Date: Tue, 21 May 2013 09:23:04 -0600
Fair enough. Indeed other drivers are doing the same thing. Guess
that if you get a watchdog timeout under memory pressure, you're
simply screwed.

ok kettenis@

David Gwynne

unread,
May 21, 2013, 8:00:00 PM5/21/13
to Mark Kettenis, der...@cvs.openbsd.org, mi...@belopuhov.com, te...@openbsd.org
sad but ok dlg@

Theo de Raadt

unread,
May 21, 2013, 8:59:53 PM5/21/13
to David Gwynne, Mark Kettenis, mi...@belopuhov.com, te...@openbsd.org
One added note. If someone out on the lists wants to carefylly
de-abstract the stuff inside bge_init() nicely....

0 new messages