Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 09 Jun 2008 08:03:40 -0400
From:      Coleman Kane <cokane@FreeBSD.org>
To:        "Paul B. Mahol" <onemda@gmail.com>
Cc:        freebsd-current@freebsd.org
Subject:   Re: Call for Testers: Convert watchdog spinlock into a sleepable mutex in if_ndis
Message-ID:  <1213013020.25641.3.camel@localhost>
In-Reply-To: <3a142e750806071855wb9f4cddk9be25e8c0f3d4984@mail.gmail.com>
References:  <1212870158.1724.25.camel@localhost> <3a142e750806071855wb9f4cddk9be25e8c0f3d4984@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--=-jcs2GeBh6xPh503SSM/J
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

On Sun, 2008-06-08 at 03:55 +0200, Paul B. Mahol wrote:
> On 6/7/08, Coleman Kane <cokane@freebsd.org> wrote:
> > Hi,
> >
> > I've got another patch to if_ndis and I'd like to know if any NDIS user=
s
> > out there can test it for me and give me some feedback. I've converted
> > the ndis_spinlock in if_ndis into a normal sleepable mutex, and would
> > like to get some testers. The idea here is to eliminate an unnecessary
> > "spinning" in the kernel, where it would be preferable to sleep (for CP=
U
> > usage, interactivity, etc...).
> >
> > You can download it here:
> >       *
> > http://people.freebsd.org/~cokane/patches/if_ndis-spinlock-to-mtx.patch
> >
> > --
> > Coleman Kane
> >
>=20
> I did not found any stability regressions in my environment with linked p=
atch.
>=20
> But I noticed   new   LOR (it appears as soon as radio of device is turne=
d on):
>=20
> lock order reversal:
>  1st 0xc401726c ndis0 (network driver) @
> /usr/local/src/sys/modules/if_ndis/../../dev/if_ndis/if_ndis.c:1648
>  2nd 0xc09b3e74 HAL preemption lock (HAL lock) @
> /usr/local/src/sys/modules/ndis/../../compat/ndis/subr_hal.c:423
> KDB: stack backtrace:
> db_trace_self_wrapper(c0708534,c3985bcc,c0541a6e,c070adf6,c09b3e74,...)
> at db_trace_self_wrapper+0x26
> kdb_backtrace(c070adf6,c09b3e74,c09b0a51,c09b0a48,c09b09f8,...) at
> kdb_backtrace+0x29
> witness_checkorder(c09b3e74,9,c09b09f8,1a7,c04f7364,...) at
> witness_checkorder+0x6de
> _mtx_lock_flags(c09b3e74,0,c09b09f8,1a7,c457ebd0,...) at _mtx_lock_flags+=
0xbc
> KfRaiseIrql(2,c457ebd0,c3985c48,c09aacdd,c3d45354,...) at KfRaiseIrql+0x6=
3
> KfAcquireSpinLock(c3d45354,c094888a,c4017200,c3d6cc40,c4017200,...) at
> KfAcquireSpinLock+0x13
> IoQueueWorkItem(c457ebd0,c3d6cc40,0,c4017200,c07040ef,...) at
> IoQueueWorkItem+0x2d
> ndis_tick(c4017200,0,c070699c,166,c077e594,...) at ndis_tick+0x143
> softclock(c077e560,0,c0701ca6,4d4,c3cda6ec,...) at softclock+0x24a
> ithread_loop(c3cdb2e0,c3985d38,c0701a09,324,c3c99a70,...) at ithread_loop=
+0x1b5
> fork_exit(c04e74f0,c3cdb2e0,c3985d38) at fork_exit+0xb8
> fork_trampoline() at fork_trampoline+0x8
> --- trap 0, eip =3D 0, esp =3D 0xc3985d70, ebp =3D 0 ---

Try this one:
      * http://people.freebsd.org/~cokane/patches/if_ndis-spinlock-to-mtx2.=
patch

There's the same LOR in the existing code as well, but it is hidden by
WITNESS_SKIPSPIN.

If you can verify that this works, I'll just commit them both as a
single update.

--=20
Coleman Kane

--=-jcs2GeBh6xPh503SSM/J
Content-Type: application/pgp-signature; name=signature.asc
Content-Description: This is a digitally signed message part

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (FreeBSD)

iEUEABECAAYFAkhNHBYACgkQcMSxQcXat5cjPwCWKsEz6tpJtx++eVnIKrNfl94h
kACeLZAuB089GH5xchRzCtH1rS0CjMA=
=ciV0
-----END PGP SIGNATURE-----

--=-jcs2GeBh6xPh503SSM/J--




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1213013020.25641.3.camel>