[PATCH 1/2] mac80211: add ieee80211_rx_ni()

[PATCH 1/2] mac80211: add ieee80211_rx_ni()

[Kalle Valo]

From: Kalle Valo <kalle.valo@nokia.com>

ieee80211_rx() must be called with bottom halves disabled. To simplify
driver development implement ieee80211_rx_ni() which disables BH. This
function must be used when in process context.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 include/net/mac80211.h |   32 ++++++++++++++++++++++++++------
 1 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index c75b960..c42c4a8 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1665,11 +1665,11 @@ void ieee80211_restart_hw(struct ieee80211_hw *hw);
  * header if %RX_FLAG_RADIOTAP is set in the @status flags.
  *
  * This function may not be called in IRQ context. Calls to this function
- * for a single hardware must be synchronized against each other. Calls
- * to this function and ieee80211_rx_irqsafe() may not be mixed for a
- * single hardware.
+ * for a single hardware must be synchronized against each other. Calls to
+ * this function, ieee80211_rx_ni() and ieee80211_rx_irqsafe() may not be
+ * mixed for a single hardware.
  *
- * Note that right now, this function must be called with softirqs disabled.
+ * In process context use instead ieee80211_rx_ni().
  *
  * @hw: the hardware this frame came in on
  * @skb: the buffer to receive, owned by mac80211 after this call
@@ -1682,8 +1682,8 @@ void ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb);
  * Like ieee80211_rx() but can be called in IRQ context
  * (internally defers to a tasklet.)
  *
- * Calls to this function and ieee80211_rx() may not be mixed for a
- * single hardware.
+ * Calls to this function, ieee80211_rx() or ieee80211_rx_ni() may not
+ * be mixed for a single hardware.
  *
  * @hw: the hardware this frame came in on
  * @skb: the buffer to receive, owned by mac80211 after this call
@@ -1691,6 +1691,26 @@ void ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb);
 void ieee80211_rx_irqsafe(struct ieee80211_hw *hw, struct sk_buff *skb);
 
 /**
+ * ieee80211_rx_ni - receive frame (in process context)
+ *
+ * Like ieee80211_rx() but can be called in process context
+ * (internally disables bottom halves).
+ *
+ * Calls to this function, ieee80211_rx() and ieee80211_rx_irqsafe() may
+ * not be mixed for a single hardware.
+ *
+ * @hw: the hardware this frame came in on
+ * @skb: the buffer to receive, owned by mac80211 after this call
+ */
+static inline void ieee80211_rx_ni(struct ieee80211_hw *hw,
+				   struct sk_buff *skb)
+{
+	local_bh_disable();
+	ieee80211_rx(hw, skb);
+	local_bh_enable();
+}
+
+/**
  * ieee80211_tx_status - transmit status callback
  *
  * Call this function for all transmitted frames after they have been

[PATCH 2/2] wl1251: use ieee80211_rx_ni()

[Kalle Valo]

From: Kalle Valo <kalle.valo@nokia.com>

Because of SPI and SDIO wl1251 does everything in a workqueue, including
calling ieee80211_rx() which should be called with bottom halves disabled.
An error message is emitted because of this:

NOHZ: local_softirq_pending 08

Fix this by using ieee80211_rx_ni().

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 drivers/net/wireless/wl12xx/wl1251_rx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1251_rx.c b/drivers/net/wireless/wl12xx/wl1251_rx.c
index 17c54b5..601fe0d 100644
--- a/drivers/net/wireless/wl12xx/wl1251_rx.c
+++ b/drivers/net/wireless/wl12xx/wl1251_rx.c
@@ -153,7 +153,7 @@ static void wl1251_rx_body(struct wl1251 *wl,
 		     beacon ? "beacon" : "");
 
 	memcpy(IEEE80211_SKB_RXCB(skb), &status, sizeof(status));
-	ieee80211_rx(wl->hw, skb);
+	ieee80211_rx_ni(wl->hw, skb);
 }
 
 static void wl1251_rx_ack(struct wl1251 *wl)

Re: [PATCH 1/2] mac80211: add ieee80211_rx_ni()

[Julian Calaby]

On Wed, Oct 14, 2009 at 03:33, Kalle Valo <kalle.valo@iki.fi> wrote:
> From: Kalle Valo <kalle.valo@nokia.com>
>
> ieee80211_rx() must be called with bottom halves disabled. To simplify
> driver development implement ieee80211_rx_ni() which disables BH. This
> function must be used when in process context.
>
> Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
> ---
>
>  include/net/mac80211.h |   32 ++++++++++++++++++++++++++------
>  1 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index c75b960..c42c4a8 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1665,11 +1665,11 @@ void ieee80211_restart_hw(struct ieee80211_hw *hw);
>  * header if %RX_FLAG_RADIOTAP is set in the @status flags.
>  *
>  * This function may not be called in IRQ context. Calls to this function
> - * for a single hardware must be synchronized against each other. Calls
> - * to this function and ieee80211_rx_irqsafe() may not be mixed for a
> - * single hardware.
> + * for a single hardware must be synchronized against each other. Calls to
> + * this function, ieee80211_rx_ni() and ieee80211_rx_irqsafe() may not be
> + * mixed for a single hardware.

[snip]

> - * Calls to this function and ieee80211_rx() may not be mixed for a
> - * single hardware.
> + * Calls to this function, ieee80211_rx() or ieee80211_rx_ni() may not
> + * be mixed for a single hardware.

You missed out ieee80211_rx_irqsafe() here.

Thanks,

-- 

Julian Calaby

Email: julian.calaby@gmail.com
.Plan: http://sites.google.com/site/juliancalaby/

Re: [PATCH 1/2] mac80211: add ieee80211_rx_ni()

[Kalle Valo]

Julian Calaby <julian.calaby@gmail.com> writes:

>> - * Calls to this function and ieee80211_rx() may not be mixed for a
>> - * single hardware.
>> + * Calls to this function, ieee80211_rx() or ieee80211_rx_ni() may not
>> + * be mixed for a single hardware.
>
> You missed out ieee80211_rx_irqsafe() here.

Here "this function" refer to ieee80211_rx_irqsafe(). But I can also
write it like this:

  Calls to ieee80211_rx_irqsafe(), ieee80211_rx() or ieee80211_rx_ni()
  may not be mixed for a single hardware.

Is that any better?

-- 
Kalle Valo

Re: [PATCH 1/2] mac80211: add ieee80211_rx_ni()

[Julian Calaby]

On Wed, Oct 14, 2009 at 16:28, Kalle Valo <kalle.valo@iki.fi> wrote:
> Julian Calaby <julian.calaby@gmail.com> writes:
>
>>> - * Calls to this function and ieee80211_rx() may not be mixed for a
>>> - * single hardware.
>>> + * Calls to this function, ieee80211_rx() or ieee80211_rx_ni() may not
>>> + * be mixed for a single hardware.
>>
>> You missed out ieee80211_rx_irqsafe() here.
>
> Here "this function" refer to ieee80211_rx_irqsafe(). But I can also
> write it like this:
>
>  Calls to ieee80211_rx_irqsafe(), ieee80211_rx() or ieee80211_rx_ni()
>  may not be mixed for a single hardware.
>
> Is that any better?

Smacks self upside the head.

This is what I get for commenting on patches in the morning.

Sorry for the noise.

Thanks,

-- 

Julian Calaby

Email: julian.calaby@gmail.com
.Plan: http://sites.google.com/site/juliancalaby/

Re: [PATCH 1/2] mac80211: add ieee80211_rx_ni()

[Kalle Valo]

Julian Calaby <julian.calaby@gmail.com> writes:

>>> You missed out ieee80211_rx_irqsafe() here.
>>
>> Here "this function" refer to ieee80211_rx_irqsafe(). But I can also
>> write it like this:
>>
>>  Calls to ieee80211_rx_irqsafe(), ieee80211_rx() or ieee80211_rx_ni()
>>  may not be mixed for a single hardware.
>>
>> Is that any better?
>
> Smacks self upside the head.
>
> This is what I get for commenting on patches in the morning.
>
> Sorry for the noise.

No problem at all, thank you for the review.

-- 
Kalle Valo