On Wednesday 04 November 2009 16:11:33 John W. Linville wrote:
> On Wed, Nov 04, 2009 at 12:00:25AM -0600, Larry Finger wrote:
> > As reported by Rick Farina (sidhayn@xxxxxxxxx), removing the RTL8187 USB
> > stick, or unloading the driver rtl8187 using rmmod will cause a kernel oops.
> > There are at least two forms of the failure, (1) BUG: Scheduling while atomic,
> > and (2) a fatal kernel page fault. This problem is reported in Bugzilla #14539.
> >
> > This problem does not occur for kernel 2.6.31, but does for 2.6.32-rc2, thus
> > it is technically a regression; however, bisection did not locate any faulty
> > patch. The fix was found by comparing the faulty code in rtl8187 with p54usb.
> > My interpretation is that the handling of work queues in mac80211 changed
> > enough to the LEDs to be unregistered before tasks on the work queues are
> > cancelled. Previously, these actions could be done in either order.
> >
> > Signed-off-by: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
> > Reported-and-tested by: Rick Farina <sidhayn@xxxxxxxxx>
> > ---
> >
> > John,
> >
> > This is 2.6.32 material. Sorry to take so long to get a patch, but it was
> > difficult for me to locate the problem. Fortunately, I had the postings of the
> > two flame wars to amuse me while all the kernel compilations were happening.
> >
> > Larry
> > ---
> >
> > Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
> > ===================================================================
> > --- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_leds.c
> > +++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
> > @@ -210,10 +210,10 @@ void rtl8187_leds_exit(struct ieee80211_
> >
> > /* turn the LED off before exiting */
> > ieee80211_queue_delayed_work(dev, &priv->led_off, 0);
> > - cancel_delayed_work_sync(&priv->led_off);
> > - cancel_delayed_work_sync(&priv->led_on);
> > rtl8187_unregister_led(&priv->led_rx);
> > rtl8187_unregister_led(&priv->led_tx);
> > + cancel_delayed_work_sync(&priv->led_off);
> > + cancel_delayed_work_sync(&priv->led_on);
> > }
> > #endif /* def CONFIG_RTL8187_LED */
> >
>
> This seems like a band-aid. If anything, the original order would
> seem to make more sense.
really?
take this code from led-class.c
void led_classdev_unregister(struct led_classdev *led_cdev)
{
device_remove_file(led_cdev->dev, &dev_attr_max_brightness);
device_remove_file(led_cdev->dev, &dev_attr_brightness);
#ifdef CONFIG_LEDS_TRIGGERS
device_remove_file(led_cdev->dev, &dev_attr_trigger);
down_write(&led_cdev->trigger_lock);
if (led_cdev->trigger)
led_trigger_set(led_cdev, NULL);
up_write(&led_cdev->trigger_lock);
#endif
device_unregister(led_cdev->dev);
down_write(&leds_list_lock);
list_del(&led_cdev->node);
up_write(&leds_list_lock);
}
as you can see the led is switched-off right before the device is unregistered.
but rtl8187, p54 & ar9170 led-triggers are timed & asynchronous. So
we really need a cancel_delayed_work_sync after the unregister routine
finished... else the timed trigger might fire when the device/module
is _faded_ from memory.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
