virtio: change comment in transmit
Subsystems (4)
PATCH 1/1 virtio: change comment in transmit
The original comment was not really informative or funny as well as sexist. Replace it with a better explanation of why the driver does stop and what the impacts are.
drivers/net/virtio_net.c +6 -2
@@ -939,8 +939,12 @@ static netdev_tx_t start_xmit(struct sk_skb_orphan (skb);nf_reset (skb); -/* Apparently nice girls don't return TX_BUSY; stop the queue -* before it gets out of hand. Naturally, this wastes entries. */ +/* It is better to stop queue if running out of space +* instead of forcing queuing layer to requeue the skb +* by returning TX_BUSY (and cause a BUG message). +* Since most packets only take 1 or 2 ring slots +* this means 16 slots are typically wasted. +*/ if (sq->vq->num_free <2 +MAX_SKB_FRAGS ) {netif_stop_subqueue (dev, qnum);if (unlikely (!virtqueue_enable_cb_delayed (sq->vq))) {
From: Stephen Hemminger <[email protected]>
Date: Tue, 24 Mar 2015 16:22:07 -0700
Quoted 5 lines
The original comment was not really informative or funny as well as sexist. Replace it with a better explanation of why the driver does stop and what the impacts are.
Signed-off-by Stephen Hemminger <[email protected]>
Applied, but _only_ because it adds clarity.
I absolutely do not believe in this for the sake of making people easily made upset feel better, that's censorship and it's crap.
From: Stephen Hemminger <[email protected]>
Date: Tue, 24 Mar 2015 16:22:07 -0700
Quoted 5 lines
The original comment was not really informative or funny as well as sexist. Replace it with a better explanation of why the driver does stop and what the impacts are.
Signed-off-by Stephen Hemminger <[email protected]>
Applied, but _only_ because it adds clarity.
I absolutely do not believe in this for the sake of making people easily made upset feel better, that's censorship and it's crap.
Quoted 5 lines · Stephen Hemminger
The original comment was not really informative or funny as well as sexist. Replace it with a better explanation of why the driver does stop and what the impacts are.
Signed-off-by Stephen Hemminger <[email protected]>
Fair call. Comment was certainly snarky, probably sexist. I think it expressed my feelings perfectly, however.
I note that there's still no comment saying "don't do this" in netdevice.h; I gather returning NETDEV_TX_BUSY is still considered a Bad Thing?
(Does it really BUG_ON?)
Thanks,
Rusty.
Quoted 21 lines
drivers/net/virtio_net.c +6
-3@@ -939,8 +939,12 @@ static netdev_tx_t start_xmit(struct sk_skb_orphan (skb);nf_reset (skb); -/* Apparently nice girls don't return TX_BUSY; stop the queue -* before it gets out of hand. Naturally, this wastes entries. */ +/* It is better to stop queue if running out of space +* instead of forcing queuing layer to requeue the skb +* by returning TX_BUSY (and cause a BUG message). +* Since most packets only take 1 or 2 ring slots +* this means 16 slots are typically wasted. +*/ if (sq->vq->num_free <2 +MAX_SKB_FRAGS ) {netif_stop_subqueue (dev, qnum);if (unlikely (!virtqueue_enable_cb_delayed (sq->vq))) { --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
David Miller
maintainerFrom: Rusty Russell <[email protected]>
Date: Wed, 25 Mar 2015 14:29:38 +1030
Quoted 3 lines
I note that there's still no comment saying "don't do this" in
netdevice.h; I gather returningNETDEV_TX_BUSYis still considered a Bad Thing?
You're not supposed to do it still, that's right.
If the driver returns NETDEV_TX_BUSY it means that it is not maintaining the TX queues stop/start state properly.
It was kept around for drivers that we didn't want to convert or fix up at the time (this is circa two decades ago).
I realize that there are situations for some devices where this is difficult to achieve, but the stack really does a non-trivial amount of useless work if you don't play along nicely.
If you grep drivers/net for it, the majority of the returns are in assertion failure paths in the TX handler of those drivers. NETDEV_TX_BUSY is not being returned in a normal path of operation.
Quoted 1 line
(Does it really
BUG_ON?)
No, it doesn't.