From: Rafael J. Wysocki <[email protected]>
If thermal polling is enabled, which for example is the case for
ACPI thermal zones with the _TZP object defined, the thermal driver
uses delayed work items for this purpose. Unfortunately, since
they are queued up using schedule_delayed_work(), the work function
may be executed during system suspend or resume, which is not
desirable.
To prevent that from happening, use a freezable workqueue for
queuing up delayed work items in the thermal driver.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/thermal/thermal_sys.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
Index: linux-2.6/drivers/thermal/thermal_sys.c
===================================================================
--- linux-2.6.orig/drivers/thermal/thermal_sys.c
+++ linux-2.6/drivers/thermal/thermal_sys.c
@@ -62,6 +62,20 @@ static DEFINE_MUTEX(thermal_list_lock);
static unsigned int thermal_event_seqnum;
+static struct workqueue_struct *thermal_wq;
+
+static int __init thermal_start_workqueue(void)
+{
+ thermal_wq = alloc_workqueue("thermal", WQ_FREEZEABLE, 0);
+ return thermal_wq ? 0 : -ENOMEM;
+}
+
+static inline void thermal_destroy_workqueue(void)
+{
+ if (thermal_wq)
+ destroy_workqueue(thermal_wq);
+}
+
static struct genl_family thermal_event_genl_family = {
.id = GENL_ID_GENERATE,
.name = THERMAL_GENL_FAMILY_NAME,
@@ -611,10 +625,10 @@ static void thermal_zone_device_set_poll
return;
if (delay > 1000)
- schedule_delayed_work(&(tz->poll_queue),
+ queue_delayed_work(thermal_wq, &(tz->poll_queue),
round_jiffies(msecs_to_jiffies(delay)));
else
- schedule_delayed_work(&(tz->poll_queue),
+ queue_delayed_work(thermal_wq, &(tz->poll_queue),
msecs_to_jiffies(delay));
}
@@ -1306,11 +1320,14 @@ static int __init thermal_init(void)
int result = 0;
result = class_register(&thermal_class);
+ if (!result)
+ result = thermal_start_workqueue();
if (result) {
idr_destroy(&thermal_tz_idr);
idr_destroy(&thermal_cdev_idr);
mutex_destroy(&thermal_idr_lock);
mutex_destroy(&thermal_list_lock);
+ thermal_destroy_workqueue();
}
result = genetlink_init();
return result;
@@ -1328,6 +1345,7 @@ static void __exit thermal_exit(void)
idr_destroy(&thermal_cdev_idr);
mutex_destroy(&thermal_idr_lock);
mutex_destroy(&thermal_list_lock);
+ thermal_destroy_workqueue();
genetlink_exit();
}
On Tue, Feb 08, 2011 at 12:29:57AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> If thermal polling is enabled, which for example is the case for
> ACPI thermal zones with the _TZP object defined, the thermal driver
> uses delayed work items for this purpose. Unfortunately, since
> they are queued up using schedule_delayed_work(), the work function
> may be executed during system suspend or resume, which is not
> desirable.
>
> To prevent that from happening, use a freezable workqueue for
> queuing up delayed work items in the thermal driver.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/thermal/thermal_sys.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/drivers/thermal/thermal_sys.c
> ===================================================================
> --- linux-2.6.orig/drivers/thermal/thermal_sys.c
> +++ linux-2.6/drivers/thermal/thermal_sys.c
> @@ -62,6 +62,20 @@ static DEFINE_MUTEX(thermal_list_lock);
>
> static unsigned int thermal_event_seqnum;
>
> +static struct workqueue_struct *thermal_wq;
> +
> +static int __init thermal_start_workqueue(void)
> +{
> + thermal_wq = alloc_workqueue("thermal", WQ_FREEZEABLE, 0);
Should probably be unbound as well.
FWIW, I would not mind if we had a global freezeable workqueue already
predefined. I could switch input_polldev and vmw_balloon to it and there
probably could be more users...
Thanks.
--
Dmitry
On Tuesday, February 08, 2011, Dmitry Torokhov wrote:
> On Tue, Feb 08, 2011 at 12:29:57AM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > If thermal polling is enabled, which for example is the case for
> > ACPI thermal zones with the _TZP object defined, the thermal driver
> > uses delayed work items for this purpose. Unfortunately, since
> > they are queued up using schedule_delayed_work(), the work function
> > may be executed during system suspend or resume, which is not
> > desirable.
> >
> > To prevent that from happening, use a freezable workqueue for
> > queuing up delayed work items in the thermal driver.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/thermal/thermal_sys.c | 22 ++++++++++++++++++++--
> > 1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > Index: linux-2.6/drivers/thermal/thermal_sys.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/thermal/thermal_sys.c
> > +++ linux-2.6/drivers/thermal/thermal_sys.c
> > @@ -62,6 +62,20 @@ static DEFINE_MUTEX(thermal_list_lock);
> >
> > static unsigned int thermal_event_seqnum;
> >
> > +static struct workqueue_struct *thermal_wq;
> > +
> > +static int __init thermal_start_workqueue(void)
> > +{
> > + thermal_wq = alloc_workqueue("thermal", WQ_FREEZEABLE, 0);
>
> Should probably be unbound as well.
Yup, thanks.
> FWIW, I would not mind if we had a global freezeable workqueue already
> predefined. I could switch input_polldev and vmw_balloon to it and there
> probably could be more users...
Hmm, OK.
Do you think we should add system_freezeable_wq to the set of predefined
workqueues?
Rafael
Hello, Dmitry, Rafael.
On Tue, Feb 08, 2011 at 10:20:25AM +0100, Rafael J. Wysocki wrote:
> > > + thermal_wq = alloc_workqueue("thermal", WQ_FREEZEABLE, 0);
> >
> > Should probably be unbound as well.
>
> Yup, thanks.
Hmm.. why should it be unbound? Unbound wqs are primarily useful for
very long running (system daemon type) works which may also consume
considerable amount of cpu cycles.
> > FWIW, I would not mind if we had a global freezeable workqueue already
> > predefined. I could switch input_polldev and vmw_balloon to it and there
> > probably could be more users...
>
> Hmm, OK.
>
> Do you think we should add system_freezeable_wq to the set of predefined
> workqueues?
Oh, yeah, definitely. I'll write up a patch.
Thanks.
--
tejun
>From de62c151df9b6568c67977920267392c432f340a Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Tue, 8 Feb 2011 10:39:03 +0100
Add system wide freezeable workqueue.
Signed-off-by: Tejun Heo <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
Please feel free to pull from the following branch.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-2.6.39
Thanks.
include/linux/workqueue.h | 4 ++++
kernel/workqueue.c | 4 +++-
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 1ac1158..de6a755 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -286,11 +286,15 @@ enum {
* any specific CPU, not concurrency managed, and all queued works are
* executed immediately as long as max_active limit is not reached and
* resources are available.
+ *
+ * system_freezeable_wq is equivalent to system_wq except that it's
+ * freezeable.
*/
extern struct workqueue_struct *system_wq;
extern struct workqueue_struct *system_long_wq;
extern struct workqueue_struct *system_nrt_wq;
extern struct workqueue_struct *system_unbound_wq;
+extern struct workqueue_struct *system_freezeable_wq;
extern struct workqueue_struct *
__alloc_workqueue_key(const char *name, unsigned int flags, int max_active,
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 11869fa..2b8de0c 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3764,8 +3764,10 @@ static int __init init_workqueues(void)
system_nrt_wq = alloc_workqueue("events_nrt", WQ_NON_REENTRANT, 0);
system_unbound_wq = alloc_workqueue("events_unbound", WQ_UNBOUND,
WQ_UNBOUND_MAX_ACTIVE);
+ system_freezeable_wq = alloc_workqueue("events_freezeable",
+ WQ_FREEZEABLE, 0);
BUG_ON(!system_wq || !system_long_wq || !system_nrt_wq ||
- !system_unbound_wq);
+ !system_unbound_wq || !system_freezeable_wq);
return 0;
}
early_initcall(init_workqueues);
--
1.7.1
On Tue, 8 Feb 2011, Tejun Heo wrote:
> From de62c151df9b6568c67977920267392c432f340a Mon Sep 17 00:00:00 2001
> From: Tejun Heo <[email protected]>
> Date: Tue, 8 Feb 2011 10:39:03 +0100
>
> Add system wide freezeable workqueue.
The preferred spelling is "freezable".
Alan Stern
On Tue, Feb 08, 2011 at 10:10:46AM -0500, Alan Stern wrote:
> On Tue, 8 Feb 2011, Tejun Heo wrote:
>
> > From de62c151df9b6568c67977920267392c432f340a Mon Sep 17 00:00:00 2001
> > From: Tejun Heo <[email protected]>
> > Date: Tue, 8 Feb 2011 10:39:03 +0100
> >
> > Add system wide freezeable workqueue.
>
> The preferred spelling is "freezable".
Hmmm... workqueue always has used freezeable (even before cmwq
changes). It seems like mass renaming is in order. :-(
Thanks.
--
tejun
On Tue, 8 Feb 2011, Tejun Heo wrote:
> On Tue, Feb 08, 2011 at 10:10:46AM -0500, Alan Stern wrote:
> > On Tue, 8 Feb 2011, Tejun Heo wrote:
> >
> > > From de62c151df9b6568c67977920267392c432f340a Mon Sep 17 00:00:00 2001
> > > From: Tejun Heo <[email protected]>
> > > Date: Tue, 8 Feb 2011 10:39:03 +0100
> > >
> > > Add system wide freezeable workqueue.
> >
> > The preferred spelling is "freezable".
>
> Hmmm... workqueue always has used freezeable (even before cmwq
> changes). It seems like mass renaming is in order. :-(
The name of create_freezeable_workqueue was misspelled originally and
then never changed. But other code and most of the comments and
documentation are spelled correctly -- grep for both spellings and
you'll see.
Agreed, some renaming and editing is in order.
Alan Stern
On Tue, Feb 08, 2011 at 10:41:55AM +0100, Tejun Heo wrote:
> From de62c151df9b6568c67977920267392c432f340a Mon Sep 17 00:00:00 2001
> From: Tejun Heo <[email protected]>
> Date: Tue, 8 Feb 2011 10:39:03 +0100
>
> Add system wide freezeable workqueue.
>
> Signed-off-by: Tejun Heo <[email protected]>
> Cc: Dmitry Torokhov <[email protected]>
Acked-by: Dmitry Torokhov <[email protected]>
And if, as Alan said, it could be "freezable" that would be great.
--
Dmitry