On Sun, 4 May 2008 19:04:22 +0200
Matteo Croce <firstname.lastname@example.org> wrote:
> This patch fixes an IRQ storm, a locking issues, moves platform code in the
> right sections
> and other small fixes.
Please feed this patch (and all future ones) through scripts/checkpatch.pl.
It picks up rather a lot of simple problems which there is no reason for
us to retain.
> + spin_unlock(&priv->rx_lock);
> + netif_rx_complete(priv->dev, napi);
> + netif_stop_queue(priv->dev);
> + napi_disable(&priv->napi);
> + atomic_inc(&priv->reset_pending);
> + cpmac_hw_stop(priv->dev);
> + if (!schedule_work(&priv->reset_work))
> + atomic_dec(&priv->reset_pending);
> + return 0;
> static int cpmac_start_xmit(struct sk_buff *skb, struct net_device *dev)
> @@ -456,6 +549,9 @@ static int cpmac_start_xmit(struct sk_buff *skb, struct
> net_device *dev)
> struct cpmac_desc *desc;
> struct cpmac_priv *priv = netdev_priv(dev);
> + if (unlikely(atomic_read(&priv->reset_pending)))
> + return NETDEV_TX_BUSY;
This looks a bit strange. schedule_work() will return zero if the work was
already scheduled, in which case we arrange for cpmac_start_xmit() to abort
But if schedule_work() *doesn't* return zero, there is a time window in
which the reset is still pending. Because it takes time for keventd to be
awoken and to run the work function.
I would have thought that we would want to prevent cpmac_start_xmit() from
running within that time window also?
But that's just a guess - the text which you used to describe your work is
missing much information, so I don't have a lot to work with here.