LKML Archive on
help / color / mirror / Atom feed
From: Richard Purdie <>
To: Nicolas Boichat <>
Cc: Andrew Morton <>,
	Bradley Hook <>,,
Subject: Re: [PATCH] applesmc - fix crash when activating a led trigger on the keyboard backlight
Date: Sat, 14 Apr 2007 09:45:11 +0100	[thread overview]
Message-ID: <1176540311.6818.9.camel@localhost.localdomain> (raw)
In-Reply-To: <>


On Sat, 2007-04-14 at 16:05 +0800, Nicolas Boichat wrote:
> Bradley Hook wrote:
> > Slightly off-topic, but I've been experiencing a minor bug in the
> > keyboard backlight feature.
> >
> > I say it is "minor" only because the feature serves no real functional
> > purpose. You can activate a trigger called "heartbeat" that will cause
> > the keyboard light to pulse at a speed based on the CPU usage. On my
> > MBP17, after activating this trigger the machine will either lock-up
> > or core dump within about a minute (timing is not consistent).
> >   
> This is caused by the fact applesmc_backlight_set locks a mutex (or more
> precisely sleeps while trying to lock a mutex) while being in a softirq
> context.
> This might be obvious for others, but it was not for me, and there is
> absolutely no mention in the documentation of the fact it is not always
> safe to sleep in the brightness_set handler of a led_class device (it is
> safe when it is called because someone wrote to the brightness sysfs file).

Its never safe for a brightness_set handler to sleep. They're designed
to be called from interrupt context and as you've noted, several
triggers do that.

The solution if you have locks like your case is to offload the work to
a workqueue, there is simply no other way to do it.

I'll have a look to see if I can improve the documentation (patches

> Also, the led-trigger code seems buggy when it comes to locking. Setting
> CONFIG_DEBUG_SPINLOCK_SLEEP causes a lot a warnings. The problem is that
> the list of triggers is locked using a rw spinlock, but the rest of the
> code seems to ignore that, and calls a lot of functions which can sleep
> (kzalloc with GFP_KERNEL, sysfs_add_file, mutex_lock, etc...). I think
> the list lock should be converted to a mutex (or maybe modified to use
> RCU). I'm not very experienced in that domain, but if you want I can
> provide a patch for this.

Someone else has mentioned this within the past few days and there is a
problem with the trigger->activate and ->deactivate calls occurring
within a spinlock. Its not a simple problem to solve unfortunately and
you can't just convert to a mutex but I'm looking at it. Again, patches
welcome but its going to need careful thought.



  reply	other threads:[~2007-04-14  8:46 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-14  9:29 [RFC][PATCH] Apple SMC driver (hardware monitoring and control) Nicolas Boichat
2007-03-14 11:11 ` Cong WANG
2007-03-14 14:00   ` Cong WANG
2007-03-15 11:31     ` Nicolas Boichat
2007-03-19  5:19 ` [PATCH] " Nicolas Boichat
2007-03-19  6:54   ` Andrew Morton
2007-03-19  7:35     ` Nicolas Boichat
2007-03-20  7:12     ` Nicolas Boichat
2007-03-22 15:37   ` Dmitry Torokhov
2007-04-09 13:53     ` [PATCH] Apple SMC driver - fix input device Nicolas Boichat
2007-04-09 15:17       ` Dmitry Torokhov
2007-04-09 20:04       ` Andrew Morton
2007-04-09 20:11         ` Dmitry Torokhov
2007-04-09 21:51         ` Paul Mackerras
2007-03-19 21:43 ` [RFC][PATCH] Apple SMC driver (hardware monitoring and control) Bob Copeland
2007-03-20  7:02   ` Nicolas Boichat
2007-03-20 15:14     ` Bob Copeland
2007-03-21  4:03     ` Bob Copeland
     [not found]     ` <>
2007-04-14  8:05       ` [PATCH] applesmc - fix crash when activating a led trigger on the keyboard backlight Nicolas Boichat
2007-04-14  8:45         ` Richard Purdie [this message]
2007-04-14 13:31           ` [PATCH] applesmc - fix crash when activating a led trigger on the keyboard backlight - use a workqueue Nicolas Boichat
2007-03-20 10:08   ` [lm-sensors] [RFC][PATCH] Apple SMC driver (hardware monitoring and control) Jean Delvare
2007-03-22 10:36     ` Nicolas Boichat
2007-03-20 16:12 ` Gerb Stralko
2007-04-11 12:25 ` [lm-sensors] " Jean Delvare
2007-04-11 12:47   ` Nicolas Boichat
2007-04-13  5:33   ` [PATCH 1/2] Apple SMC driver - standardize and sanitize sysfs tree + minor features addition Nicolas Boichat
2007-04-13  6:38     ` [PATCH 2/2] Apple SMC driver - implement key enumeration Nicolas Boichat

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1176540311.6818.9.camel@localhost.localdomain \ \ \ \ \ \ \
    --subject='Re: [PATCH] applesmc - fix crash when activating a led trigger on the keyboard backlight' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).