On Monday 13 August 2007 21:54:37 Jeremy Fitzhardinge wrote:
> xennet_tx_bug_gc can free the skb before we use it, so make sure we don't.
>
> Jeff, this is -rc material.
>
> Signed-off-by: Keir Fraser <[EMAIL PROTECTED]>
> Signed-off-by: Jeremy Fitzhardinge <[EMAIL PROTECTED]>
> Cc: Jeff Garzik <[EMAIL PROTECTED]>
>
> diff -r 8bfc43f6d1b0 drivers/net/xen-netfront.c
> --- a/drivers/net/xen-netfront.c Tue Aug 07 14:26:30 2007 -0700
> +++ b/drivers/net/xen-netfront.c Mon Aug 13 09:39:15 2007 -0700
> @@ -566,15 +566,16 @@ static int xennet_start_xmit(struct sk_b
> if (notify)
> notify_remote_via_irq(np->netdev->irq);
>
> + np->stats.tx_bytes += skb->len;
> + np->stats.tx_packets++;
> +
> + /* Note: It is not safe to access skb after xennet_tx_buf_gc()! */
> xennet_tx_buf_gc(dev);
>
> if (!netfront_tx_slot_available(np))
> netif_stop_queue(dev);
>
> spin_unlock_irq(&np->tx_lock);
> -
> - np->stats.tx_bytes += skb->len;
> - np->stats.tx_packets++;
>
> return 0;
>
This moves the updating of both tx_bytes and tx_packets inside the
spinlock, but as far as I can see we only _really_ need to move the
tx_bytes update. Considering that we generally want to do as little
work as possible while holding a lock, wouldn't the following be
slightly better?
Signed-off-by: Keir Fraser <[EMAIL PROTECTED]>
Signed-off-by: Jeremy Fitzhardinge <[EMAIL PROTECTED]>
Cc: Jeff Garzik <[EMAIL PROTECTED]>
Signed-off-by: Jesper Juhl <[EMAIL PROTECTED]>
---
drivers/net/xen-netfront.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 489f69c..640e270 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -566,6 +566,9 @@ static int xennet_start_xmit(struct sk_buff *skb, struct
net_device *dev)
if (notify)
notify_remote_via_irq(np->netdev->irq);
+ np->stats.tx_bytes += skb->len;
+
+ /* Note: It is not safe to access skb after xennet_tx_buf_gc()! */
xennet_tx_buf_gc(dev);
if (!netfront_tx_slot_available(np))
@@ -573,7 +576,6 @@ static int xennet_start_xmit(struct sk_buff *skb, struct
net_device *dev)
spin_unlock_irq(&np->tx_lock);
- np->stats.tx_bytes += skb->len;
np->stats.tx_packets++;
return 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
