From: Stephen Irons [mailto:[email protected]]
Sent: Thursday, April 23, 2009 3:09 AM
>
> Nori, Sekhar wrote:
[...]
> >
> > I think just checking for (num_rxpkts < budget) should be sufficient
> instead of !rxpending && (num_rxpkts != budget). This is what is being
> done in other drivers using NAPI (like 8139too or e100).
> >
> > Thanks,
> > Sekhar
>
> I agree that (num_rxpkts < budget) is enough to make the system work.
> However, I feel that the complete condition explains exactly WHY we
> are
> doing things. In fact, I would prefer to write it:
>
> if (!txpending && !rxpending) /* There is no more work to do, so we
> can stop polling... */
> {
> if (num_rxpkts != budget) /* ...except in the special case that we
> have use the entire budget (see comment net_rx_action() in file
> net/core/dev.c). We will stop polling next time around unless, of
> course, we have suddenly been given more work to do. */
>
> {
> netif_rx_complete(napi);
> emac_int_enable(priv);
> }
> }
>
> It depends whether we want the code to be explicit or 'efficient'. If
> we
> use the minimum condition, we have code that reads
>
> if (!txpending && (num_rxpkts < budget))
> {
> netif_rx_complete(napi);
> emac_int_enable(priv);
> }
>
> and we have to explain why we are not using rxpending: the tx_bdproc()
> and rx_bdproc() function appear symmetrical, so I would expect to see
> identical code using the results from those functions.
I think we can get rid of both txpending and rxpending and keep the
functions symmetrical.
We can take an approach similar to that taken by gainfar.c for overloading Tx
processing on NAPI.
Lifting ideas from that driver, the poll function becomes:
---
tx_pkts = emac_tx_bdproc(...);
rx_pkts = emac_rx_bdproc(...);
if(tx_pkts)
return budget;
if(rx_pkts < budget) {
napi_complete(napi);
emac_int_enable();
}
return rx_pkts;
---
> And we still
> have
> to explain about not playing around with the NAPI state in the special
> case.
>
> My feeling is that the requirement to leave the NAPI state alone in
> one
> special case is artificial. It should be made explicit right where the
> problem is. That condition might go away in the future.
This actually argues for not keeping code explicitly taking care of this
condition in the driver. It will be tough to keep track of when this
condition is relaxed in NAPI code. Driver code typically changes only when
something broken is discovered.
>
> Sure, we all understand the issue now, but in 6 months time, someone
> else will come along and ask why we are not using rxpending. I don't
> want to drag this whole thing up again...
They wont ask if rxpending itself is gone (along with txpending) :)
>
> Stephen Irons
Thanks,
Sekhar
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
