2013-04-24 11:46:23

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V5 0/5] Queue work on power efficient wq

This patchset was called: "Create sched_select_cpu() and use it for workqueues"
for the first three versions.

Earlier discussions over v3, v2 and v1 can be found here:
https://lkml.org/lkml/2013/3/18/364
http://lists.linaro.org/pipermail/linaro-dev/2012-November/014344.html
http://www.mail-archive.com/[email protected]/msg13342.html

V4 is here:
https://lkml.org/lkml/2013/3/31/55

Workqueues can be performance or power oriented. For performance we may want to
keep them running on a single cpu, so that it remains cache hot. For power we
can give scheduler the liberty to choose target cpu for running work handler.

Later one (Power oriented WQ) can be achieved if the workqueue is allocated with
WQ_UNBOUND flag. Enabling CONFIG_WQ_POWER_EFFICIENT will set
'wq_power_efficient' to 'true'. Setting 'power_efficient' boot param will
override value of 'wq_power_efficient' variable. When 'wq_power_efficient' is
set to 'true', we will convert WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq
allocation. And so scheduler will have the liberty to choose where to run this
work.

Here we are migrating few users of workqueues to WQ_POWER_EFFICIENT. These
drivers are found to be very much active on idle or lightly busy system and
using WQ_POWER_EFFICIENT for these gave impressive results.

These would be used in power saving mode only if relevant configs are enabled
at compile time or in bootargs. Otherwise behavior is unchanged.

Setup:
-----
- ARM Vexpress TC2 - big.LITTLE CPU
- Core 0-1: A15, 2-4: A7
- rootfs: linaro-ubuntu-devel

This patchset has been tested on a big LITTLE system (heterogeneous) but is
useful for all other homogeneous systems as well. During these tests audio was
played in background using aplay.

Results:
-------

Cluster A15 Energy Cluster A7 Energy Total
------------------------- ----------------------- ------

Without this patchset (Energy in Joules):
---------------------------------------------------

0.151162 2.183545 2.334707
0.223730 2.687067 2.910797
0.289687 2.732702 3.022389
0.454198 2.745908 3.200106
0.495552 2.746465 3.242017

Average:
0.322866 2.619137 2.942003


With this patchset (Energy in Joules):
-----------------------------------------------

0.226421 2.283658 2.510079
0.151361 2.236656 2.388017
0.197726 2.249849 2.447575
0.221915 2.229446 2.451361
0.347098 2.257707 2.604805

Average:
0.2289042 2.2514632 2.4803674

Above tests are repeated multiple times and events are tracked using trace-cmd
and analysed using kernelshark. And it was easily noticeable that idle time for
many cpus has increased considerably, which eventually saved some power.

V4->V5:
-------
- Created new wq flag: WQ_POWER_EFFICIENT, config option:
CONFIG_WQ_POWER_EFFICIENT and kernel param workqueue.power_efficient.
- Created few system wide workqueues aligned towards power saving.

V3->V4:
-------
- Dropped changes to kernel/sched directory and hence
sched_select_non_idle_cpu().
- Dropped queue_work_on_any_cpu()
- Created system_freezable_unbound_wq
- Changed all patches accordingly.

V2->V3:
-------
- Dropped changes into core queue_work() API, rather create *_on_any_cpu()
APIs
- Dropped running timers migration patch as that was broken
- Migrated few users of workqueues to use *_on_any_cpu() APIs.

Viresh Kumar (5):
workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented
workqueues
workqueue: Add system wide power_efficient workqueues
PHYLIB: queue work on system_power_efficient_wq
block: queue work on power efficient wq
fbcon: queue work on power efficient wq

Documentation/kernel-parameters.txt | 17 +++++++++++++++++
block/blk-core.c | 3 ++-
block/blk-ioc.c | 3 ++-
block/genhd.c | 12 ++++++++----
drivers/net/phy/phy.c | 9 +++++----
drivers/video/console/fbcon.c | 2 +-
include/linux/workqueue.h | 10 ++++++++++
kernel/power/Kconfig | 19 +++++++++++++++++++
kernel/workqueue.c | 24 +++++++++++++++++++++++-
9 files changed, 87 insertions(+), 12 deletions(-)

--
1.7.12.rc2.18.g61b472e


2013-04-24 11:46:35

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

Workqueues can be performance or power oriented. For performance we may want to
keep them running on a single cpu, so that it remains cache hot. For power we
can give scheduler the liberty to choose target cpu for running work handler.

Later one (Power oriented WQ) can be achieved if the workqueue is allocated with
WQ_UNBOUND flag. Enabling CONFIG_WQ_POWER_EFFICIENT will set
'wq_power_efficient' to 'true'. Setting 'power_efficient' boot param will
override value of 'wq_power_efficient' variable. When 'wq_power_efficient' is
set to 'true', we will convert WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq
allocation. And so scheduler will have the liberty to choose where to run this
work.

Signed-off-by: Viresh Kumar <[email protected]>
---
Documentation/kernel-parameters.txt | 17 +++++++++++++++++
include/linux/workqueue.h | 3 +++
kernel/power/Kconfig | 19 +++++++++++++++++++
kernel/workqueue.c | 11 +++++++++++
4 files changed, 50 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index c4fa000..a9040fa 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3312,6 +3312,23 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
that this also can be controlled per-workqueue for
workqueues visible under /sys/bus/workqueue/.

+ workqueue.power_efficient
+ Workqueues can be performance or power oriented. For
+ performance we may want to keep them running on a single
+ cpu, so that it remains cache hot. For power we can give
+ scheduler the liberty to choose target cpu for running
+ work handler.
+
+ Later one (Power oriented WQ) can be achieved if the
+ workqueue is allocated with WQ_UNBOUND flag. Enabling
+ CONFIG_WQ_POWER_EFFICIENT will set 'wq_power_efficient'
+ to 'true'. Setting 'power_efficient' boot param will
+ override value of 'wq_power_efficient' variable. When
+ 'wq_power_efficient' is set to 'true', we will convert
+ WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq allocation.
+ And so scheduler will have the liberty to choose where
+ to run this work.
+
x2apic_phys [X86-64,APIC] Use x2apic physical mode instead of
default x2apic cluster mode on platforms
supporting x2apic.
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 623488f..83fa570 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -302,6 +302,9 @@ enum {
WQ_HIGHPRI = 1 << 4, /* high priority */
WQ_CPU_INTENSIVE = 1 << 5, /* cpu instensive workqueue */
WQ_SYSFS = 1 << 6, /* visible in sysfs, see wq_sysfs_register() */
+ WQ_POWER_EFFICIENT = 1 << 7, /* WQ_UNBOUND, for power
+ * saving, if wq_power_efficient is
+ * enabled. Unused otherwise. */

__WQ_DRAINING = 1 << 16, /* internal: workqueue is draining */
__WQ_ORDERED = 1 << 17, /* internal: workqueue is ordered */
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 5dfdc9e..e1e9c8b 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -263,6 +263,25 @@ config PM_GENERIC_DOMAINS
bool
depends on PM

+config WQ_POWER_EFFICIENT
+ bool "Workqueue allocated as UNBOUND (by default) for power efficiency"
+ depends on PM
+ help
+ Workqueues can be performance or power oriented. For performance we
+ may want to keep them running on a single cpu, so that it remains
+ cache hot. For power we can give scheduler the liberty to choose
+ target cpu for running work handler.
+
+ Later one (Power oriented WQ) can be achieved if the workqueue is
+ allocated with WQ_UNBOUND flag. Enabling CONFIG_WQ_POWER_EFFICIENT
+ will set 'wq_power_efficient' to 'true'. Setting 'power_efficient'
+ boot param will override value of 'wq_power_efficient' variable. When
+ 'wq_power_efficient' is set to 'true', we will convert
+ WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq allocation. And so
+ scheduler will have the liberty to choose where to run this work.
+
+ If in doubt, say N.
+
config PM_GENERIC_DOMAINS_SLEEP
def_bool y
depends on PM_SLEEP && PM_GENERIC_DOMAINS
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 4aa9f5b..a327027 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -272,6 +272,14 @@ static cpumask_var_t *wq_numa_possible_cpumask;
static bool wq_disable_numa;
module_param_named(disable_numa, wq_disable_numa, bool, 0444);

+#ifdef CONFIG_WQ_POWER_EFFICIENT
+static bool wq_power_efficient = true;
+#else
+static bool wq_power_efficient;
+#endif
+
+module_param_named(power_efficient, wq_power_efficient, bool, 0444);
+
static bool wq_numa_enabled; /* unbound NUMA affinity enabled */

/* buf for wq_update_unbound_numa_attrs(), protected by CPU hotplug exclusion */
@@ -4085,6 +4093,9 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
struct workqueue_struct *wq;
struct pool_workqueue *pwq;

+ if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient)
+ flags |= WQ_UNBOUND;
+
/* allocate wq and format name */
if (flags & WQ_UNBOUND)
tbl_size = wq_numa_tbl_len * sizeof(wq->numa_pwq_tbl[0]);
--
1.7.12.rc2.18.g61b472e

2013-04-24 11:46:45

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V5 2/5] workqueue: Add system wide power_efficient workqueues

This patch adds system wide workqueues aligned towards power saving. This is
done by allocating them with WQ_UNBOUND flag if 'wq_power_efficient' is set to
'true'.

Signed-off-by: Viresh Kumar <[email protected]>
---
include/linux/workqueue.h | 7 +++++++
kernel/workqueue.c | 13 ++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 83fa570..ac3e0e29 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -336,11 +336,18 @@ enum {
*
* system_freezable_wq is equivalent to system_wq except that it's
* freezable.
+ *
+ * *_power_efficient_wq are inclined towards saving power. They are converted
+ * into WQ_UNBOUND variants, if 'wq_power_efficient' is set to 'true'. Otherwise
+ * they are same as their sibling types. Like: system_power_efficient_wq will
+ * behave like system_wq if 'wq_power_efficient' is not set to 'true'.
*/
extern struct workqueue_struct *system_wq;
extern struct workqueue_struct *system_long_wq;
extern struct workqueue_struct *system_unbound_wq;
extern struct workqueue_struct *system_freezable_wq;
+extern struct workqueue_struct *system_power_efficient_wq;
+extern struct workqueue_struct *system_freezable_power_efficient_wq;

static inline struct workqueue_struct * __deprecated __system_nrt_wq(void)
{
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index a327027..cb1c2cc 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -313,6 +313,10 @@ struct workqueue_struct *system_unbound_wq __read_mostly;
EXPORT_SYMBOL_GPL(system_unbound_wq);
struct workqueue_struct *system_freezable_wq __read_mostly;
EXPORT_SYMBOL_GPL(system_freezable_wq);
+struct workqueue_struct *system_power_efficient_wq __read_mostly;
+EXPORT_SYMBOL_GPL(system_power_efficient_wq);
+struct workqueue_struct *system_freezable_power_efficient_wq __read_mostly;
+EXPORT_SYMBOL_GPL(system_freezable_power_efficient_wq);

static int worker_thread(void *__worker);
static void copy_workqueue_attrs(struct workqueue_attrs *to,
@@ -4985,8 +4989,15 @@ static int __init init_workqueues(void)
WQ_UNBOUND_MAX_ACTIVE);
system_freezable_wq = alloc_workqueue("events_freezable",
WQ_FREEZABLE, 0);
+ system_power_efficient_wq = alloc_workqueue("events_power_efficient",
+ WQ_POWER_EFFICIENT, 0);
+ system_freezable_power_efficient_wq = alloc_workqueue("events_freezable_power_efficient",
+ WQ_FREEZABLE | WQ_POWER_EFFICIENT,
+ 0);
BUG_ON(!system_wq || !system_highpri_wq || !system_long_wq ||
- !system_unbound_wq || !system_freezable_wq);
+ !system_unbound_wq || !system_freezable_wq ||
+ !system_power_efficient_wq ||
+ !system_freezable_power_efficient_wq);
return 0;
}
early_initcall(init_workqueues);
--
1.7.12.rc2.18.g61b472e

2013-04-24 11:46:55

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V5 3/5] PHYLIB: queue work on system_power_efficient_wq

Phylib uses workqueues for multiple purposes. There is no real dependency of
scheduling these on the cpu which scheduled them.

On a idle system, it is observed that and idle cpu wakes up many times just to
service this work. It would be better if we can schedule it on a cpu which the
scheduler believes to be the most appropriate one.

This patch replaces system_wq with system_power_efficient_wq for PHYLIB.

Cc: David S. Miller <[email protected]>
Cc: [email protected]
Signed-off-by: Viresh Kumar <[email protected]>
Acked-by: David S. Miller <[email protected]>
---
drivers/net/phy/phy.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c14f147..984c0b5 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -439,7 +439,7 @@ void phy_start_machine(struct phy_device *phydev,
{
phydev->adjust_state = handler;

- schedule_delayed_work(&phydev->state_queue, HZ);
+ queue_delayed_work(system_power_efficient_wq, &phydev->state_queue, HZ);
}

/**
@@ -500,7 +500,7 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
disable_irq_nosync(irq);
atomic_inc(&phydev->irq_disable);

- schedule_work(&phydev->phy_queue);
+ queue_work(system_power_efficient_wq, &phydev->phy_queue);

return IRQ_HANDLED;
}
@@ -655,7 +655,7 @@ static void phy_change(struct work_struct *work)

/* reschedule state queue work to run as soon as possible */
cancel_delayed_work_sync(&phydev->state_queue);
- schedule_delayed_work(&phydev->state_queue, 0);
+ queue_delayed_work(system_power_efficient_wq, &phydev->state_queue, 0);

return;

@@ -918,7 +918,8 @@ void phy_state_machine(struct work_struct *work)
if (err < 0)
phy_error(phydev);

- schedule_delayed_work(&phydev->state_queue, PHY_STATE_TIME * HZ);
+ queue_delayed_work(system_power_efficient_wq, &phydev->state_queue,
+ PHY_STATE_TIME * HZ);
}

static inline void mmd_phy_indirect(struct mii_bus *bus, int prtad, int devad,
--
1.7.12.rc2.18.g61b472e

2013-04-24 11:47:06

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V5 4/5] block: queue work on power efficient wq

Block layer uses workqueues for multiple purposes. There is no real dependency
of scheduling these on the cpu which scheduled them.

On a idle system, it is observed that and idle cpu wakes up many times just to
service this work. It would be better if we can schedule it on a cpu which the
scheduler believes to be the most appropriate one.

This patch replaces normal workqueues with power efficient versions.

Cc: Jens Axboe <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
block/blk-core.c | 3 ++-
block/blk-ioc.c | 3 ++-
block/genhd.c | 12 ++++++++----
3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 94aa4e7..3eb9870 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -3187,7 +3187,8 @@ int __init blk_dev_init(void)

/* used for unplugging and affects IO latency/throughput - HIGHPRI */
kblockd_workqueue = alloc_workqueue("kblockd",
- WQ_MEM_RECLAIM | WQ_HIGHPRI, 0);
+ WQ_MEM_RECLAIM | WQ_HIGHPRI |
+ WQ_POWER_EFFICIENT, 0);
if (!kblockd_workqueue)
panic("Failed to create kblockd\n");

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 9c4bb82..4464c82 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -144,7 +144,8 @@ void put_io_context(struct io_context *ioc)
if (atomic_long_dec_and_test(&ioc->refcount)) {
spin_lock_irqsave(&ioc->lock, flags);
if (!hlist_empty(&ioc->icq_list))
- schedule_work(&ioc->release_work);
+ queue_work(system_power_efficient_wq,
+ &ioc->release_work);
else
free_ioc = true;
spin_unlock_irqrestore(&ioc->lock, flags);
diff --git a/block/genhd.c b/block/genhd.c
index 5a9f893..2e1cfe3 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1489,9 +1489,11 @@ static void __disk_unblock_events(struct gendisk *disk, bool check_now)
intv = disk_events_poll_jiffies(disk);
set_timer_slack(&ev->dwork.timer, intv / 4);
if (check_now)
- queue_delayed_work(system_freezable_wq, &ev->dwork, 0);
+ queue_delayed_work(system_freezable_power_efficient_wq,
+ &ev->dwork, 0);
else if (intv)
- queue_delayed_work(system_freezable_wq, &ev->dwork, intv);
+ queue_delayed_work(system_freezable_power_efficient_wq,
+ &ev->dwork, intv);
out_unlock:
spin_unlock_irqrestore(&ev->lock, flags);
}
@@ -1534,7 +1536,8 @@ void disk_flush_events(struct gendisk *disk, unsigned int mask)
spin_lock_irq(&ev->lock);
ev->clearing |= mask;
if (!ev->block)
- mod_delayed_work(system_freezable_wq, &ev->dwork, 0);
+ mod_delayed_work(system_freezable_power_efficient_wq,
+ &ev->dwork, 0);
spin_unlock_irq(&ev->lock);
}

@@ -1627,7 +1630,8 @@ static void disk_check_events(struct disk_events *ev,

intv = disk_events_poll_jiffies(disk);
if (!ev->block && intv)
- queue_delayed_work(system_freezable_wq, &ev->dwork, intv);
+ queue_delayed_work(system_freezable_power_efficient_wq,
+ &ev->dwork, intv);

spin_unlock_irq(&ev->lock);

--
1.7.12.rc2.18.g61b472e

2013-04-24 11:47:17

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V5 5/5] fbcon: queue work on power efficient wq

fbcon uses workqueues and it has no real dependency of scheduling these on the
cpu which scheduled them.

On a idle system, it is observed that and idle cpu wakes up many times just to
service this work. It would be better if we can schedule it on a cpu which the
scheduler believes to be the most appropriate one.

This patch replaces system_wq with system_power_efficient_wq.

Cc: Dave Airlie <[email protected]>
Cc: [email protected]
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/video/console/fbcon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 3cd6759..0cae83d 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -404,7 +404,7 @@ static void cursor_timer_handler(unsigned long dev_addr)
struct fb_info *info = (struct fb_info *) dev_addr;
struct fbcon_ops *ops = info->fbcon_par;

- schedule_work(&info->queue);
+ queue_work(system_power_efficient_wq, &info->queue);
mod_timer(&ops->cursor_timer, jiffies + HZ/5);
}

--
1.7.12.rc2.18.g61b472e

2013-04-24 12:20:07

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

On Wed, Apr 24, 2013 at 5:12 PM, Viresh Kumar <[email protected]> wrote:
> Workqueues can be performance or power oriented. For performance we may want to
> keep them running on a single cpu, so that it remains cache hot. For power we
> can give scheduler the liberty to choose target cpu for running work handler.

Consider the following rewording:

Workqueues can be performance or power-oriented. Currently, most
workqueues are bound to the CPU they were created on. This gives good
performance (due to cache effects) at the cost of potentially waking
up otherwise idle cores just to process some work. To save power, we
can allow the work to be rescheduled on a core that is already awake.

> Later one (Power oriented WQ) can be achieved if the workqueue is allocated with
> WQ_UNBOUND flag. Enabling CONFIG_WQ_POWER_EFFICIENT will set
> 'wq_power_efficient' to 'true'. Setting 'power_efficient' boot param will
> override value of 'wq_power_efficient' variable. When 'wq_power_efficient' is
> set to 'true', we will convert WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq
> allocation. And so scheduler will have the liberty to choose where to run this
> work.

Consider the following rewording which is easier to understand IMO:

Workqueues created with the WQ_UNBOUND flag will allow some power
savings. However, we don't change the default behaviour of the system.
To enable power-saving behaviour, a new config option
CONFIG_WQ_POWER_EFFICIENT needs to be turned on. This option can also
be overridden by the workqueue.power_efficient boot parameter.



> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> Documentation/kernel-parameters.txt | 17 +++++++++++++++++
> include/linux/workqueue.h | 3 +++
> kernel/power/Kconfig | 19 +++++++++++++++++++
> kernel/workqueue.c | 11 +++++++++++
> 4 files changed, 50 insertions(+)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index c4fa000..a9040fa 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3312,6 +3312,23 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> that this also can be controlled per-workqueue for
> workqueues visible under /sys/bus/workqueue/.
>
> + workqueue.power_efficient
> + Workqueues can be performance or power oriented. For
> + performance we may want to keep them running on a single
> + cpu, so that it remains cache hot. For power we can give
> + scheduler the liberty to choose target cpu for running
> + work handler.
> +
> + Later one (Power oriented WQ) can be achieved if the
> + workqueue is allocated with WQ_UNBOUND flag. Enabling
> + CONFIG_WQ_POWER_EFFICIENT will set 'wq_power_efficient'
> + to 'true'. Setting 'power_efficient' boot param will
> + override value of 'wq_power_efficient' variable. When
> + 'wq_power_efficient' is set to 'true', we will convert
> + WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq allocation.
> + And so scheduler will have the liberty to choose where
> + to run this work.
> +

Rewrite as above.

> x2apic_phys [X86-64,APIC] Use x2apic physical mode instead of
> default x2apic cluster mode on platforms
> supporting x2apic.
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index 623488f..83fa570 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -302,6 +302,9 @@ enum {
> WQ_HIGHPRI = 1 << 4, /* high priority */
> WQ_CPU_INTENSIVE = 1 << 5, /* cpu instensive workqueue */
> WQ_SYSFS = 1 << 6, /* visible in sysfs, see wq_sysfs_register() */
> + WQ_POWER_EFFICIENT = 1 << 7, /* WQ_UNBOUND, for power
> + * saving, if wq_power_efficient is
> + * enabled. Unused otherwise. */
>
> __WQ_DRAINING = 1 << 16, /* internal: workqueue is draining */
> __WQ_ORDERED = 1 << 17, /* internal: workqueue is ordered */
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 5dfdc9e..e1e9c8b 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -263,6 +263,25 @@ config PM_GENERIC_DOMAINS
> bool
> depends on PM
>
> +config WQ_POWER_EFFICIENT
> + bool "Workqueue allocated as UNBOUND (by default) for power efficiency"
> + depends on PM

default n

> + help
> + Workqueues can be performance or power oriented. For performance we
> + may want to keep them running on a single cpu, so that it remains
> + cache hot. For power we can give scheduler the liberty to choose
> + target cpu for running work handler.
> +
> + Later one (Power oriented WQ) can be achieved if the workqueue is
> + allocated with WQ_UNBOUND flag. Enabling CONFIG_WQ_POWER_EFFICIENT
> + will set 'wq_power_efficient' to 'true'. Setting 'power_efficient'
> + boot param will override value of 'wq_power_efficient' variable. When
> + 'wq_power_efficient' is set to 'true', we will convert
> + WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq allocation. And so
> + scheduler will have the liberty to choose where to run this work.
> +
> + If in doubt, say N.
> +

Rewrite as above

> config PM_GENERIC_DOMAINS_SLEEP
> def_bool y
> depends on PM_SLEEP && PM_GENERIC_DOMAINS
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 4aa9f5b..a327027 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -272,6 +272,14 @@ static cpumask_var_t *wq_numa_possible_cpumask;
> static bool wq_disable_numa;
> module_param_named(disable_numa, wq_disable_numa, bool, 0444);
>
> +#ifdef CONFIG_WQ_POWER_EFFICIENT
> +static bool wq_power_efficient = true;
> +#else
> +static bool wq_power_efficient;
> +#endif
> +
> +module_param_named(power_efficient, wq_power_efficient, bool, 0444);
> +
> static bool wq_numa_enabled; /* unbound NUMA affinity enabled */
>
> /* buf for wq_update_unbound_numa_attrs(), protected by CPU hotplug exclusion */
> @@ -4085,6 +4093,9 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
> struct workqueue_struct *wq;
> struct pool_workqueue *pwq;
>
> + if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient)
> + flags |= WQ_UNBOUND;
> +
> /* allocate wq and format name */
> if (flags & WQ_UNBOUND)
> tbl_size = wq_numa_tbl_len * sizeof(wq->numa_pwq_tbl[0]);
> --
> 1.7.12.rc2.18.g61b472e
>
>
> _______________________________________________
> linaro-kernel mailing list
> [email protected]
> http://lists.linaro.org/mailman/listinfo/linaro-kernel

2013-04-24 12:27:13

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

On 24 April 2013 17:50, Amit Kucheria <[email protected]> wrote:
> On Wed, Apr 24, 2013 at 5:12 PM, Viresh Kumar <[email protected]> wrote:
>> +config WQ_POWER_EFFICIENT
>> + bool "Workqueue allocated as UNBOUND (by default) for power efficiency"
>> + depends on PM
>
> default n

If this line is not present i believe it is 'default n' only. So, i skipped it.

All others comments accepted.

2013-04-24 16:12:19

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

On Wed, Apr 24, 2013 at 05:57:09PM +0530, Viresh Kumar wrote:
> If this line is not present i believe it is 'default n' only. So, i skipped it.

I think the difference is that when you configure the option for the
first time - ie. building kernel with .config used for an older kernel
- that it prompts for selection, thus annoying the big penguin. You
probably want the explicit 'N'.

Thanks.

--
tejun

2013-04-25 03:43:46

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

On 25 April 2013 09:00, Amit Kucheria <[email protected]> wrote:
> Yes. That was my intention - preventing a prompt on existing defconfigs and
> there by maintaining current behavior.

Hmm... Following is the version after fixing all problems you reported.
@Tejun: I have attached it too as gmail's copy-paste may break it. Please
consider applying this series if it looks fine to you.


---------------x----------------x---------------------

From: Viresh Kumar <[email protected]>
Date: Mon, 8 Apr 2013 16:45:40 +0530
Subject: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for
power oriented workqueues

Workqueues can be performance or power-oriented. Currently, most workqueues are
bound to the CPU they were created on. This gives good performance (due to cache
effects) at the cost of potentially waking up otherwise idle cores just to
process some work. To save power, we can allow the work to be rescheduled on a
core that is already awake.

Workqueues created with the WQ_UNBOUND flag will allow some power savings.
However, we don't change the default behaviour of the system. To enable
power-saving behaviour, a new config option CONFIG_WQ_POWER_EFFICIENT needs to
be turned on. This option can also be overridden by the
workqueue.power_efficient boot parameter.

Signed-off-by: Viresh Kumar <[email protected]>
---
Documentation/kernel-parameters.txt | 17 +++++++++++++++++
include/linux/workqueue.h | 3 +++
kernel/power/Kconfig | 19 +++++++++++++++++++
kernel/workqueue.c | 11 +++++++++++
4 files changed, 50 insertions(+)

diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index c4fa000..22edc83 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3312,6 +3312,23 @@ bytes respectively. Such letter suffixes can
also be entirely omitted.
that this also can be controlled per-workqueue for
workqueues visible under /sys/bus/workqueue/.

+ workqueue.power_efficient
+ Workqueues can be performance or power-oriented.
+ Currently, most workqueues are bound to the CPU they
+ were created on. This gives good performance (due to
+ cache effects) at the cost of potentially waking up
+ otherwise idle cores just to process some work. To save
+ power, we can allow the work to be rescheduled on a core
+ that is already awake.
+
+ Workqueues created with the WQ_UNBOUND flag will allow
+ some power savings. However, we don't change the
+ default behaviour of the system. To enable power-saving
+ behaviour, a new config option CONFIG_WQ_POWER_EFFICIENT
+ needs to be turned on. This option can also be
+ overridden by the workqueue.power_efficient boot
+ parameter.
+
x2apic_phys [X86-64,APIC] Use x2apic physical mode instead of
default x2apic cluster mode on platforms
supporting x2apic.
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 623488f..83fa570 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -302,6 +302,9 @@ enum {
WQ_HIGHPRI = 1 << 4, /* high priority */
WQ_CPU_INTENSIVE = 1 << 5, /* cpu instensive workqueue */
WQ_SYSFS = 1 << 6, /* visible in sysfs, see wq_sysfs_register() */
+ WQ_POWER_EFFICIENT = 1 << 7, /* WQ_UNBOUND, for power
+ * saving, if wq_power_efficient is
+ * enabled. Unused otherwise. */

__WQ_DRAINING = 1 << 16, /* internal: workqueue is draining */
__WQ_ORDERED = 1 << 17, /* internal: workqueue is ordered */
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 5dfdc9e..018f039 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -263,6 +263,25 @@ config PM_GENERIC_DOMAINS
bool
depends on PM

+config WQ_POWER_EFFICIENT
+ bool "Workqueue allocated as UNBOUND (by default) for power efficiency"
+ depends on PM
+ default n
+ help
+ Workqueues can be performance or power-oriented. Currently, most
+ workqueues are bound to the CPU they were created on. This gives good
+ performance (due to cache effects) at the cost of potentially waking
+ up otherwise idle cores just to process some work. To save power, we
+ can allow the work to be rescheduled on a core that is already awake.
+
+ Workqueues created with the WQ_UNBOUND flag will allow some power
+ savings. However, we don't change the default behaviour of the
+ system. To enable power-saving behaviour, a new config option
+ CONFIG_WQ_POWER_EFFICIENT needs to be turned on. This option can also
+ be overridden by the workqueue.power_efficient boot parameter.
+
+ If in doubt, say N.
+
config PM_GENERIC_DOMAINS_SLEEP
def_bool y
depends on PM_SLEEP && PM_GENERIC_DOMAINS
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 4aa9f5b..a327027 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -272,6 +272,14 @@ static cpumask_var_t *wq_numa_possible_cpumask;
static bool wq_disable_numa;
module_param_named(disable_numa, wq_disable_numa, bool, 0444);

+#ifdef CONFIG_WQ_POWER_EFFICIENT
+static bool wq_power_efficient = true;
+#else
+static bool wq_power_efficient;
+#endif
+
+module_param_named(power_efficient, wq_power_efficient, bool, 0444);
+
static bool wq_numa_enabled; /* unbound NUMA affinity enabled */

/* buf for wq_update_unbound_numa_attrs(), protected by CPU hotplug
exclusion */
@@ -4085,6 +4093,9 @@ struct workqueue_struct
*__alloc_workqueue_key(const char *fmt,
struct workqueue_struct *wq;
struct pool_workqueue *pwq;

+ if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient)
+ flags |= WQ_UNBOUND;
+
/* allocate wq and format name */
if (flags & WQ_UNBOUND)
tbl_size = wq_numa_tbl_len * sizeof(wq->numa_pwq_tbl[0]);


Attachments:
0001-workqueues-Introduce-new-flag-WQ_POWER_EFFICIENT-for.patch (5.34 kB)

2013-04-25 11:13:55

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

On Thu, Apr 25, 2013 at 9:13 AM, Viresh Kumar <[email protected]> wrote:
> On 25 April 2013 09:00, Amit Kucheria <[email protected]> wrote:
>> Yes. That was my intention - preventing a prompt on existing defconfigs and
>> there by maintaining current behavior.
>
> Hmm... Following is the version after fixing all problems you reported.
> @Tejun: I have attached it too as gmail's copy-paste may break it. Please
> consider applying this series if it looks fine to you.
>
>
> ---------------x----------------x---------------------
>
> From: Viresh Kumar <[email protected]>
> Date: Mon, 8 Apr 2013 16:45:40 +0530
> Subject: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for
> power oriented workqueues
>
> Workqueues can be performance or power-oriented. Currently, most workqueues are
> bound to the CPU they were created on. This gives good performance (due to cache
> effects) at the cost of potentially waking up otherwise idle cores just to
> process some work. To save power, we can allow the work to be rescheduled on a
> core that is already awake.
>
> Workqueues created with the WQ_UNBOUND flag will allow some power savings.
> However, we don't change the default behaviour of the system. To enable
> power-saving behaviour, a new config option CONFIG_WQ_POWER_EFFICIENT needs to
> be turned on. This option can also be overridden by the
> workqueue.power_efficient boot parameter.
>
> Signed-off-by: Viresh Kumar <[email protected]>

For the series, you can add my:

Reviewed-by: Amit Kucheria <[email protected]>

> ---
> Documentation/kernel-parameters.txt | 17 +++++++++++++++++
> include/linux/workqueue.h | 3 +++
> kernel/power/Kconfig | 19 +++++++++++++++++++
> kernel/workqueue.c | 11 +++++++++++
> 4 files changed, 50 insertions(+)
>
> diff --git a/Documentation/kernel-parameters.txt
> b/Documentation/kernel-parameters.txt
> index c4fa000..22edc83 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3312,6 +3312,23 @@ bytes respectively. Such letter suffixes can
> also be entirely omitted.
> that this also can be controlled per-workqueue for
> workqueues visible under /sys/bus/workqueue/.
>
> + workqueue.power_efficient
> + Workqueues can be performance or power-oriented.
> + Currently, most workqueues are bound to the CPU they
> + were created on. This gives good performance (due to
> + cache effects) at the cost of potentially waking up
> + otherwise idle cores just to process some work. To save
> + power, we can allow the work to be rescheduled on a core
> + that is already awake.
> +
> + Workqueues created with the WQ_UNBOUND flag will allow
> + some power savings. However, we don't change the
> + default behaviour of the system. To enable power-saving
> + behaviour, a new config option CONFIG_WQ_POWER_EFFICIENT
> + needs to be turned on. This option can also be
> + overridden by the workqueue.power_efficient boot
> + parameter.
> +
> x2apic_phys [X86-64,APIC] Use x2apic physical mode instead of
> default x2apic cluster mode on platforms
> supporting x2apic.
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index 623488f..83fa570 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -302,6 +302,9 @@ enum {
> WQ_HIGHPRI = 1 << 4, /* high priority */
> WQ_CPU_INTENSIVE = 1 << 5, /* cpu instensive workqueue */
> WQ_SYSFS = 1 << 6, /* visible in sysfs, see wq_sysfs_register() */
> + WQ_POWER_EFFICIENT = 1 << 7, /* WQ_UNBOUND, for power
> + * saving, if wq_power_efficient is
> + * enabled. Unused otherwise. */
>
> __WQ_DRAINING = 1 << 16, /* internal: workqueue is draining */
> __WQ_ORDERED = 1 << 17, /* internal: workqueue is ordered */
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 5dfdc9e..018f039 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -263,6 +263,25 @@ config PM_GENERIC_DOMAINS
> bool
> depends on PM
>
> +config WQ_POWER_EFFICIENT
> + bool "Workqueue allocated as UNBOUND (by default) for power efficiency"
> + depends on PM
> + default n
> + help
> + Workqueues can be performance or power-oriented. Currently, most
> + workqueues are bound to the CPU they were created on. This gives good
> + performance (due to cache effects) at the cost of potentially waking
> + up otherwise idle cores just to process some work. To save power, we
> + can allow the work to be rescheduled on a core that is already awake.
> +
> + Workqueues created with the WQ_UNBOUND flag will allow some power
> + savings. However, we don't change the default behaviour of the
> + system. To enable power-saving behaviour, a new config option
> + CONFIG_WQ_POWER_EFFICIENT needs to be turned on. This option can also
> + be overridden by the workqueue.power_efficient boot parameter.
> +
> + If in doubt, say N.
> +
> config PM_GENERIC_DOMAINS_SLEEP
> def_bool y
> depends on PM_SLEEP && PM_GENERIC_DOMAINS
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 4aa9f5b..a327027 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -272,6 +272,14 @@ static cpumask_var_t *wq_numa_possible_cpumask;
> static bool wq_disable_numa;
> module_param_named(disable_numa, wq_disable_numa, bool, 0444);
>
> +#ifdef CONFIG_WQ_POWER_EFFICIENT
> +static bool wq_power_efficient = true;
> +#else
> +static bool wq_power_efficient;
> +#endif
> +
> +module_param_named(power_efficient, wq_power_efficient, bool, 0444);
> +
> static bool wq_numa_enabled; /* unbound NUMA affinity enabled */
>
> /* buf for wq_update_unbound_numa_attrs(), protected by CPU hotplug
> exclusion */
> @@ -4085,6 +4093,9 @@ struct workqueue_struct
> *__alloc_workqueue_key(const char *fmt,
> struct workqueue_struct *wq;
> struct pool_workqueue *pwq;
>
> + if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient)
> + flags |= WQ_UNBOUND;
> +
> /* allocate wq and format name */
> if (flags & WQ_UNBOUND)
> tbl_size = wq_numa_tbl_len * sizeof(wq->numa_pwq_tbl[0]);

2013-04-25 11:15:26

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

On 25 April 2013 16:43, Amit Kucheria <[email protected]> wrote:
> On Thu, Apr 25, 2013 at 9:13 AM, Viresh Kumar <[email protected]> wrote:
>> On 25 April 2013 09:00, Amit Kucheria <[email protected]> wrote:
>>> Yes. That was my intention - preventing a prompt on existing defconfigs and
>>> there by maintaining current behavior.
>>
>> Hmm... Following is the version after fixing all problems you reported.
>> @Tejun: I have attached it too as gmail's copy-paste may break it. Please
>> consider applying this series if it looks fine to you.
>>
>>
>> ---------------x----------------x---------------------
>>
>> From: Viresh Kumar <[email protected]>
>> Date: Mon, 8 Apr 2013 16:45:40 +0530
>> Subject: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for
>> power oriented workqueues
>>
>> Workqueues can be performance or power-oriented. Currently, most workqueues are
>> bound to the CPU they were created on. This gives good performance (due to cache
>> effects) at the cost of potentially waking up otherwise idle cores just to
>> process some work. To save power, we can allow the work to be rescheduled on a
>> core that is already awake.
>>
>> Workqueues created with the WQ_UNBOUND flag will allow some power savings.
>> However, we don't change the default behaviour of the system. To enable
>> power-saving behaviour, a new config option CONFIG_WQ_POWER_EFFICIENT needs to
>> be turned on. This option can also be overridden by the
>> workqueue.power_efficient boot parameter.
>>
>> Signed-off-by: Viresh Kumar <[email protected]>
>
> For the series, you can add my:
>
> Reviewed-by: Amit Kucheria <[email protected]>

Thanks.

2013-04-26 19:11:26

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

Hey, Viresh.

It's already too late for the upcoming merge window, but things
generally look good to me and I'll apply the patchset once wq/for-3.11
opens. One nitpick tho.

On Thu, Apr 25, 2013 at 09:13:44AM +0530, Viresh Kumar wrote:
> + workqueue.power_efficient
> + Workqueues can be performance or power-oriented.
> + Currently, most workqueues are bound to the CPU they
^^^^
per-cpu would be better

> + were created on. This gives good performance (due to
> + cache effects) at the cost of potentially waking up
> + otherwise idle cores just to process some work. To save
> + power, we can allow the work to be rescheduled on a core
> + that is already awake.

The above description is confusing to me. As have been discussed
multiple times before, per-cpu workqueue in itself doesn't wake up the
CPU physically. The timer may but per-cpu workqueue doesn't. It was
confusing when this patchset was first posted and the above phrasing
is still confusing. What the patchset tries to do is preventing the
scheduler from perceiving the CPU as active due to the activated
worker thread pinned to that CPU, right? The knob doesn't really do
anything about waking up the processor in itself. It just avoids
feeding the scheduler with noisy activation events and allows it to
allocate work item execution according to the scheduler's view of CPU
active/idleness. As the scheduler has longer / larger scope of
overall CPU activities and means to regulate them, this leads to more
power-efficient allocation of work item executions, right? It'd be
really great if the descriptions and the comment above the flag makes
this abundantly clear because it's not something too apparent.

Thanks.

--
tejun

2013-04-29 06:36:31

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

Hey Tejun,

On 27 April 2013 00:41, Tejun Heo <[email protected]> wrote:
> Hey, Viresh.
>
> It's already too late for the upcoming merge window, but things
> generally look good to me and I'll apply the patchset once wq/for-3.11
> opens. One nitpick tho.

Obviously. I understand this and agree with you on it. It should go in 3.11 now.

> On Thu, Apr 25, 2013 at 09:13:44AM +0530, Viresh Kumar wrote:
>> + workqueue.power_efficient
>> + Workqueues can be performance or power-oriented.
>> + Currently, most workqueues are bound to the CPU they
> ^^^^
> per-cpu would be better
>
>> + were created on. This gives good performance (due to
>> + cache effects) at the cost of potentially waking up
>> + otherwise idle cores just to process some work. To save
>> + power, we can allow the work to be rescheduled on a core
>> + that is already awake.
>
> The above description is confusing to me. As have been discussed
> multiple times before, per-cpu workqueue in itself doesn't wake up the
> CPU physically. The timer may but per-cpu workqueue doesn't. It was
> confusing when this patchset was first posted and the above phrasing
> is still confusing. What the patchset tries to do is preventing the
> scheduler from perceiving the CPU as active due to the activated
> worker thread pinned to that CPU, right? The knob doesn't really do
> anything about waking up the processor in itself. It just avoids
> feeding the scheduler with noisy activation events and allows it to
> allocate work item execution according to the scheduler's view of CPU
> active/idleness. As the scheduler has longer / larger scope of
> overall CPU activities and means to regulate them, this leads to more
> power-efficient allocation of work item executions, right? It'd be
> really great if the descriptions and the comment above the flag makes
> this abundantly clear because it's not something too apparent.

Whatever you wrote above confused me even more :)
This is what i had in my mind until now. Its not about per-cpu workqueue.

Lets take example of system_wq. It doesn't have WQ_UNBOUND flag set.
Now if we call queue_work_on() with cpu x and sytem_wq, then work
will execute on cpu x. If we call queue_work() then it will queue the work
on local cpu.

At this time local cpu may be busy or idle (Atleast according to scheduler).
We don't want a idle cpu (From schedulers perspective) to be used for
running this work's handler due to two reasons.
- idle cpu may be in WFI or deeper idle states and so we can avoid waking
it up.
- We will make idle cpu look busy and so other kernel stuff may be scheduled
on it now. But we could have kept it idle for a long time.

And what timer are you talking about? I am not talking about deffered work only,
but normal work too.

I might have wrongly phrased some part of my patch (maybe used workqueue
instead of work), will fix that up.

--
viresh

2013-04-29 16:19:26

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

Hello,

On Mon, Apr 29, 2013 at 12:06:28PM +0530, Viresh Kumar wrote:
> Whatever you wrote above confused me even more :)

Heh heh, confumageddon!!!!

> This is what i had in my mind until now. Its not about per-cpu workqueue.
>
> Lets take example of system_wq. It doesn't have WQ_UNBOUND flag set.
> Now if we call queue_work_on() with cpu x and sytem_wq, then work
> will execute on cpu x. If we call queue_work() then it will queue the work
> on local cpu.

Yeap, !WQ_UNBOUND workqueues == per-cpu workqueues.

> At this time local cpu may be busy or idle (Atleast according to scheduler).
> We don't want a idle cpu (From schedulers perspective) to be used for
> running this work's handler due to two reasons.
> - idle cpu may be in WFI or deeper idle states and so we can avoid waking
> it up.

I have no idea what WFI is but the physical CPU is already awake at
that time. It can't be idle - it's running queue_work(). It could be
running in lower freq tho, which each code piece doesn't really have
much control over.

> - We will make idle cpu look busy and so other kernel stuff may be scheduled
> on it now. But we could have kept it idle for a long time.

Hmmm... yeah, about the same thing I wrote, it's not really about not
waking up the CPU right now physically but avoiding forcing the
scheduler scheduling a pinned task on an otherwise quiescent CPU.
This effectively allows the scheduler to migrate such work items
towards a CPU which the scheduler considers to be better (in power or
whatever) leading to noticeable powersave.

> And what timer are you talking about? I am not talking about deffered work only,
> but normal work too.

Deferred work item == timer + work item.

> I might have wrongly phrased some part of my patch (maybe used workqueue
> instead of work), will fix that up.

I think it'd be necessary to distinguish the physical CPU being idle
and the scheduler considers it to be idle (no task to schedule on it)
and explain how increasing the latter can lead to powersave. As it's
currently written, it seemingly, to me anyway, suggests that the
proposed change somehow avoids waking up actually idle CPU, which
isn't the case as queue_work() *always* schedules on the local CPU.
The local CPU can't be idle by definition.

Thanks.

--
tejun

2013-04-29 16:42:49

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

On 29 April 2013 21:49, Tejun Heo <[email protected]> wrote:
> On Mon, Apr 29, 2013 at 12:06:28PM +0530, Viresh Kumar wrote:
> Yeap, !WQ_UNBOUND workqueues == per-cpu workqueues.

Sigh!! You were talking about thread per cpu here... Sorry for missing
it earlier :(

>> At this time local cpu may be busy or idle (Atleast according to scheduler).
>> We don't want a idle cpu (From schedulers perspective) to be used for
>> running this work's handler due to two reasons.
>> - idle cpu may be in WFI or deeper idle states and so we can avoid waking
>> it up.
>
> I have no idea what WFI is but the physical CPU is already awake at
> that time. It can't be idle - it's running queue_work(). It could be
> running in lower freq tho, which each code piece doesn't really have
> much control over.

Stupid point. WFI: Wait for interrupt (low power mode of cpu).

>> - We will make idle cpu look busy and so other kernel stuff may be scheduled
>> on it now. But we could have kept it idle for a long time.
>
> Hmmm... yeah, about the same thing I wrote, it's not really about not
> waking up the CPU right now physically but avoiding forcing the
> scheduler scheduling a pinned task on an otherwise quiescent CPU.
> This effectively allows the scheduler to migrate such work items
> towards a CPU which the scheduler considers to be better (in power or
> whatever) leading to noticeable powersave.

Correct.

>> And what timer are you talking about? I am not talking about deffered work only,
>> but normal work too.
>
> Deferred work item == timer + work item.

Ya, i knew that :)

>> I might have wrongly phrased some part of my patch (maybe used workqueue
>> instead of work), will fix that up.
>
> I think it'd be necessary to distinguish the physical CPU being idle
> and the scheduler considers it to be idle (no task to schedule on it)
> and explain how increasing the latter can lead to powersave. As it's
> currently written, it seemingly, to me anyway, suggests that the
> proposed change somehow avoids waking up actually idle CPU, which
> isn't the case as queue_work() *always* schedules on the local CPU.
> The local CPU can't be idle by definition.

Yes you are correct. I will fix it.

2013-05-13 08:29:36

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

On 27 April 2013 00:41, Tejun Heo <[email protected]> wrote:
>
> It's already too late for the upcoming merge window, but things
> generally look good to me and I'll apply the patchset once wq/for-3.11
> opens. One nitpick tho.
>
> On Thu, Apr 25, 2013 at 09:13:44AM +0530, Viresh Kumar wrote:
>> + workqueue.power_efficient
>> + Workqueues can be performance or power-oriented.
>> + Currently, most workqueues are bound to the CPU they
> ^^^^
> per-cpu would be better
>
>> + were created on. This gives good performance (due to
>> + cache effects) at the cost of potentially waking up
>> + otherwise idle cores just to process some work. To save
>> + power, we can allow the work to be rescheduled on a core
>> + that is already awake.
>
> The above description is confusing to me. As have been discussed
> multiple times before, per-cpu workqueue in itself doesn't wake up the
> CPU physically. The timer may but per-cpu workqueue doesn't. It was
> confusing when this patchset was first posted and the above phrasing
> is still confusing. What the patchset tries to do is preventing the
> scheduler from perceiving the CPU as active due to the activated
> worker thread pinned to that CPU, right? The knob doesn't really do
> anything about waking up the processor in itself. It just avoids
> feeding the scheduler with noisy activation events and allows it to
> allocate work item execution according to the scheduler's view of CPU
> active/idleness. As the scheduler has longer / larger scope of
> overall CPU activities and means to regulate them, this leads to more
> power-efficient allocation of work item executions, right? It'd be
> really great if the descriptions and the comment above the flag makes
> this abundantly clear because it's not something too apparent.

Sorry for the long delay for such a small change. I went on long leaves..

I have added following to make things more clear at places:
(Idle from scheduler's perspective. Which may or may not be physically idle)..

Let me know if it is still unclear..

And this is the new patch: (Attached it too for applying cleanly)

---------x---------------x-------------------

From: Viresh Kumar <[email protected]>
Date: Mon, 8 Apr 2013 16:45:40 +0530
Subject: [PATCH V5 resent 1/5] workqueues: Introduce new flag
WQ_POWER_EFFICIENT for power oriented workqueues

Workqueues can be performance or power-oriented. Currently, most workqueues are
bound to the CPU they were created on. This gives good performance (due to cache
effects) at the cost of potentially waking up otherwise idle cores (Idle from
scheduler's perspective. Which may or may not be physically idle) just to
process some work. To save power, we can allow the work to be rescheduled on a
core that is already awake.

Workqueues created with the WQ_UNBOUND flag will allow some power savings.
However, we don't change the default behaviour of the system. To enable
power-saving behaviour, a new config option CONFIG_WQ_POWER_EFFICIENT needs to
be turned on. This option can also be overridden by the
workqueue.power_efficient boot parameter.

Signed-off-by: Viresh Kumar <[email protected]>
Reviewed-by: Amit Kucheria <[email protected]>
---
Documentation/kernel-parameters.txt | 18 ++++++++++++++++++
include/linux/workqueue.h | 3 +++
kernel/power/Kconfig | 21 +++++++++++++++++++++
kernel/workqueue.c | 11 +++++++++++
4 files changed, 53 insertions(+)

diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index c3bfacb..9a991b6 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3320,6 +3320,24 @@ bytes respectively. Such letter suffixes can
also be entirely omitted.
that this also can be controlled per-workqueue for
workqueues visible under /sys/bus/workqueue/.

+ workqueue.power_efficient
+ Workqueues can be performance or power-oriented.
+ Currently, most workqueues are bound to the CPU they
+ were created on. This gives good performance (due to
+ cache effects) at the cost of potentially waking up
+ otherwise idle cores (Idle from scheduler's perspective.
+ Which may or may not be physically idle) just to process
+ some work. To save power, we can allow the work to be
+ rescheduled on a core that is already awake.
+
+ Workqueues created with the WQ_UNBOUND flag will allow
+ some power savings. However, we don't change the
+ default behaviour of the system. To enable power-saving
+ behaviour, a new config option CONFIG_WQ_POWER_EFFICIENT
+ needs to be turned on. This option can also be
+ overridden by the workqueue.power_efficient boot
+ parameter.
+
x2apic_phys [X86-64,APIC] Use x2apic physical mode instead of
default x2apic cluster mode on platforms
supporting x2apic.
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 623488f..83fa570 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -302,6 +302,9 @@ enum {
WQ_HIGHPRI = 1 << 4, /* high priority */
WQ_CPU_INTENSIVE = 1 << 5, /* cpu instensive workqueue */
WQ_SYSFS = 1 << 6, /* visible in sysfs, see wq_sysfs_register() */
+ WQ_POWER_EFFICIENT = 1 << 7, /* WQ_UNBOUND, for power
+ * saving, if wq_power_efficient is
+ * enabled. Unused otherwise. */

__WQ_DRAINING = 1 << 16, /* internal: workqueue is draining */
__WQ_ORDERED = 1 << 17, /* internal: workqueue is ordered */
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 5dfdc9e..e8c2b77 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -263,6 +263,27 @@ config PM_GENERIC_DOMAINS
bool
depends on PM

+config WQ_POWER_EFFICIENT
+ bool "Workqueue allocated as UNBOUND (by default) for power efficiency"
+ depends on PM
+ default n
+ help
+ Workqueues can be performance or power-oriented. Currently, most
+ workqueues are bound to the CPU they were created on. This gives good
+ performance (due to cache effects) at the cost of potentially waking
+ up otherwise idle cores (Idle from scheduler's perspective. Which may
+ or may not be physically idle) just to process some work. To save
+ power, we can allow the work to be rescheduled on a core that is
+ already awake.
+
+ Workqueues created with the WQ_UNBOUND flag will allow some power
+ savings. However, we don't change the default behaviour of the
+ system. To enable power-saving behaviour, a new config option
+ CONFIG_WQ_POWER_EFFICIENT needs to be turned on. This option can also
+ be overridden by the workqueue.power_efficient boot parameter.
+
+ If in doubt, say N.
+
config PM_GENERIC_DOMAINS_SLEEP
def_bool y
depends on PM_SLEEP && PM_GENERIC_DOMAINS
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 1ae6028..9d8753e 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -272,6 +272,14 @@ static cpumask_var_t *wq_numa_possible_cpumask;
static bool wq_disable_numa;
module_param_named(disable_numa, wq_disable_numa, bool, 0444);

+#ifdef CONFIG_WQ_POWER_EFFICIENT
+static bool wq_power_efficient = true;
+#else
+static bool wq_power_efficient;
+#endif
+
+module_param_named(power_efficient, wq_power_efficient, bool, 0444);
+
static bool wq_numa_enabled; /* unbound NUMA affinity enabled */

/* buf for wq_update_unbound_numa_attrs(), protected by CPU hotplug
exclusion */
@@ -4085,6 +4093,9 @@ struct workqueue_struct
*__alloc_workqueue_key(const char *fmt,
struct workqueue_struct *wq;
struct pool_workqueue *pwq;

+ if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient)
+ flags |= WQ_UNBOUND;
+
/* allocate wq and format name */
if (flags & WQ_UNBOUND)
tbl_size = wq_numa_tbl_len * sizeof(wq->numa_pwq_tbl[0]);


Attachments:
0001-workqueues-Introduce-new-flag-WQ_POWER_EFFICIENT-for.patch (5.64 kB)

2013-05-14 17:54:49

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH V5 0/5] Queue work on power efficient wq

On Wed, Apr 24, 2013 at 05:12:52PM +0530, Viresh Kumar wrote:
> This patchset was called: "Create sched_select_cpu() and use it for workqueues"
> for the first three versions.

Applied to wq/for-3.11 with some updates to comments and
documentation. Will post the applied versions. If something is awry,
please let me know.

Thanks.

--
tejun

2013-05-14 17:56:08

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH V5 1/5] workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

>From cee22a15052faa817e3ec8985a28154d3fabc7aa Mon Sep 17 00:00:00 2001
From: Viresh Kumar <[email protected]>
Date: Mon, 8 Apr 2013 16:45:40 +0530
Subject: workqueues: Introduce new flag WQ_POWER_EFFICIENT for power oriented workqueues

Workqueues can be performance or power-oriented. Currently, most workqueues are
bound to the CPU they were created on. This gives good performance (due to cache
effects) at the cost of potentially waking up otherwise idle cores (Idle from
scheduler's perspective. Which may or may not be physically idle) just to
process some work. To save power, we can allow the work to be rescheduled on a
core that is already awake.

Workqueues created with the WQ_UNBOUND flag will allow some power savings.
However, we don't change the default behaviour of the system. To enable
power-saving behaviour, a new config option CONFIG_WQ_POWER_EFFICIENT needs to
be turned on. This option can also be overridden by the
workqueue.power_efficient boot parameter.

tj: Updated config description and comments. Renamed
CONFIG_WQ_POWER_EFFICIENT to CONFIG_WQ_POWER_EFFICIENT_DEFAULT.

Signed-off-by: Viresh Kumar <[email protected]>
Reviewed-by: Amit Kucheria <[email protected]>
Signed-off-by: Tejun Heo <[email protected]>
---
Documentation/kernel-parameters.txt | 15 +++++++++++++++
include/linux/workqueue.h | 27 +++++++++++++++++++++++++++
kernel/power/Kconfig | 20 ++++++++++++++++++++
kernel/workqueue.c | 13 +++++++++++++
4 files changed, 75 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index c3bfacb..37dfd72 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3320,6 +3320,21 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
that this also can be controlled per-workqueue for
workqueues visible under /sys/bus/workqueue/.

+ workqueue.power_efficient
+ Per-cpu workqueues are generally preferred because
+ they show better performance thanks to cache
+ locality; unfortunately, per-cpu workqueues tend to
+ be more power hungry than unbound workqueues.
+
+ Enabling this makes the per-cpu workqueues which
+ were observed to contribute significantly to power
+ consumption unbound, leading to measurably lower
+ power usage at the cost of small performance
+ overhead.
+
+ The default value of this parameter is determined by
+ the config option CONFIG_WQ_POWER_EFFICIENT_DEFAULT.
+
x2apic_phys [X86-64,APIC] Use x2apic physical mode instead of
default x2apic cluster mode on platforms
supporting x2apic.
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 623488f..fc0136b 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -303,6 +303,33 @@ enum {
WQ_CPU_INTENSIVE = 1 << 5, /* cpu instensive workqueue */
WQ_SYSFS = 1 << 6, /* visible in sysfs, see wq_sysfs_register() */

+ /*
+ * Per-cpu workqueues are generally preferred because they tend to
+ * show better performance thanks to cache locality. Per-cpu
+ * workqueues exclude the scheduler from choosing the CPU to
+ * execute the worker threads, which has an unfortunate side effect
+ * of increasing power consumption.
+ *
+ * The scheduler considers a CPU idle if it doesn't have any task
+ * to execute and tries to keep idle cores idle to conserve power;
+ * however, for example, a per-cpu work item scheduled from an
+ * interrupt handler on an idle CPU will force the scheduler to
+ * excute the work item on that CPU breaking the idleness, which in
+ * turn may lead to more scheduling choices which are sub-optimal
+ * in terms of power consumption.
+ *
+ * Workqueues marked with WQ_POWER_EFFICIENT are per-cpu by default
+ * but become unbound if workqueue.power_efficient kernel param is
+ * specified. Per-cpu workqueues which are identified to
+ * contribute significantly to power-consumption are identified and
+ * marked with this flag and enabling the power_efficient mode
+ * leads to noticeable power saving at the cost of small
+ * performance disadvantage.
+ *
+ * http://thread.gmane.org/gmane.linux.kernel/1480396
+ */
+ WQ_POWER_EFFICIENT = 1 << 7,
+
__WQ_DRAINING = 1 << 16, /* internal: workqueue is draining */
__WQ_ORDERED = 1 << 17, /* internal: workqueue is ordered */

diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 5dfdc9e..4645596 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -263,6 +263,26 @@ config PM_GENERIC_DOMAINS
bool
depends on PM

+config WQ_POWER_EFFICIENT_DEFAULT
+ bool "Enable workqueue power-efficient mode by default"
+ depends on PM
+ default n
+ help
+ Per-cpu workqueues are generally preferred because they show
+ better performance thanks to cache locality; unfortunately,
+ per-cpu workqueues tend to be more power hungry than unbound
+ workqueues.
+
+ Enabling workqueue.power_efficient kernel parameter makes the
+ per-cpu workqueues which were observed to contribute
+ significantly to power consumption unbound, leading to measurably
+ lower power usage at the cost of small performance overhead.
+
+ This config option determines whether workqueue.power_efficient
+ is enabled by default.
+
+ If in doubt, say N.
+
config PM_GENERIC_DOMAINS_SLEEP
def_bool y
depends on PM_SLEEP && PM_GENERIC_DOMAINS
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 4aa9f5b..8068d97 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -272,6 +272,15 @@ static cpumask_var_t *wq_numa_possible_cpumask;
static bool wq_disable_numa;
module_param_named(disable_numa, wq_disable_numa, bool, 0444);

+/* see the comment above the definition of WQ_POWER_EFFICIENT */
+#ifdef CONFIG_WQ_POWER_EFFICIENT_DEFAULT
+static bool wq_power_efficient = true;
+#else
+static bool wq_power_efficient;
+#endif
+
+module_param_named(power_efficient, wq_power_efficient, bool, 0444);
+
static bool wq_numa_enabled; /* unbound NUMA affinity enabled */

/* buf for wq_update_unbound_numa_attrs(), protected by CPU hotplug exclusion */
@@ -4085,6 +4094,10 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
struct workqueue_struct *wq;
struct pool_workqueue *pwq;

+ /* see the comment above the definition of WQ_POWER_EFFICIENT */
+ if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient)
+ flags |= WQ_UNBOUND;
+
/* allocate wq and format name */
if (flags & WQ_UNBOUND)
tbl_size = wq_numa_tbl_len * sizeof(wq->numa_pwq_tbl[0]);
--
1.8.1.4

2013-05-14 17:56:49

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH V5 2/5] workqueue: Add system wide power_efficient workqueues

This one is slightly updated too. The rest remain the same.

>From 0668106ca3865ba945e155097fb042bf66d364d3 Mon Sep 17 00:00:00 2001
From: Viresh Kumar <[email protected]>
Date: Wed, 24 Apr 2013 17:12:54 +0530
Subject: workqueue: Add system wide power_efficient workqueues

This patch adds system wide workqueues aligned towards power saving. This is
done by allocating them with WQ_UNBOUND flag if 'wq_power_efficient' is set to
'true'.

tj: updated comments a bit.

Signed-off-by: Viresh Kumar <[email protected]>
Signed-off-by: Tejun Heo <[email protected]>
---
include/linux/workqueue.h | 8 ++++++++
kernel/workqueue.c | 13 ++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index fc0136b..a9f4119 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -360,11 +360,19 @@ enum {
*
* system_freezable_wq is equivalent to system_wq except that it's
* freezable.
+ *
+ * *_power_efficient_wq are inclined towards saving power and converted
+ * into WQ_UNBOUND variants if 'wq_power_efficient' is enabled; otherwise,
+ * they are same as their non-power-efficient counterparts - e.g.
+ * system_power_efficient_wq is identical to system_wq if
+ * 'wq_power_efficient' is disabled. See WQ_POWER_EFFICIENT for more info.
*/
extern struct workqueue_struct *system_wq;
extern struct workqueue_struct *system_long_wq;
extern struct workqueue_struct *system_unbound_wq;
extern struct workqueue_struct *system_freezable_wq;
+extern struct workqueue_struct *system_power_efficient_wq;
+extern struct workqueue_struct *system_freezable_power_efficient_wq;

static inline struct workqueue_struct * __deprecated __system_nrt_wq(void)
{
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 8068d97..16ca2d3 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -314,6 +314,10 @@ struct workqueue_struct *system_unbound_wq __read_mostly;
EXPORT_SYMBOL_GPL(system_unbound_wq);
struct workqueue_struct *system_freezable_wq __read_mostly;
EXPORT_SYMBOL_GPL(system_freezable_wq);
+struct workqueue_struct *system_power_efficient_wq __read_mostly;
+EXPORT_SYMBOL_GPL(system_power_efficient_wq);
+struct workqueue_struct *system_freezable_power_efficient_wq __read_mostly;
+EXPORT_SYMBOL_GPL(system_freezable_power_efficient_wq);

static int worker_thread(void *__worker);
static void copy_workqueue_attrs(struct workqueue_attrs *to,
@@ -4987,8 +4991,15 @@ static int __init init_workqueues(void)
WQ_UNBOUND_MAX_ACTIVE);
system_freezable_wq = alloc_workqueue("events_freezable",
WQ_FREEZABLE, 0);
+ system_power_efficient_wq = alloc_workqueue("events_power_efficient",
+ WQ_POWER_EFFICIENT, 0);
+ system_freezable_power_efficient_wq = alloc_workqueue("events_freezable_power_efficient",
+ WQ_FREEZABLE | WQ_POWER_EFFICIENT,
+ 0);
BUG_ON(!system_wq || !system_highpri_wq || !system_long_wq ||
- !system_unbound_wq || !system_freezable_wq);
+ !system_unbound_wq || !system_freezable_wq ||
+ !system_power_efficient_wq ||
+ !system_freezable_power_efficient_wq);
return 0;
}
early_initcall(init_workqueues);
--
1.8.1.4

2013-05-14 17:57:34

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH V5 4/5] block: queue work on power efficient wq

On Wed, Apr 24, 2013 at 05:12:56PM +0530, Viresh Kumar wrote:
> Block layer uses workqueues for multiple purposes. There is no real dependency
> of scheduling these on the cpu which scheduled them.
>
> On a idle system, it is observed that and idle cpu wakes up many times just to
> service this work. It would be better if we can schedule it on a cpu which the
> scheduler believes to be the most appropriate one.
>
> This patch replaces normal workqueues with power efficient versions.
>
> Cc: Jens Axboe <[email protected]>
> Signed-off-by: Viresh Kumar <[email protected]>

Jens, applied this to wq/for-3.11. Please holler if you wanna route
this differently.

Thanks.

--
tejun

2013-05-14 17:58:10

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH V5 5/5] fbcon: queue work on power efficient wq

On Wed, Apr 24, 2013 at 05:12:57PM +0530, Viresh Kumar wrote:
> fbcon uses workqueues and it has no real dependency of scheduling these on the
> cpu which scheduled them.
>
> On a idle system, it is observed that and idle cpu wakes up many times just to
> service this work. It would be better if we can schedule it on a cpu which the
> scheduler believes to be the most appropriate one.
>
> This patch replaces system_wq with system_power_efficient_wq.
>
> Cc: Dave Airlie <[email protected]>
> Cc: [email protected]
> Signed-off-by: Viresh Kumar <[email protected]>

Dave, applied this to wq/for-3.11. Please holler if you want this to
be routed differently.

Thanks!

--
tejun

2013-05-15 05:49:02

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V5 0/5] Queue work on power efficient wq

On 14 May 2013 23:24, Tejun Heo <[email protected]> wrote:
> On Wed, Apr 24, 2013 at 05:12:52PM +0530, Viresh Kumar wrote:
>> This patchset was called: "Create sched_select_cpu() and use it for workqueues"
>> for the first three versions.
>
> Applied to wq/for-3.11 with some updates to comments and
> documentation. Will post the applied versions. If something is awry,
> please let me know.

Thanks. All looks good.

2013-07-08 15:37:44

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH V5 0/5] Queue work on power efficient wq

Hello,

On Wed, Apr 24, 2013 at 05:12:52PM +0530, Viresh Kumar wrote:
> This patchset was called: "Create sched_select_cpu() and use it for workqueues"
> for the first three versions.
>
> Earlier discussions over v3, v2 and v1 can be found here:
> https://lkml.org/lkml/2013/3/18/364
> http://lists.linaro.org/pipermail/linaro-dev/2012-November/014344.html
> http://www.mail-archive.com/[email protected]/msg13342.html
>
> V4 is here:
> https://lkml.org/lkml/2013/3/31/55
>
> Workqueues can be performance or power oriented. For performance we may want to
> keep them running on a single cpu, so that it remains cache hot. For power we
> can give scheduler the liberty to choose target cpu for running work handler.
>
> Later one (Power oriented WQ) can be achieved if the workqueue is allocated with
> WQ_UNBOUND flag. Enabling CONFIG_WQ_POWER_EFFICIENT will set
> 'wq_power_efficient' to 'true'. Setting 'power_efficient' boot param will
> override value of 'wq_power_efficient' variable. When 'wq_power_efficient' is
> set to 'true', we will convert WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq
> allocation. And so scheduler will have the liberty to choose where to run this
> work.
>
> Here we are migrating few users of workqueues to WQ_POWER_EFFICIENT. These
> drivers are found to be very much active on idle or lightly busy system and
> using WQ_POWER_EFFICIENT for these gave impressive results.
>
> These would be used in power saving mode only if relevant configs are enabled
> at compile time or in bootargs. Otherwise behavior is unchanged.
>
> Setup:
> -----
> - ARM Vexpress TC2 - big.LITTLE CPU
> - Core 0-1: A15, 2-4: A7
> - rootfs: linaro-ubuntu-devel
This patch set hit my tree now. I wonder if it makes sense to make
WQ_POWER_EFFICIENT_DEFAULT depend on SMP. (Oh wait, big.LITTLE isn't
SMP, so probably there is a better symbol to depend on?)

Just my 2 cent
Uwe


--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2013-07-08 15:47:05

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V5 0/5] Queue work on power efficient wq

On 8 July 2013 21:07, Uwe Kleine-K?nig <[email protected]> wrote:
> This patch set hit my tree now. I wonder if it makes sense to make
> WQ_POWER_EFFICIENT_DEFAULT depend on SMP. (Oh wait, big.LITTLE isn't
> SMP, so probably there is a better symbol to depend on?)

Well, big LITTLE still runs an SMP kernel :) and so has this flag set.
You can make it dependent on that if required.

2013-07-08 15:58:08

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH V5 0/5] Queue work on power efficient wq

On Mon, Jul 08, 2013 at 09:17:01PM +0530, Viresh Kumar wrote:
> On 8 July 2013 21:07, Uwe Kleine-K?nig <[email protected]> wrote:
> > This patch set hit my tree now. I wonder if it makes sense to make
> > WQ_POWER_EFFICIENT_DEFAULT depend on SMP. (Oh wait, big.LITTLE isn't
> > SMP, so probably there is a better symbol to depend on?)
>
> Well, big LITTLE still runs an SMP kernel :) and so has this flag set.
The 'S' is justified because cpu0 and cpu1 are of the same type? Are
there b.L systems that have only one big and one LITTLE cpu? Do these
use SMP, too?

> You can make it dependent on that if required.
Well, it's not required. It's just that the corresponding question in
make oldconfig isn't really an enrichment for a kernel targeting an
Cortex M3 :-)

Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2013-07-08 16:48:12

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V5 0/5] Queue work on power efficient wq

On 8 July 2013 21:27, Uwe Kleine-K?nig <[email protected]> wrote:
> On Mon, Jul 08, 2013 at 09:17:01PM +0530, Viresh Kumar wrote:

>> Well, big LITTLE still runs an SMP kernel :) and so has this flag set.
> The 'S' is justified because cpu0 and cpu1 are of the same type? Are
> there b.L systems that have only one big and one LITTLE cpu? Do these
> use SMP, too?

Following definition of SMP says:
http://en.wikipedia.org/wiki/Symmetric_Multiprocessor

A system is SMP when:
- It has same type of cores
- controlled by a single instance of OS.

In big LITTLE first one is obviously not completely true as you pointed out.
But second one is and so I would say its an SMP system :)

Don't know how it should be called though.

>> You can make it dependent on that if required.
> Well, it's not required. It's just that the corresponding question in
> make oldconfig isn't really an enrichment for a kernel targeting an
> Cortex M3 :-)

Just to make it clear enough, you are saying it doesn't make any
sense to enable it for M3? But because it is disabled by default,
the problem is not seen?

Why? Can't we have two M3's on a SoC and run an SMP kernel over
it?

--
viresh

2013-07-08 18:55:31

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH V5 0/5] Queue work on power efficient wq

Hello,

On Mon, Jul 08, 2013 at 10:18:08PM +0530, Viresh Kumar wrote:
> On 8 July 2013 21:27, Uwe Kleine-K?nig <[email protected]> wrote:
> > On Mon, Jul 08, 2013 at 09:17:01PM +0530, Viresh Kumar wrote:
>
> >> Well, big LITTLE still runs an SMP kernel :) and so has this flag set.
> > The 'S' is justified because cpu0 and cpu1 are of the same type? Are
> > there b.L systems that have only one big and one LITTLE cpu? Do these
> > use SMP, too?
>
> Following definition of SMP says:
> http://en.wikipedia.org/wiki/Symmetric_Multiprocessor
>
> A system is SMP when:
> - It has same type of cores
> - controlled by a single instance of OS.
>
> In big LITTLE first one is obviously not completely true as you pointed out.
> But second one is and so I would say its an SMP system :)
>
> Don't know how it should be called though.
MP maybe.

> >> You can make it dependent on that if required.
> > Well, it's not required. It's just that the corresponding question in
> > make oldconfig isn't really an enrichment for a kernel targeting an
> > Cortex M3 :-)
>
> Just to make it clear enough, you are saying it doesn't make any
> sense to enable it for M3? But because it is disabled by default,
> the problem is not seen?
No I'm saying that asking me is bad because on an UP machine it won't
matter what I answer. So please don't ask me if SMP is off.

> Why? Can't we have two M3's on a SoC and run an SMP kernel over
> it?
Yeah, you can. Then you'd have SMP (or MP) enabled though and the choice
for that workqueue thing makes a difference. In that case asking is OK.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |