* [PATCH v2] rt2x00mac: In error case stop netdev queue, free skb and return NETDEV_TX_OK
@ 2008-07-23 17:17 Daniel Wagner
2008-07-23 18:05 ` Ivo van Doorn
0 siblings, 1 reply; 2+ messages in thread
From: Daniel Wagner @ 2008-07-23 17:17 UTC (permalink / raw)
To: linux-wireless; +Cc: Ivo van Doorn, Daniel Wagner
It is not allowed to use NETDEV_TX_BUSY in tx path anymore.
Signed-off-by: Daniel Wagner <wagi@monom.org>
Cc: Ivo van Doorn <IvDoorn@gmail.com>
---
v2: - Return non fishy error values.
- Use a exit_fail goto for the error path.
---
drivers/net/wireless/rt2x00/rt2x00mac.c | 44 ++++++++++++++-----------------
1 files changed, 20 insertions(+), 24 deletions(-)
diff --git a/drivers/net/wireless/rt2x00/rt2x00mac.c b/drivers/net/wireless/rt2x00/rt2x00mac.c
index 77af1df..bb0ce51 100644
--- a/drivers/net/wireless/rt2x00/rt2x00mac.c
+++ b/drivers/net/wireless/rt2x00/rt2x00mac.c
@@ -37,6 +37,7 @@ static int rt2x00mac_tx_rts_cts(struct rt2x00_dev *rt2x00dev,
struct ieee80211_tx_info *rts_info;
struct sk_buff *skb;
int size;
+ int retval = 0;
if (tx_info->flags & IEEE80211_TX_CTL_USE_CTS_PROTECT)
size = sizeof(struct ieee80211_cts);
@@ -44,9 +45,9 @@ static int rt2x00mac_tx_rts_cts(struct rt2x00_dev *rt2x00dev,
size = sizeof(struct ieee80211_rts);
skb = dev_alloc_skb(size + rt2x00dev->hw->extra_tx_headroom);
- if (!skb) {
+ if (unlikely(!skb)) {
WARNING(rt2x00dev, "Failed to create RTS/CTS frame.\n");
- return NETDEV_TX_BUSY;
+ return -ENOMEM;
}
skb_reserve(skb, rt2x00dev->hw->extra_tx_headroom);
@@ -82,12 +83,11 @@ static int rt2x00mac_tx_rts_cts(struct rt2x00_dev *rt2x00dev,
frag_skb->data, size, tx_info,
(struct ieee80211_rts *)(skb->data));
- if (rt2x00queue_write_tx_frame(queue, skb)) {
+ retval = rt2x00queue_write_tx_frame(queue, skb);
+ if (retval)
WARNING(rt2x00dev, "Failed to send RTS/CTS frame.\n");
- return NETDEV_TX_BUSY;
- }
- return NETDEV_TX_OK;
+ return retval;
}
int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
@@ -106,11 +106,8 @@ int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
* Note that we can only stop the TX queues inside the TX path
* due to possible race conditions in mac80211.
*/
- if (!test_bit(DEVICE_PRESENT, &rt2x00dev->flags)) {
- ieee80211_stop_queues(hw);
- dev_kfree_skb_any(skb);
- return NETDEV_TX_OK;
- }
+ if (!test_bit(DEVICE_PRESENT, &rt2x00dev->flags))
+ goto exit_fail;
/*
* Determine which queue to put packet on.
@@ -141,15 +138,11 @@ int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
if ((tx_info->flags & (IEEE80211_TX_CTL_USE_RTS_CTS |
IEEE80211_TX_CTL_USE_CTS_PROTECT)) &&
!rt2x00dev->ops->hw->set_rts_threshold) {
- if (rt2x00queue_available(queue) <= 1) {
- ieee80211_stop_queue(rt2x00dev->hw, qid);
- return NETDEV_TX_BUSY;
- }
-
- if (rt2x00mac_tx_rts_cts(rt2x00dev, queue, skb)) {
- ieee80211_stop_queue(rt2x00dev->hw, qid);
- return NETDEV_TX_BUSY;
- }
+ if (rt2x00queue_available(queue) <= 1)
+ goto exit_fail;
+
+ if (rt2x00mac_tx_rts_cts(rt2x00dev, queue, skb))
+ goto exit_fail;
}
/*
@@ -164,15 +157,18 @@ int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
ieee80211hdr->seq_ctrl |= cpu_to_le16(intf->seqno);
}
- if (rt2x00queue_write_tx_frame(queue, skb)) {
- ieee80211_stop_queue(rt2x00dev->hw, qid);
- return NETDEV_TX_BUSY;
- }
+ if (rt2x00queue_write_tx_frame(queue, skb))
+ goto exit_fail;
if (rt2x00queue_threshold(queue))
ieee80211_stop_queue(rt2x00dev->hw, qid);
return NETDEV_TX_OK;
+
+ exit_fail:
+ ieee80211_stop_queue(rt2x00dev->hw, qid);
+ dev_kfree_skb_any(skb);
+ return NETDEV_TX_OK;
}
EXPORT_SYMBOL_GPL(rt2x00mac_tx);
--
1.5.6.2
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v2] rt2x00mac: In error case stop netdev queue, free skb and return NETDEV_TX_OK
2008-07-23 17:17 [PATCH v2] rt2x00mac: In error case stop netdev queue, free skb and return NETDEV_TX_OK Daniel Wagner
@ 2008-07-23 18:05 ` Ivo van Doorn
0 siblings, 0 replies; 2+ messages in thread
From: Ivo van Doorn @ 2008-07-23 18:05 UTC (permalink / raw)
To: Daniel Wagner; +Cc: linux-wireless, John Linville
On Wednesday 23 July 2008, Daniel Wagner wrote:
> It is not allowed to use NETDEV_TX_BUSY in tx path anymore.
>
> Signed-off-by: Daniel Wagner <wagi@monom.org>
> Cc: Ivo van Doorn <IvDoorn@gmail.com>
Thanks.
Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
> ---
> v2: - Return non fishy error values.
> - Use a exit_fail goto for the error path.
> ---
> drivers/net/wireless/rt2x00/rt2x00mac.c | 44 ++++++++++++++-----------------
> 1 files changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00mac.c b/drivers/net/wireless/rt2x00/rt2x00mac.c
> index 77af1df..bb0ce51 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00mac.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00mac.c
> @@ -37,6 +37,7 @@ static int rt2x00mac_tx_rts_cts(struct rt2x00_dev *rt2x00dev,
> struct ieee80211_tx_info *rts_info;
> struct sk_buff *skb;
> int size;
> + int retval = 0;
>
> if (tx_info->flags & IEEE80211_TX_CTL_USE_CTS_PROTECT)
> size = sizeof(struct ieee80211_cts);
> @@ -44,9 +45,9 @@ static int rt2x00mac_tx_rts_cts(struct rt2x00_dev *rt2x00dev,
> size = sizeof(struct ieee80211_rts);
>
> skb = dev_alloc_skb(size + rt2x00dev->hw->extra_tx_headroom);
> - if (!skb) {
> + if (unlikely(!skb)) {
> WARNING(rt2x00dev, "Failed to create RTS/CTS frame.\n");
> - return NETDEV_TX_BUSY;
> + return -ENOMEM;
> }
>
> skb_reserve(skb, rt2x00dev->hw->extra_tx_headroom);
> @@ -82,12 +83,11 @@ static int rt2x00mac_tx_rts_cts(struct rt2x00_dev *rt2x00dev,
> frag_skb->data, size, tx_info,
> (struct ieee80211_rts *)(skb->data));
>
> - if (rt2x00queue_write_tx_frame(queue, skb)) {
> + retval = rt2x00queue_write_tx_frame(queue, skb);
> + if (retval)
> WARNING(rt2x00dev, "Failed to send RTS/CTS frame.\n");
> - return NETDEV_TX_BUSY;
> - }
>
> - return NETDEV_TX_OK;
> + return retval;
> }
>
> int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
> @@ -106,11 +106,8 @@ int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
> * Note that we can only stop the TX queues inside the TX path
> * due to possible race conditions in mac80211.
> */
> - if (!test_bit(DEVICE_PRESENT, &rt2x00dev->flags)) {
> - ieee80211_stop_queues(hw);
> - dev_kfree_skb_any(skb);
> - return NETDEV_TX_OK;
> - }
> + if (!test_bit(DEVICE_PRESENT, &rt2x00dev->flags))
> + goto exit_fail;
>
> /*
> * Determine which queue to put packet on.
> @@ -141,15 +138,11 @@ int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
> if ((tx_info->flags & (IEEE80211_TX_CTL_USE_RTS_CTS |
> IEEE80211_TX_CTL_USE_CTS_PROTECT)) &&
> !rt2x00dev->ops->hw->set_rts_threshold) {
> - if (rt2x00queue_available(queue) <= 1) {
> - ieee80211_stop_queue(rt2x00dev->hw, qid);
> - return NETDEV_TX_BUSY;
> - }
> -
> - if (rt2x00mac_tx_rts_cts(rt2x00dev, queue, skb)) {
> - ieee80211_stop_queue(rt2x00dev->hw, qid);
> - return NETDEV_TX_BUSY;
> - }
> + if (rt2x00queue_available(queue) <= 1)
> + goto exit_fail;
> +
> + if (rt2x00mac_tx_rts_cts(rt2x00dev, queue, skb))
> + goto exit_fail;
> }
>
> /*
> @@ -164,15 +157,18 @@ int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
> ieee80211hdr->seq_ctrl |= cpu_to_le16(intf->seqno);
> }
>
> - if (rt2x00queue_write_tx_frame(queue, skb)) {
> - ieee80211_stop_queue(rt2x00dev->hw, qid);
> - return NETDEV_TX_BUSY;
> - }
> + if (rt2x00queue_write_tx_frame(queue, skb))
> + goto exit_fail;
>
> if (rt2x00queue_threshold(queue))
> ieee80211_stop_queue(rt2x00dev->hw, qid);
>
> return NETDEV_TX_OK;
> +
> + exit_fail:
> + ieee80211_stop_queue(rt2x00dev->hw, qid);
> + dev_kfree_skb_any(skb);
> + return NETDEV_TX_OK;
> }
> EXPORT_SYMBOL_GPL(rt2x00mac_tx);
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2008-07-23 17:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-23 17:17 [PATCH v2] rt2x00mac: In error case stop netdev queue, free skb and return NETDEV_TX_OK Daniel Wagner
2008-07-23 18:05 ` Ivo van Doorn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).