On Tue, 2006-01-24 at 23:09 +0100, Stefan Rompf wrote:
> This interface is totally broken. Actually this happens in ipw2200 since
> version 1.0.8:
>
> -dev->hard_start_xmit() calls ieee80211_xmit()
> -ieee80211_xmit() calls into ieee->is_queue_full() which is
> ipw_net_is_queue_full() for the ipw2200 driver. If this function indicates
> that the queue is full, ieee80211_xmit() returns NETDEV_TX_BUSY.
> -On this return code, qdisc_restart() requeues the packet at the tail of leaf
> qdisc and immediatly reschedules output via netif_schedule(dev), effectivly
> busy polling for the queue becoming empty.
>
> This is what leads to the high ksoftirqd usage I reported October 2005 into
> the ipw bugzilla (http://www.bughost.org/bugzilla/show_bug.cgi?id=825).
Sorry, I'm not aware of this bug since I'm not on the cc list.
> Please consider the workaround I posted there.
> --- ipw2200.c.orig 2005-10-20 23:35:24.000000000 +0200
> +++ ipw2200.c 2005-10-29 18:03:30.000000000 +0200
> @@ -3564,9 +3564,7 @@ static void ipw_queue_init(struct ipw_pr
> if (q->low_mark < 4)
> q->low_mark = 4;
>
> - q->high_mark = q->n_bd / 8;
> - if (q->high_mark < 2)
> - q->high_mark = 2;
> + q->high_mark = 2;
I believe this is for your own testing. Right?
>
> q->first_empty = q->last_used = 0;
> q->reg_r = read;
> @@ -10412,8 +10410,10 @@ static int ipw_net_is_queue_full(struct
> struct clx2_tx_queue *txq = &priv->txq[0];
> #endif /* CONFIG_IPW_QOS */
>
> - if (ipw_queue_space(&txq->q) < txq->q.high_mark)
> + if (ipw_queue_space(&txq->q) < txq->q.high_mark) {
> + if (!netif_queue_stopped(dev)) netif_stop_queue(dev);
> return 1;
> + }
The function is_queue_full() returns whether the queue is full or not, it
should not
make any decision to stop the queue. IMHO, The decision should be made from the
network
schedule layer. For example, if a device continually returns NETDEV_TX_BUSY
exceeds
a certain rate, the netdev scheduler should put it to "sleep" instead of busy
polling.
BTW, where did you restart the queue after it is empty again?
>
> return 0;
> }
Your comment is valid and the high cpu usage of ksoftirqd should be fixed. But
this
patch should still be merged since it fixed a different problem.
No?
Thanks,
-yi
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
