[PATCH] rt2x00: configure_filter() callback is allowed to sleep

[PATCH] rt2x00: configure_filter() callback is allowed to sleep

[Ivo van Doorn]

The configure_filter() callback function no longer needs
to be atomic. Remove the scheduled work structure and
call into the driver configure_filter() directly.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00.h    |    1 -
 drivers/net/wireless/rt2x00/rt2x00dev.c |   10 ----------
 drivers/net/wireless/rt2x00/rt2x00mac.c |    5 +----
 3 files changed, 1 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 39d7d9b..ad70946 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -826,7 +826,6 @@ struct rt2x00_dev {
 	 * due to RTNL locking requirements.
 	 */
 	struct work_struct intf_work;
-	struct work_struct filter_work;
 
 	/*
 	 * Data queue arrays for RX, TX and Beacon.
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 9ab70e4..16b560a 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -118,14 +118,6 @@ void rt2x00lib_toggle_rx(struct rt2x00_dev *rt2x00dev, enum dev_state state)
 		rt2x00link_start_tuner(rt2x00dev);
 }
 
-static void rt2x00lib_packetfilter_scheduled(struct work_struct *work)
-{
-	struct rt2x00_dev *rt2x00dev =
-	    container_of(work, struct rt2x00_dev, filter_work);
-
-	rt2x00dev->ops->lib->config_filter(rt2x00dev, rt2x00dev->packet_filter);
-}
-
 static void rt2x00lib_intf_scheduled_iter(void *data, u8 *mac,
 					  struct ieee80211_vif *vif)
 {
@@ -859,7 +851,6 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
 	 * Initialize configuration work.
 	 */
 	INIT_WORK(&rt2x00dev->intf_work, rt2x00lib_intf_scheduled);
-	INIT_WORK(&rt2x00dev->filter_work, rt2x00lib_packetfilter_scheduled);
 
 	/*
 	 * Allocate queue array.
@@ -907,7 +898,6 @@ void rt2x00lib_remove_dev(struct rt2x00_dev *rt2x00dev)
 	/*
 	 * Stop all work.
 	 */
-	cancel_work_sync(&rt2x00dev->filter_work);
 	cancel_work_sync(&rt2x00dev->intf_work);
 
 	/*
diff --git a/drivers/net/wireless/rt2x00/rt2x00mac.c b/drivers/net/wireless/rt2x00/rt2x00mac.c
index 3011aea..967d3b5 100644
--- a/drivers/net/wireless/rt2x00/rt2x00mac.c
+++ b/drivers/net/wireless/rt2x00/rt2x00mac.c
@@ -430,10 +430,7 @@ void rt2x00mac_configure_filter(struct ieee80211_hw *hw,
 		return;
 	rt2x00dev->packet_filter = *total_flags;
 
-	if (!test_bit(DRIVER_REQUIRE_SCHEDULED, &rt2x00dev->flags))
-		rt2x00dev->ops->lib->config_filter(rt2x00dev, *total_flags);
-	else
-		ieee80211_queue_work(rt2x00dev->hw, &rt2x00dev->filter_work);
+	rt2x00dev->ops->lib->config_filter(rt2x00dev, *total_flags);
 }
 EXPORT_SYMBOL_GPL(rt2x00mac_configure_filter);
 
-- 
1.6.4

Re: [PATCH] rt2x00: configure_filter() callback is allowed to sleep

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

On Tue, 2009-08-18 at 19:54 +0200, Ivo van Doorn wrote:
> The configure_filter() callback function no longer needs
> to be atomic. Remove the scheduled work structure and
> call into the driver configure_filter() directly.

Nice, yeah, I didn't want to do that myself :)

Speaking of which -- bss_info_changed can sleep too, which means you can
remove DRIVER_REQUIRE_SCHEDULED completely I think?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] rt2x00: configure_filter() callback is allowed to sleep

[Ivo van Doorn]

On Tuesday 18 August 2009, Johannes Berg wrote:
> On Tue, 2009-08-18 at 19:54 +0200, Ivo van Doorn wrote:
> > The configure_filter() callback function no longer needs
> > to be atomic. Remove the scheduled work structure and
> > call into the driver configure_filter() directly.
> 
> Nice, yeah, I didn't want to do that myself :)
> 
> Speaking of which -- bss_info_changed can sleep too, which means you can
> remove DRIVER_REQUIRE_SCHEDULED completely I think?

Seriously, that would be great. Patch coming up in a few minutes. ;)

Ivo

Re: [PATCH] rt2x00: configure_filter() callback is allowed to sleep

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

On Tue, 2009-08-18 at 20:04 +0200, Ivo van Doorn wrote:

> > Speaking of which -- bss_info_changed can sleep too, which means you can
> > remove DRIVER_REQUIRE_SCHEDULED completely I think?
> 
> Seriously, that would be great. Patch coming up in a few minutes. ;)

Cool.

Hey, sorry it took so long to make it possible :)

I wonder if drivers now ever have a reason to queue work on the mac80211
workqueue? Some periodic calibration tasks maybe?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] rt2x00: configure_filter() callback is allowed to sleep

[Luis R. Rodriguez]

On Tue, Aug 18, 2009 at 11:07 AM, Johannes
Berg<johannes@sipsolutions.net> wrote:
> On Tue, 2009-08-18 at 20:04 +0200, Ivo van Doorn wrote:
>
>> > Speaking of which -- bss_info_changed can sleep too, which means you can
>> > remove DRIVER_REQUIRE_SCHEDULED completely I think?
>>
>> Seriously, that would be great. Patch coming up in a few minutes. ;)
>
> Cool.
>
> Hey, sorry it took so long to make it possible :)
>
> I wonder if drivers now ever have a reason to queue work on the mac80211
> workqueue? Some periodic calibration tasks maybe?

ath9k just has the virtual wiphy stuff and that uses the mac80211
workqueue.  Maybe we can review this stuff at the summit in
consideration for how we may want this eventually on
mac80211/cfg80211.

  Luis

Re: [PATCH] rt2x00: configure_filter() callback is allowed to sleep

[Ivo van Doorn]

On Tuesday 18 August 2009, Johannes Berg wrote:
> On Tue, 2009-08-18 at 20:04 +0200, Ivo van Doorn wrote:
> 
> > > Speaking of which -- bss_info_changed can sleep too, which means you can
> > > remove DRIVER_REQUIRE_SCHEDULED completely I think?
> > 
> > Seriously, that would be great. Patch coming up in a few minutes. ;)
> 
> Cool.
> 
> Hey, sorry it took so long to make it possible :)
>
> I wonder if drivers now ever have a reason to queue work on the mac80211
> workqueue? Some periodic calibration tasks maybe?

Well after the removal of DRIVER_REQUIRE_SCHEDULED I only have 1
workqueue structure left: Updating beacons.

During the update it needs to iterate through all interfaces using
ieee80211_iterate_active_interfaces_atomic(), during the actual update
a mutex is required because USB device access is required. And not in the least
the function is called by the set_tim() callback function.

Ivo

Re: [PATCH] rt2x00: configure_filter() callback is allowed to sleep

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> I wonder if drivers now ever have a reason to queue work on the mac80211
> workqueue? Some periodic calibration tasks maybe?

wl1251 and wl1271 queue tx frames to a workqueue. This is because they
use synchronous spi interface. This might change if we switch
asynchronous interface in the future. No idea how this affects sdio,
though.

-- 
Kalle Valo