[PATCH] dell-laptop: defer dell_rfkill_update to worker thread

Andy Whitcroft apw at canonical.com
Tue Apr 13 14:29:00 UTC 2010


On Fri, Apr 09, 2010 at 06:17:55PM -0400, Chase Douglas wrote:
> From: Chase Douglas <cndougla at emerald.redvoodoo.org>
> 
> dell_rfkill_update fires an SMI, which must occur on cpu 0. Thus, if called
> on a different cpu, the task will block. Since it's called in hard irq
> context, we must defer this to the worker thread.
> 
> It is potentially possible that two rfkill key events could be processed
> before the work completes on the first. The second event is dropped with a
> KERN_NOTICE message.
> 
> BugLink: http://bugs.launchpad.net/bugs/555261
> 
> Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
> ---
>  drivers/platform/x86/dell-laptop.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index 8bf7ac7..15d96a0 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -288,8 +288,10 @@ static const struct rfkill_ops dell_rfkill_ops = {
>  /*
>   * Called for each KEY_WLAN key press event. Note that a physical
>   * rf-kill switch change also causes the BIOS to emit a KEY_WLAN.
> + *
> + * dell_rfkill_set may block, so schedule it on a worker thread.
>   */
> -static void dell_rfkill_update(void)
> +static void dell_rfkill_update(struct work_struct *work)
>  {
>  	hw_switch_status ^= BIT(HW_SWITCH_MASK);
>  	if (wifi_rfkill && (hw_switch_status & BIT(WLAN_SWITCH_MASK))) {
> @@ -307,6 +309,7 @@ static void dell_rfkill_update(void)
>  		dell_rfkill_set((void*)3, rfkill_blocked(wwan_rfkill));
>  	}
>  }
> +DECLARE_WORK(dell_rfkill_update_work, &dell_rfkill_update);
>  
>  static int dell_setup_rfkill(void)
>  {
> @@ -431,7 +434,9 @@ static bool dell_input_filter(struct input_handle *handle, unsigned int type,
>  			     unsigned int code, int value)
>  {
>  	if (type == EV_KEY && code == KEY_WLAN && value == 1) {
> -		dell_rfkill_update();
> +		if (!schedule_work(&dell_rfkill_update_work))
> +			printk(KERN_NOTICE "rfkill switch handling already "
> +					   "scheduled, dropping this event\n");
>  		return 1;
>  	}
>  

Looks like a fair work-around.  In theory we could loose key-press
effects here which would could lead us to be out of sync with the H/W,
though to do that one would have to hit the key before the handler could
complete -- which I suspect is close to impossible to achieve.  In that
case we get a clear diagnostic, and its a heck of a lot better than a
Panic.

Acked-by: Andy Whitcroft <apw at canonical.com>

-apw




More information about the kernel-team mailing list