linux-kernel - Re: hp_accel: do not call ACPI from invalid context

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20090113224013.GC7770@elf.ucw.cz>
Date:	Tue, 13 Jan 2009 23:40:13 +0100
From:	Pavel Machek <pavel@...e.cz>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, eric.piel@...mplin-utc.net
Subject: Re: hp_accel: do not call ACPI from invalid context

On Tue 2009-01-13 14:17:21, Andrew Morton wrote:
> On Mon, 12 Jan 2009 11:31:55 +0100
> Pavel Machek <pavel@...e.cz> wrote:
> 
> > 
> > LED on HP notebooks is connected through ACPI. That unfortunately
> > means that it needs to be delayed by using schedule_work() to avoid
> > calling ACPI interpretter in invalid context. This patch fixes that.
> > 
> > ...
> >  
> > +/* Delayed LEDs infrastructure ------------------------------------ */ 
> > +
> > +/* Special LED class that can defer work */
> > +struct delayed_led_classdev {
> > +	struct led_classdev led_classdev;
> > +	struct work_struct work;
> > +	enum led_brightness new_brightness;
> > +
> > +	unsigned int led;		/* For driver */
> > +	void (*set_brightness)(struct delayed_led_classdev *data, enum led_brightness value);
> > +};
> > +
> > +static inline void delayed_set_status_worker(struct work_struct *work)
> > +{
> > +	struct delayed_led_classdev *data =
> > +			container_of(work, struct delayed_led_classdev, work);
> > +
> > +	data->set_brightness(data, data->new_brightness);
> > +}
> > +
> > +static inline void delayed_sysfs_set(struct led_classdev *led_cdev,
> > +			      enum led_brightness brightness)
> > +{
> > +	struct delayed_led_classdev *data = container_of(led_cdev,
> > +			     struct delayed_led_classdev, led_classdev);
> > +	data->new_brightness = brightness;
> > +	schedule_work(&data->work);
> > +}
> 
> Is this the only LED driver int he current and future history of the
> world which uses ACPI?
> 
> coz otherwise, this code might better live in LEDs core somewhere, so
> those other drivers can use it?

There are 2 other drivers (thinkpad_acpi and asus-something IIRC) that
want the same infrastructure. So I put that code in
include/linux/delayed-leds.h in my tree, but then I realized I'd need
to coordinate too many maintainers and just inlined it for the
merge. (We do not want to schedule from atomic, right?)

I already have a patch for thinkpad_acpi, but it needs a bit more
testing and perhaps some tweaks by maintainer. So yes, eventually
putting that into shared place is a plan.

> >  	if (ret) {
> > -		led_classdev_unregister(&hpled_led);
> > +		while (work_pending(&hpled_led.work))
> > +			schedule();
> 
> We have flush_work() for this?

I did not realize flush_work() exists/does what I need... sorry.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/