On Tue, 2007-09-04 at 02:10 -0700, Eric Cheng wrote:
> On Fri, Aug 31, 2007 at 01:47:28PM -0700, Garrett D'Amore wrote:
> >
> > The webrev is at http://cr.opensolaris.org/~gdamore/mac-notify/
> >
>
> a few minor comments:
>
> 1428: who is going to wake up this waiter here? your
> i_mac_notify_thread doesn't seem to be doing cv_broadcast()
> until it exits and i_mac_notify() callers can't enter
> because mi_disabled is B_TRUE.
You're right!
I'm thinking of removing lines 1428-1434. Its already been suggested by
several people. The only problem then is that there could be pending
mac_notify operations when dls_destroy gets called. I think this isn't
an issue, though.
>
> 254: I think a mutex_exit() is needed here.
No, CALLB_CPR_EXIT() does that.
>
> I think you could use mi_lock instead of adding mi_notify_bits_lock.
> mi_lock was added recently and it is meant to be used in
> non-performance critical paths (which yours is).
Possibly. But note that mac_tx_update() can become *quite* hot. The
reason for this is that if you have a NIC with a small descriptor rings
(say dmfe, which has 32 tx descriptors), and you have high traffic, then
mac_tx_update is going to get called *alot*. (E.g. maybe 10's of
thousands of times per second!) Even devices like hme, with 256 tx
rings, wind up doing this up to 1,000 times per second when pushing over
200K packets per second.
-- Garrett
>
> eric
>
> _______________________________________________
> networking-discuss mailing list
> [email protected]
_______________________________________________
networking-discuss mailing list
[email protected]
