On Sun, Mar 31, 2013 at 8:01 PM, Viresh Kumar viresh.kumar@linaro.org wrote:

This patch adds system wide system_freezable_unbound_wq which will be used by code that currently uses system_freezable_wq and can be moved to unbound workqueues.

_Why_ do i need this change?

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org

include/linux/workqueue.h | 4 ++++ kernel/workqueue.c | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index 835d12b..ab7597b 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -325,11 +325,15 @@ enum {

• system_freezable_wq is equivalent to system_wq except that it's
• freezable.

• • system_freezable_unbound_wq is equivalent to system_unbound_wq except that
• • it's freezable.
  */

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_freezable_unbound_wq;

static inline struct workqueue_struct * __deprecated __system_nrt_wq(void) { diff --git a/kernel/workqueue.c b/kernel/workqueue.c index df2f6c4..771a5cc 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -281,6 +281,8 @@ 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_freezable_unbound_wq __read_mostly; +EXPORT_SYMBOL_GPL(system_freezable_unbound_wq);

static int worker_thread(void *__worker); static void copy_workqueue_attrs(struct workqueue_attrs *to, @@ -4467,8 +4469,11 @@ static int __init init_workqueues(void) WQ_UNBOUND_MAX_ACTIVE); system_freezable_wq = alloc_workqueue("events_freezable", WQ_FREEZABLE, 0);

•   system_freezable_unbound_wq = alloc_workqueue("events_unbound_freezable",
  

•                                         WQ_FREEZABLE | WQ_UNBOUND, 0);
    BUG_ON(!system_wq || !system_highpri_wq || !system_long_wq ||
  

•          !system_unbound_wq || !system_freezable_wq);
  

•          !system_unbound_wq || !system_freezable_wq ||
  

•          !system_freezable_unbound_wq);
    return 0;
  

} early_initcall(init_workqueues); -- 1.7.12.rc2.18.g61b472e

---

linaro-kernel mailing list linaro-kernel@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-kernel

On Tue, Apr 9, 2013 at 1:00 PM, Viresh Kumar viresh.kumar@linaro.org wrote:

On 31 March 2013 23:49, Tejun Heo tj@kernel.org wrote:

...

So, let's please introduce a new flag to mark these workqueues, say, WQ_UNBOUND_FOR_POWER_SAVE or whatever (please come up with a better name) and provide a compile time switch with boot time override.

Hi Tejun,

I have written a patch to get this done and want to get that a some review before touching other drivers and spamming LKML with other patches :)

Here it is:

From: Viresh Kumar viresh.kumar@linaro.org Date: Mon, 8 Apr 2013 16:45:40 +0530 Subject: [PATCH] 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. To make this compile time configurable with boot time override this patch adds in another flag WQ_POWER_EFFICIENT. This will be converted to WQ_UNBOUND (on wq allocation) if CONFIG_WQ_POWER_EFFICIENT is enabled and bootargs contain workqueue.power_efficient=1. It is unused otherwise and normal behavior of WQ without this flag is expected.

CONFIG option is currently added under kernel/power/Kconfig and is looking for relevant Kconfig file.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org

Documentation/kernel-parameters.txt | 15 +++++++++++++++ include/linux/workqueue.h | 3 +++ kernel/power/Kconfig | 18 ++++++++++++++++++ kernel/workqueue.c | 14 ++++++++++++++ 4 files changed, 50 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 12c42a5..210e5fc 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -3249,6 +3249,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
  

•                   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
  

•                   power_efficient boot param will convert
  

•                   WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq allocation.
  

•                   This requires CONFIG_WQ_POWER_EFFICIENT to be enabled.
  

•                   WQ_POWER_EFFICIENT is unused if power_efficient is not
  

•                   set in boot params.
  

•   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 1a53816..168b5be 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -299,6 +299,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..8d62400 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -263,6 +263,24 @@ config PM_GENERIC_DOMAINS bool depends on PM

+config WQ_POWER_EFFICIENT

•   bool "Workqueue allocated as UNBOUND for power efficiency"
  

•   depends on PM
  

Should default to N so that existing configs don't get prompted for a choice.

•   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 power_efficient boot param
  

•     will convert WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq allocation.
  

•     This requires CONFIG_WQ_POWER_EFFICIENT to be enabled.
  

•     WQ_POWER_EFFICIENT is unused if power_efficient is not set in boot
  

•     params.
  

•     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 1228fd7..590e333 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -271,6 +271,11 @@ 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 = 0;

1?

+module_param_named(power_efficient, wq_power_efficient, bool, 0444); +#endif

static bool wq_numa_enabled; /* unbound NUMA affinity enabled */

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

•   if (flags & WQ_POWER_EFFICIENT) {
  

•           flags &= ~WQ_POWER_EFFICIENT;
  

+#ifdef CONFIG_WQ_POWER_EFFICIENT

•           if (wq_power_efficient)
  

•                   flags |= WQ_UNBOUND;
  

+#endif

•   }
  

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

---

linaro-kernel mailing list linaro-kernel@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-kernel

On Tue, Apr 9, 2013 at 1:00 PM, Viresh Kumar viresh.kumar@linaro.org wrote:

On 31 March 2013 23:49, Tejun Heo tj@kernel.org wrote:

...

So, let's please introduce a new flag to mark these workqueues, say, WQ_UNBOUND_FOR_POWER_SAVE or whatever (please come up with a better name) and provide a compile time switch with boot time override.

Hi Tejun,

I have written a patch to get this done and want to get that a some review before touching other drivers and spamming LKML with other patches :)

Here it is:

From: Viresh Kumar viresh.kumar@linaro.org Date: Mon, 8 Apr 2013 16:45:40 +0530 Subject: [PATCH] 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

Rephrase as "Power-efficient workqueues can be achieved..."

WQ_UNBOUND flag. To make this compile time configurable with boot time override this patch adds in another flag WQ_POWER_EFFICIENT. This will be converted to WQ_UNBOUND (on wq allocation) if CONFIG_WQ_POWER_EFFICIENT is enabled and bootargs contain workqueue.power_efficient=1. It is unused otherwise and normal behavior of WQ without this flag is expected.

Addition of a new flag seems a bit excessive IMHO. Why can't we just set the UNBOUND flag when the wq_power_efficient module param is set?

It would make the patch a lot simpler too.

CONFIG option is currently added under kernel/power/Kconfig and is looking for relevant Kconfig file.

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org

Documentation/kernel-parameters.txt | 15 +++++++++++++++ include/linux/workqueue.h | 3 +++ kernel/power/Kconfig | 18 ++++++++++++++++++ kernel/workqueue.c | 14 ++++++++++++++ 4 files changed, 50 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 12c42a5..210e5fc 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -3249,6 +3249,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
  

•                   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
  

•                   power_efficient boot param will convert
  

•                   WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq allocation.
  

•                   This requires CONFIG_WQ_POWER_EFFICIENT to be enabled.
  

•                   WQ_POWER_EFFICIENT is unused if power_efficient is not
  

•                   set in boot params.
  

•   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 1a53816..168b5be 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -299,6 +299,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..8d62400 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -263,6 +263,24 @@ config PM_GENERIC_DOMAINS bool depends on PM

+config WQ_POWER_EFFICIENT

•   bool "Workqueue allocated as UNBOUND 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 power_efficient boot param
  

•     will convert WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq allocation.
  

•     This requires CONFIG_WQ_POWER_EFFICIENT to be enabled.
  

•     WQ_POWER_EFFICIENT is unused if power_efficient is not set in boot
  

•     params.
  

•     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 1228fd7..590e333 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -271,6 +271,11 @@ 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 = 0; +module_param_named(power_efficient, wq_power_efficient, bool, 0444); +#endif

static bool wq_numa_enabled; /* unbound NUMA affinity enabled */

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

•   if (flags & WQ_POWER_EFFICIENT) {
  

•           flags &= ~WQ_POWER_EFFICIENT;
  

+#ifdef CONFIG_WQ_POWER_EFFICIENT

•           if (wq_power_efficient)
  

•                   flags |= WQ_UNBOUND;
  

+#endif

•   }
  

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

---

linaro-kernel mailing list linaro-kernel@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-kernel

Yo!

On Tue, Apr 09, 2013 at 01:00:59PM +0530, Viresh Kumar wrote:

• 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
  

• 	power_efficient boot param will convert
  

• 	WQ_POWER_EFFICIENT flag to WQ_UNBOUND on wq allocation.
  

• 	This requires CONFIG_WQ_POWER_EFFICIENT to be enabled.
  

• 	WQ_POWER_EFFICIENT is unused if power_efficient is not
  

• 	set in boot params.
  

Looks mostly good to me but I think it'd be better if the parameter name is a bit more specific.

@@ -299,6 +299,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. */
  

Ditto for the flag name. It's not a requirement tho. If you can think of something which is a bit more specific to what it does while not being unreasonably long, great. If not, we'll live.

@@ -271,6 +271,11 @@ 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 = 0; +module_param_named(power_efficient, wq_power_efficient, bool, 0444); +#endif

I don't think we need to make the whole thing configurable. Turning it off isn't gonna save much - my gut tells me it's gonna be four instructions. :)

What I meant was that the default value for the parameter would probably be need to be configurable so that mobile people don't have to include the kernel param all the time or patch the kernel themselves.

• if (flags & WQ_POWER_EFFICIENT) {

• flags &= ~WQ_POWER_EFFICIENT;
  

No need to clear the flag.

+#ifdef CONFIG_WQ_POWER_EFFICIENT

• if (wq_power_efficient)
  

• 	flags |= WQ_UNBOUND;
  

+#endif

Thanks!