[openib-general] [PATCH] IB/ipoib: NAPI

Roland Dreier rdreier at cisco.com
Thu Sep 21 16:00:32 PDT 2006


 > > So probably what we need is a feature bit in the struct ib_device
 > > to say whether the peek CQ is needed or whether req notify will
 > > generate events for existing CQEs.

 > Sounds good to me

The biggest problem I have with this is that I don't know what to call
the feature bit.  Any suggestions?

 > > Also, the handling of how many completions to poll and the logic of
 > > when to call netif_rx_complete() seems very strange to me.  First, you
 > > totally ignore the budget parameter,

 > Not totally. I update it whenever I decide it is a "not_done" which
 > happens in two cases:
 > a. I finish my quota
 > b. I handled any completions - rx or tx. I do count tx as well since I
 > want to  coalesce as many completions as possible.

Right, but as far as I can see you don't handle the case where *budget
is less than dev->quota -- you only update *budget, you never look at
the original value passed through it.

 > > +		if (n != t) {
 > > +			empty = 1;
 > > +			break;

 > I don't think this is the right thing to do. Polling less completions then
 > you could is no reason to quit polling mode. You may receive more
 > completions in the next time you got called.

I misread your code slightly the first time through, so I don't think
it's actually wrong now.  But I am pretty confident that my code is
"more correct": if we ask for n CQEs and only poll t < n of them, then
we know we have drained the CQ without exhausting our quota of work.
So we should switch back to event-driven mode at that point.

I don't think a correct NAPI implementation would drain the CQ and
then schedule another poll on an empty CQ.

 - R.




More information about the general mailing list