Broadcom bge(4) panics while shutting down
Alexander Sack
pisymbol at gmail.com
Thu May 14 23:03:23 UTC 2009
On Thu, May 14, 2009 at 6:52 PM, Xin LI <delphij at delphij.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi, Alexander,
>
> Alexander Sack wrote:
> [...]
>>> @@ -3193,6 +3193,9 @@ bge_rxeof(struct bge_softc *sc)
>>> BGE_UNLOCK(sc);
>>> (*ifp->if_input)(ifp, m);
>>> BGE_LOCK(sc);
>>> +
>>> + if (!(ifp->if_drv_flags & IFF_DRV_RUNNING))
>>> + return;
>>> }
>>
>> Xin this looks fine by me, I actually put this up in the while loop as
>> I mentioned before which I think is functionality equivalent (can you
>> gain some optimizations by putting in the while loop though compiler
>> wise than a separate compilation unit?).
>
> I think the two is not semantically the same... For this case an
> explicit 'return' would mean that no further actions, say the things
> right after the while loop, would be taken. In my opinion that this is
> better since there is no protection over these DMA maps (which could
> have been released elsewhere).
Oh snap, I had a brain fart. Ugh. You are correct Xin. Oh man, I
even said in my first post I want to AVOID touching the DMA maps
and/or hardware registers (notice the access in the jumbo case) as
soon as IFF_DRV_RUNNING gets reset. I like the explicit return much
better that's why I had it that way but as John pointed out, tis
better to do it where you put it which is right after reacquiring the
lock.
> Note that I'm still not quite confident about the logic, we might just
> narrowed but not closed the race completely - these
> BGE_LOCK()/UNLOCK()'s could hit some problem if one thread has done
> bge_stop() very quickly. That would require more work, though, I don't
> have a very good plan at this moment...
This has been a constant problem that I've seen in the labs. I've
seen this with polling as well as tuning the number of RING
descriptors (there is a thread I had a while back ago about this when
I was dealing with BPF drops and bge(4)). I think the patch as it
stands though will suffice for the majority of cases and again I thank
you and John for addressing it in short-order.
-aps
More information about the freebsd-current
mailing list