Re: [Linaro-validation] LAVA: Jobs submitted to vexpress device type
On 24 Sep 2012, at 12:03, Alexander Sack wrote: > On Mon, Sep 24, 2012 at 12:48 PM, Dave Pigott wrote: >> Unfortunately, not as easy as it sounds. We'd have to actually change the >> dispatcher to intercept submissions to vexpress, and then re-route them to >> vexpress-a9, because we can'\t have two device instances talking to the same >> device. I'll look into it, but it would be quite a nasty bodge. > > > right. guess would justify an "alias" feature for device-types in LAVA > framework before we can do that. > > For now let's move our builds over to use vexpress-a9 > > 1. android-build should be fixed now (at least the code got > submitted); if you still see it tomorrow let me know. > 2. for ci.linaro.org I have to talk to fabo; will not bother him until > he has worked around the android-build git timeout issues currently > blocking release. Looks like android is switched over. I assume Fathi is still blocked for getting ci switched. Dave ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [Linaro-validation] LAVA: Jobs submitted to vexpress device type
On 25 September 2012 10:22, Dave Pigott wrote: > On 24 Sep 2012, at 12:03, Alexander Sack wrote: > >> On Mon, Sep 24, 2012 at 12:48 PM, Dave Pigott wrote: >>> Unfortunately, not as easy as it sounds. We'd have to actually change the >>> dispatcher to intercept submissions to vexpress, and then re-route them to >>> vexpress-a9, because we can'\t have two device instances talking to the same >>> device. I'll look into it, but it would be quite a nasty bodge. >> >> >> right. guess would justify an "alias" feature for device-types in LAVA >> framework before we can do that. >> >> For now let's move our builds over to use vexpress-a9 >> >> 1. android-build should be fixed now (at least the code got >> submitted); if you still see it tomorrow let me know. >> 2. for ci.linaro.org I have to talk to fabo; will not bother him until >> he has worked around the android-build git timeout issues currently >> blocking release. > > Looks like android is switched over. I assume Fathi is still blocked for > getting ci switched. All jobs are now updated. Only 3 missing, fixed now by Deepti. Cheers, Fathi ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [Linaro-validation] LAVA: Jobs submitted to vexpress device type
On 25 Sep 2012, at 08:40, Fathi Boudra wrote: > On 25 September 2012 10:22, Dave Pigott wrote: >> On 24 Sep 2012, at 12:03, Alexander Sack wrote: >> >>> On Mon, Sep 24, 2012 at 12:48 PM, Dave Pigott >>> wrote: Unfortunately, not as easy as it sounds. We'd have to actually change the dispatcher to intercept submissions to vexpress, and then re-route them to vexpress-a9, because we can'\t have two device instances talking to the same device. I'll look into it, but it would be quite a nasty bodge. >>> >>> >>> right. guess would justify an "alias" feature for device-types in LAVA >>> framework before we can do that. >>> >>> For now let's move our builds over to use vexpress-a9 >>> >>> 1. android-build should be fixed now (at least the code got >>> submitted); if you still see it tomorrow let me know. >>> 2. for ci.linaro.org I have to talk to fabo; will not bother him until >>> he has worked around the android-build git timeout issues currently >>> blocking release. >> >> Looks like android is switched over. I assume Fathi is still blocked for >> getting ci switched. > > All jobs are now updated. Only 3 missing, fixed now by Deepti. Thanks Fathi. Backlog of vexpress jobs has now been cleared. Dave ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH 2/3] workqueue: create __flush_delayed_work to avoid duplicating code
flush_delayed_work() and flush_delayed_work_sync() had major portion of code
similar. This patch introduces another routine __flush_delayed_work() which
contains the common part to avoid code duplication.
Signed-off-by: Viresh Kumar
---
kernel/workqueue.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 692d976..692a55b 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2820,6 +2820,13 @@ bool cancel_work_sync(struct work_struct *work)
}
EXPORT_SYMBOL_GPL(cancel_work_sync);
+static inline void __flush_delayed_work(struct delayed_work *dwork)
+{
+ if (del_timer_sync(&dwork->timer))
+ __queue_work(raw_smp_processor_id(),
+get_work_cwq(&dwork->work)->wq, &dwork->work);
+}
+
/**
* flush_delayed_work - wait for a dwork to finish executing the last queueing
* @dwork: the delayed work to flush
@@ -2834,9 +2841,7 @@ EXPORT_SYMBOL_GPL(cancel_work_sync);
*/
bool flush_delayed_work(struct delayed_work *dwork)
{
- if (del_timer_sync(&dwork->timer))
- __queue_work(raw_smp_processor_id(),
-get_work_cwq(&dwork->work)->wq, &dwork->work);
+ __flush_delayed_work(dwork);
return flush_work(&dwork->work);
}
EXPORT_SYMBOL(flush_delayed_work);
@@ -2855,9 +2860,7 @@ EXPORT_SYMBOL(flush_delayed_work);
*/
bool flush_delayed_work_sync(struct delayed_work *dwork)
{
- if (del_timer_sync(&dwork->timer))
- __queue_work(raw_smp_processor_id(),
-get_work_cwq(&dwork->work)->wq, &dwork->work);
+ __flush_delayed_work(dwork);
return flush_work_sync(&dwork->work);
}
EXPORT_SYMBOL(flush_delayed_work_sync);
--
1.7.12.rc2.18.g61b472e
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH 3/3] workqueue: Schedule work on non-idle cpu instead of current one
Workqueues queues work on current cpu, if the caller haven't passed a preferred
cpu. This may wake up an idle CPU, which is actually not required.
This work can be processed by any CPU and so we must select a non-idle CPU here.
This patch adds in support in workqueue framework to get preferred CPU details
from the scheduler, instead of using current CPU.
Signed-off-by: Viresh Kumar
---
arch/arm/Kconfig | 11 +++
kernel/workqueue.c | 25 ++---
2 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5944511..da17bd0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1594,6 +1594,17 @@ config HMP_SLOW_CPU_MASK
Specify the cpuids of the slow CPUs in the system as a list string,
e.g. cpuid 0+1 should be specified as 0-1.
+config MIGRATE_WQ
+ bool "(EXPERIMENTAL) Migrate Workqueues to non-idle cpu"
+ depends on SMP && EXPERIMENTAL
+ help
+ Workqueues queues work on current cpu, if the caller haven't passed a
+ preferred cpu. This may wake up an idle CPU, which is actually not
+ required. This work can be processed by any CPU and so we must select
+ a non-idle CPU here. This patch adds in support in workqueue
+ framework to get preferred CPU details from the scheduler, instead of
+ using current CPU.
+
config HAVE_ARM_SCU
bool
help
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 692a55b..fd8df4a 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -456,6 +456,16 @@ static inline void debug_work_activate(struct work_struct
*work) { }
static inline void debug_work_deactivate(struct work_struct *work) { }
#endif
+/* This enables migration of a work to a non-IDLE cpu instead of current cpu */
+#ifdef CONFIG_MIGRATE_WQ
+static int wq_select_cpu(void)
+{
+ return sched_select_cpu(SD_NUMA, -1);
+}
+#else
+#define wq_select_cpu()smp_processor_id()
+#endif
+
/* Serializes the accesses to the list of workqueues. */
static DEFINE_SPINLOCK(workqueue_lock);
static LIST_HEAD(workqueues);
@@ -995,7 +1005,7 @@ static void __queue_work(unsigned int cpu, struct
workqueue_struct *wq,
struct global_cwq *last_gcwq;
if (unlikely(cpu == WORK_CPU_UNBOUND))
- cpu = raw_smp_processor_id();
+ cpu = wq_select_cpu();
/*
* It's multi cpu. If @wq is non-reentrant and @work
@@ -1066,8 +1076,9 @@ int queue_work(struct workqueue_struct *wq, struct
work_struct *work)
{
int ret;
- ret = queue_work_on(get_cpu(), wq, work);
- put_cpu();
+ preempt_disable();
+ ret = queue_work_on(wq_select_cpu(), wq, work);
+ preempt_enable();
return ret;
}
@@ -1102,7 +1113,7 @@ static void delayed_work_timer_fn(unsigned long __data)
struct delayed_work *dwork = (struct delayed_work *)__data;
struct cpu_workqueue_struct *cwq = get_work_cwq(&dwork->work);
- __queue_work(smp_processor_id(), cwq->wq, &dwork->work);
+ __queue_work(wq_select_cpu(), cwq->wq, &dwork->work);
}
/**
@@ -1158,7 +1169,7 @@ int queue_delayed_work_on(int cpu, struct
workqueue_struct *wq,
if (gcwq && gcwq->cpu != WORK_CPU_UNBOUND)
lcpu = gcwq->cpu;
else
- lcpu = raw_smp_processor_id();
+ lcpu = wq_select_cpu();
} else
lcpu = WORK_CPU_UNBOUND;
@@ -2823,8 +2834,8 @@ EXPORT_SYMBOL_GPL(cancel_work_sync);
static inline void __flush_delayed_work(struct delayed_work *dwork)
{
if (del_timer_sync(&dwork->timer))
- __queue_work(raw_smp_processor_id(),
-get_work_cwq(&dwork->work)->wq, &dwork->work);
+ __queue_work(wq_select_cpu(), get_work_cwq(&dwork->work)->wq,
+&dwork->work);
}
/**
--
1.7.12.rc2.18.g61b472e
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH 0/3] Create sched_select_cpu() and use it in workqueues
In order to save power, it would be useful to schedule work onto non-IDLE cpus instead of waking up an IDLE one. To achieve this, we need scheduler to guide kernel frameworks (like: timers & workqueues) on which is the most preferred CPU that must be used for these tasks. This patchset is about implementing this concept. The first patch adds sched_select_cpu() routine which returns the preferred cpu which is non-idle. It accepts max level of sched domain, upto which we can choose a CPU from. It can accept following options: SD_SIBLING, SD_MC, SD_BOOK, SD_CPU or SD_NUMA. Second and Third patch are about adapting this change in workqueue framework. Earlier discussions over this concept were done at last LPC: http://summit.linuxplumbersconf.org/lpc-2012/meeting/90/lpc2012-sched-timer-workqueue/ Figures: Test case 1: - Performed on TC2 with ubuntu-devel - Boot TC2 and run $ trace-cmd record -e workqueue_execute_start This will trace only the points, where the work actually runs. Do, this for 150 seconds. Results: - Domain 0: CPU 0-1 Domain 1: CPU 2-4 Base Kernel: Without my modifications: - CPU No. of works run by CPU - --- CPU0: 7 CPU1: 445 CPU2: 444 CPU3: 315 CPU4: 226 With my modifications: -- CPU No. of works run by CPU --- CPU0: 31 CPU2: 797 CPU3: 274 CPU4: 86 Test case 2: --- I have created a small module, which does following: - Create one work for each CPU (using queue_work_on(), so must schedule on that cpu) - Above work, will queue "n" works for each cpu with queue_work(). These works are tracked within the module and results are printed at the end. This gave similar results, with n ranging from 10 to 1000. Viresh Kumar (3): sched: Create sched_select_cpu() to give preferred CPU for power saving workqueue: create __flush_delayed_work to avoid duplicating code workqueue: Schedule work on non-idle cpu instead of current one arch/arm/Kconfig | 11 +++ include/linux/sched.h | 11 +++ kernel/sched/core.c | 88 +++ kernel/workqueue.c| 36 ++--- 4 files changed, 115 insertions(+), 31 deletions(-) -- 1.7.12.rc2.18.g61b472e ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH 1/3] sched: Create sched_select_cpu() to give preferred CPU for power saving
In order to save power, it would be useful to schedule work onto non-IDLE cpus
instead of waking up an IDLE one.
To achieve this, we need scheduler to guide kernel frameworks (like: timers &
workqueues) on which is the most preferred CPU that must be used for these
tasks.
This routine returns the preferred cpu which is non-idle. It accepts max level
of sched domain, upto which we can choose a CPU from. It can accept following
options: SD_SIBLING, SD_MC, SD_BOOK, SD_CPU or SD_NUMA.
If user passed SD_MC, then we can return a CPU from SD_SIBLING or SD_MC. If the
level requested by user is not available for the current kernel configuration,
then current CPU will be returned.
If user has passed NUMA level, then we may need to go through numa_levels too.
Second parameter to this routine will now come into play. Its minimum value is
zero, in which case there is only one NUMA level to go through. If you want to
go through all NUMA levels, pass -1 here. This should cover all NUMA levels.
This patch reuses the code from get_nohz_timer_target() routine, which had
similar implementation. get_nohz_timer_target() is also modified to use
sched_select_cpu() now.
Signed-off-by: Viresh Kumar
---
include/linux/sched.h | 11 +++
kernel/sched/core.c | 88 +++
2 files changed, 79 insertions(+), 20 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 0059212..4b660ee 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -281,6 +281,10 @@ static inline void select_nohz_load_balancer(int
stop_tick) { }
static inline void set_cpu_sd_state_idle(void) { }
#endif
+#ifdef CONFIG_SMP
+extern int sched_select_cpu(int sd_max_level, u32 numa_level);
+#endif
+
/*
* Only dump TASK_* tasks. (0 for all tasks)
*/
@@ -868,6 +872,13 @@ enum cpu_idle_type {
#define SD_PREFER_SIBLING 0x1000 /* Prefer to place tasks in a sibling
domain */
#define SD_OVERLAP 0x2000 /* sched_domains of this level overlap
*/
+/* sched-domain levels */
+#define SD_SIBLING 0x01/* Only for CONFIG_SCHED_SMT */
+#define SD_MC 0x02/* Only for CONFIG_SCHED_MC */
+#define SD_BOOK0x04/* Only for CONFIG_SCHED_BOOK */
+#define SD_CPU 0x08/* Always enabled */
+#define SD_NUMA0x10/* Only for CONFIG_NUMA */
+
extern int __weak arch_sd_sibiling_asym_packing(void);
struct sched_group_power {
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index de97083..a14014c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -551,22 +551,7 @@ void resched_cpu(int cpu)
*/
int get_nohz_timer_target(void)
{
- int cpu = smp_processor_id();
- int i;
- struct sched_domain *sd;
-
- rcu_read_lock();
- for_each_domain(cpu, sd) {
- for_each_cpu(i, sched_domain_span(sd)) {
- if (!idle_cpu(i)) {
- cpu = i;
- goto unlock;
- }
- }
- }
-unlock:
- rcu_read_unlock();
- return cpu;
+ return sched_select_cpu(SD_NUMA, -1);
}
/*
* When add_timer_on() enqueues a timer into the timer wheel of an
@@ -639,6 +624,66 @@ void sched_avg_update(struct rq *rq)
}
}
+/* Mask of all the SD levels present in current configuration */
+static int sd_present_levels;
+
+/*
+ * This routine returns the preferred cpu which is non-idle. It accepts max
+ * level of sched domain, upto which we can choose a CPU from. It can accept
+ * following options: SD_SIBLING, SD_MC, SD_BOOK, SD_CPU or SD_NUMA.
+ *
+ * If user passed SD_MC, then we can return a CPU from SD_SIBLING or SD_MC.
+ * If the level requested by user is not available for the current kernel
+ * configuration, then current CPU will be returned.
+ *
+ * If user has passed NUMA level, then we may need to go through numa_levels
+ * too. Second parameter to this routine will now come into play. Its minimum
+ * value is zero, in which case there is only one NUMA level to go through. If
+ * you want to go through all NUMA levels, pass -1 here. This should cover all
+ * NUMA levels.
+ */
+int sched_select_cpu(int sd_max_level, u32 numa_level)
+{
+ struct sched_domain *sd;
+ int cpu = smp_processor_id();
+ int i, sd_target_levels;
+
+ sd_target_levels = (sd_max_level | (sd_max_level - 1))
+ & sd_present_levels;
+
+ /* return current cpu if no sd_present_levels <= sd_max_level */
+ if (!sd_target_levels)
+ return cpu;
+
+ rcu_read_lock();
+ for_each_domain(cpu, sd) {
+ for_each_cpu(i, sched_domain_span(sd)) {
+ if (!idle_cpu(i)) {
+ cpu = i;
+ goto unlock;
+ }
+ }
+
+ /* Do we need to go through NU
Re: [PATCH 1/3] sched: Create sched_select_cpu() to give preferred CPU for power saving
On Tue, 2012-09-25 at 16:06 +0530, Viresh Kumar wrote: > +/* sched-domain levels */ > +#define SD_SIBLING 0x01/* Only for CONFIG_SCHED_SMT */ > +#define SD_MC 0x02/* Only for CONFIG_SCHED_MC */ > +#define SD_BOOK0x04/* Only for CONFIG_SCHED_BOOK > */ > +#define SD_CPU 0x08/* Always enabled */ > +#define SD_NUMA0x10/* Only for CONFIG_NUMA */ Urgh, no, not more of that nonsense.. I want to get rid of that hardcoded stuff, not add more of it. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 0/3] Create sched_select_cpu() and use it in workqueues
On 25 September 2012 16:06, Viresh Kumar wrote: > Test case 2: > --- > I have created a small module, which does following: > - Create one work for each CPU (using queue_work_on(), so must schedule on > that > cpu) > - Above work, will queue "n" works for each cpu with queue_work(). These works > are tracked within the module and results are printed at the end. > > This gave similar results, with n ranging from 10 to 1000. > http://git.linaro.org/gitweb?p=people/vireshk/module.git;a=summary Source of this module can be found at above repo. -- viresh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/3] workqueue: Schedule work on non-idle cpu instead of current one
On Tue, 2012-09-25 at 16:06 +0530, Viresh Kumar wrote:
> @@ -1066,8 +1076,9 @@ int queue_work(struct workqueue_struct *wq,
> struct work_struct *work)
> {
> int ret;
>
> - ret = queue_work_on(get_cpu(), wq, work);
> - put_cpu();
> + preempt_disable();
> + ret = queue_work_on(wq_select_cpu(), wq, work);
> + preempt_enable();
>
> return ret;
> }
Right, so the problem I see here is that wq_select_cpu() is horridly
expensive..
> @@ -1102,7 +1113,7 @@ static void delayed_work_timer_fn(unsigned long
> __data)
> struct delayed_work *dwork = (struct delayed_work *)__data;
> struct cpu_workqueue_struct *cwq = get_work_cwq(&dwork->work);
>
> - __queue_work(smp_processor_id(), cwq->wq, &dwork->work);
> + __queue_work(wq_select_cpu(), cwq->wq, &dwork->work);
> }
Shouldn't timer migration have sorted this one?
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/3] workqueue: Schedule work on non-idle cpu instead of current one
On 25 September 2012 16:52, Peter Zijlstra wrote:
> On Tue, 2012-09-25 at 16:06 +0530, Viresh Kumar wrote:
>> @@ -1066,8 +1076,9 @@ int queue_work(struct workqueue_struct *wq,
>> struct work_struct *work)
>> {
>> int ret;
>>
>> - ret = queue_work_on(get_cpu(), wq, work);
>> - put_cpu();
>> + preempt_disable();
>> + ret = queue_work_on(wq_select_cpu(), wq, work);
>> + preempt_enable();
>>
>> return ret;
>> }
>
> Right, so the problem I see here is that wq_select_cpu() is horridly
> expensive..
But this is what the initial idea during LPC we had. Any improvements here
you can suggest?
>> @@ -1102,7 +1113,7 @@ static void delayed_work_timer_fn(unsigned long
>> __data)
>> struct delayed_work *dwork = (struct delayed_work *)__data;
>> struct cpu_workqueue_struct *cwq = get_work_cwq(&dwork->work);
>>
>> - __queue_work(smp_processor_id(), cwq->wq, &dwork->work);
>> + __queue_work(wq_select_cpu(), cwq->wq, &dwork->work);
>> }
>
> Shouldn't timer migration have sorted this one?
Maybe yes. Will investigate more on it.
Thanks for your early feedback.
--
viresh
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/3] workqueue: Schedule work on non-idle cpu instead of current one
On 25 September 2012 13:30, Viresh Kumar wrote:
> On 25 September 2012 16:52, Peter Zijlstra wrote:
>> On Tue, 2012-09-25 at 16:06 +0530, Viresh Kumar wrote:
>>> @@ -1066,8 +1076,9 @@ int queue_work(struct workqueue_struct *wq,
>>> struct work_struct *work)
>>> {
>>> int ret;
>>>
>>> - ret = queue_work_on(get_cpu(), wq, work);
>>> - put_cpu();
>>> + preempt_disable();
>>> + ret = queue_work_on(wq_select_cpu(), wq, work);
>>> + preempt_enable();
>>>
>>> return ret;
>>> }
>>
>> Right, so the problem I see here is that wq_select_cpu() is horridly
>> expensive..
>
> But this is what the initial idea during LPC we had. Any improvements here
> you can suggest?
The main outcome of the LPC was that we should be able to select
another CPU than the local one.
Using the same policy than timer, is a 1st step to consolidate
interface. A next step should be to update the policy of the function
Vincent
>
>>> @@ -1102,7 +1113,7 @@ static void delayed_work_timer_fn(unsigned long
>>> __data)
>>> struct delayed_work *dwork = (struct delayed_work *)__data;
>>> struct cpu_workqueue_struct *cwq = get_work_cwq(&dwork->work);
>>>
>>> - __queue_work(smp_processor_id(), cwq->wq, &dwork->work);
>>> + __queue_work(wq_select_cpu(), cwq->wq, &dwork->work);
>>> }
>>
>> Shouldn't timer migration have sorted this one?
>
> Maybe yes. Will investigate more on it.
>
> Thanks for your early feedback.
>
> --
> viresh
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/3] workqueue: Schedule work on non-idle cpu instead of current one
On Tue, 2012-09-25 at 17:00 +0530, Viresh Kumar wrote: > But this is what the initial idea during LPC we had. Yeah.. that's true. > Any improvements here you can suggest? We could uhm... /me tries thinking ... reuse some of the NOHZ magic? Would that be sufficient, not waking a NOHZ cpu, or do you really want not waking any idle cpu? ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/3] workqueue: Schedule work on non-idle cpu instead of current one
On Tue, 2012-09-25 at 13:40 +0200, Peter Zijlstra wrote: > On Tue, 2012-09-25 at 17:00 +0530, Viresh Kumar wrote: > > But this is what the initial idea during LPC we had. > > Yeah.. that's true. > > > Any improvements here you can suggest? > > We could uhm... /me tries thinking ... reuse some of the NOHZ magic? > Would that be sufficient, not waking a NOHZ cpu, or do you really want > not waking any idle cpu? Depending on the trade-off we could have the NOHZ stuff track a non-NOHZ-idle cpu and avoid having to compute one every time we need it. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH][RESEND] power: opp: rcu reclaim
synchronize_rcu blocks the caller of opp_enable/disbale
for a complete grace period. This blocking duration prevents
any intensive use of the functions. Replace synchronize_rcu
by call_rcu which will call our function for freeing the old
opp element.
The duration of opp_enable and opp_disable will be no more
dependant of the grace period.
Signed-off-by: Vincent Guittot
---
drivers/base/power/opp.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index ac993ea..49e4626 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -64,6 +64,7 @@ struct opp {
unsigned long u_volt;
struct device_opp *dev_opp;
+ struct rcu_head head;
};
/**
@@ -441,6 +442,17 @@ int opp_add(struct device *dev, unsigned long freq,
unsigned long u_volt)
}
/**
+ * opp_free_rcu() - helper to clear the struct opp when grace period has
+ * elapsed without blocking the the caller of opp_set_availability
+ */
+static void opp_free_rcu(struct rcu_head *head)
+{
+ struct opp *opp = container_of(head, struct opp, head);
+
+ kfree(opp);
+}
+
+/**
* opp_set_availability() - helper to set the availability of an opp
* @dev: device for which we do this operation
* @freq: OPP frequency to modify availability
@@ -511,7 +523,7 @@ static int opp_set_availability(struct device *dev,
unsigned long freq,
list_replace_rcu(&opp->node, &new_opp->node);
mutex_unlock(&dev_opp_list_lock);
- synchronize_rcu();
+ call_rcu(&opp->head, opp_free_rcu);
/* Notify the change of the OPP availability */
if (availability_req)
@@ -521,13 +533,10 @@ static int opp_set_availability(struct device *dev,
unsigned long freq,
srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_DISABLE,
new_opp);
- /* clean up old opp */
- new_opp = opp;
- goto out;
+ return 0;
unlock:
mutex_unlock(&dev_opp_list_lock);
-out:
kfree(new_opp);
return r;
}
--
1.7.9.5
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [Powertop] [PATCH v2 1/2] Updates to support Android platform
On 09/24/2012 10:34 PM, Rajagopal Venkat wrote:
On 24 September 2012 21:27, Chris Ferron wrote:
On 09/24/2012 06:28 AM, Rajagopal Venkat wrote:
This patch adds following minor changes to prepare powertop
to support Android platform.
- Add missing HAVE_CONFIG_H conditional check.
- remove un-used ethtool_cmd_speed_set and ethtool_cmd_speed
functions.
- Minimize dependency on exception handling in catch blocks.
These changes will not affect powertop functionality.
Signed-off-by: Rajagopal Venkat
---
src/devices/ahci.cpp| 4 ++--
src/devices/alsa.cpp| 4 ++--
src/devices/network.cpp | 16
src/main.cpp| 2 ++
4 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
index 1fe39c7..67ce06e 100644
--- a/src/devices/ahci.cpp
+++ b/src/devices/ahci.cpp
@@ -170,7 +170,7 @@ void ahci::start_measurement(void)
file.close();
}
catch (std::ios_base::failure &c) {
- fprintf(stderr, "%s\n", c.what());
+ fprintf(stderr, "Failed to start measurement for ahci
device\n");
adding addition message here is acceptable, but eliminating the information
from the catches error is not.
As discussed in first patch set, android doesn't support exception
handling. This
is the reason powertop had DISABLE_TRYCATCH conditional macro which is
removed in recent commit.
The patch 2/2 adds stubs for exception handling
#define try if (true)
#define catch(x) if (false)
With this, fprintf(stderr, "%s\n", c.what()); in catch block throws
undefined reference
to c. So added message instead of c.what().
Any better ways of adding stubs are welcome.
So it is my opinion that this is a bug for deficiency's in Android, and
one that is not appropriate to work around in this core tool. Any
patch/fix/workaround should be submitted to the people maintaining
Android. The reason the DISABLE_TRYCATCH was removed was to eliminate
any non generic need for defines within the tool. We are actively trying
to eliminate and discouraging distribution specific code within the core
PowerTOP source. There is some flex ability for example your stub header
for android.
My current suggestion is to re-submit your patches minus the Android
workarounds for C++ try-catches, and then work this issues with the fine
people at google.
PowerTOP focus should be on platforms, architectures, and the Linux
kernel, not distributions.
-C
}
}
@@ -203,7 +203,7 @@ void ahci::end_measurement(void)
file.close();
}
catch (std::ios_base::failure &c) {
- fprintf(stderr, "%s\n", c.what());
+ fprintf(stderr, "Failed to end measurement for ahci
device\n");
}
if (end_active < start_active)
end_active = start_active;
diff --git a/src/devices/alsa.cpp b/src/devices/alsa.cpp
index 4f5d3f9..a67780c 100644
--- a/src/devices/alsa.cpp
+++ b/src/devices/alsa.cpp
@@ -104,7 +104,7 @@ void alsa::start_measurement(void)
file.close();
}
catch (std::ios_base::failure &c) {
- fprintf(stderr, "%s\n", c.what());
+ fprintf(stderr, "Failed to start measurement for alsa
device\n");
}
}
@@ -130,7 +130,7 @@ void alsa::end_measurement(void)
file.close();
}
catch (std::ios_base::failure &c) {
- fprintf(stderr, "%s\n", c.what());
+ fprintf(stderr, "Failed to end measurement for alsa
device\n");
}
p = (end_active - start_active) / (0.001 + end_active +
end_inactive - start_active - start_inactive) * 100.0;
diff --git a/src/devices/network.cpp b/src/devices/network.cpp
index b8a5c9c..ed9d7aa 100644
--- a/src/devices/network.cpp
+++ b/src/devices/network.cpp
@@ -55,22 +55,6 @@ extern "C" {
static map nics;
-#ifdef DISABLE_TRYCATCH
-
-static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
- __u32 speed)
-{
-
- ep->speed = (__u16)speed;
- ep->speed_hi = (__u16)(speed >> 16);
-}
-
-static inline __u32 ethtool_cmd_speed(struct ethtool_cmd *ep)
-{
- return (ep->speed_hi << 16) | ep->speed;
-}
-
-#endif
static void do_proc_net_dev(void)
{
diff --git a/src/main.cpp b/src/main.cpp
index 1815075..dc49dba 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -42,7 +42,9 @@
#include "perf/perf.h"
#include "perf/perf_bundle.h"
#include "lib.h"
+#ifdef HAVE_CONFIG_H
#include "../config.h"
+#endif
#include "devices/device.h"
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
big.LITTLE MP status Sep 25, 2012
https://wiki.linaro.org/projects/big.LITTLE.MP Announcement == == big.LITTLE Mini-Summit @ Linaro Connect Copenhagen Q32012 == Thursday November 1, 2012 Plenaries: 8.30 - 9.00, 2 x 15 minutes 9am - 1pm: 1 break 10.45am to 11.00am Plenaries - Introduction to the mini-summit, what we’re going to discuss, what we want to achieve Aims - Update the community on progress to date - Discuss the big.LITTLE work in Linaro for the next quarter (workload annotation, power aware scheduler, testing and automation) - Other open source activities Blueprint updates https://blueprints.launchpad.net/linaro-power-kernel/+spec/big.little.mp-hotplug-performance-improvements - Delayed for a month but should land for 12.10 cycle. - A useful file https://wiki.linaro.org/WorkingGroups/PowerManagement/Doc/Hotplug?action=AttachFile&do=view&target=test_cpu_hotplug_latency.sh https://blueprints.launchpad.net/linaro-power-kernel/+spec/perf-record-replay - works in 3.6 mainline kernels https://blueprints.launchpad.net/linaro-power-kernel/+spec/cpuidle-add-cpu-specific-states-capability - per cpu states not accepted. Suggested to modify multiple drivers support : http://www.spinics.net/lists/linux-acpi/msg37921.html https://blueprints.launchpad.net/linaro-qa/+spec/sched-test-porting-to-android - first builds are in progress https://blueprints.launchpad.net/linaro-qa/+spec/task-placement-testing - in progress Roadmap Cards == The roadmap cards http://cards.linaro.org/browse/CARD-34 and http://cards.linaro.org/browse/CARD-35 have been delivered and closed. The roadmap cards http://cards.linaro.org/browse/CARD-189 and http://cards.linaro.org/browse/CARD-190 are marked for for review by the Technical Steering Committee. Miscellaneous == Private QA builds containing test code should be ready this week. These builds give an added value to Linaro members in that they contain code for more complete testing. The project plan is at https://docs.google.com/a/linaro.org/document/d/1XI9FaEd6dYfCEDezboF7gWtrTOmOrShUVoDOy1hr-5I/edit# -- David Zinman Linaro Release Manager | Project Manager Linaro.org | Open source software for ARM SoCs ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 2/3] workqueue: create __flush_delayed_work to avoid duplicating code
On Tue, Sep 25, 2012 at 04:06:07PM +0530, Viresh Kumar wrote: > flush_delayed_work() and flush_delayed_work_sync() had major portion of code > similar. This patch introduces another routine __flush_delayed_work() which > contains the common part to avoid code duplication. This part has seen a lot of update in pending wq/for-3.7 branch. Please rebase on top of that. git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-3.7 Thanks. -- tejun ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/3] workqueue: Schedule work on non-idle cpu instead of current one
Hello,
On Tue, Sep 25, 2012 at 04:06:08PM +0530, Viresh Kumar wrote:
> +config MIGRATE_WQ
> + bool "(EXPERIMENTAL) Migrate Workqueues to non-idle cpu"
> + depends on SMP && EXPERIMENTAL
> + help
> + Workqueues queues work on current cpu, if the caller haven't passed a
> + preferred cpu. This may wake up an idle CPU, which is actually not
> + required. This work can be processed by any CPU and so we must select
> + a non-idle CPU here. This patch adds in support in workqueue
> + framework to get preferred CPU details from the scheduler, instead of
> + using current CPU.
I don't think it's a good idea to make behavior like this a config
option. The behavior difference is subtle and may induce incorrect
behavior.
> +/* This enables migration of a work to a non-IDLE cpu instead of current cpu
> */
> +#ifdef CONFIG_MIGRATE_WQ
> +static int wq_select_cpu(void)
> +{
> + return sched_select_cpu(SD_NUMA, -1);
> +}
> +#else
> +#define wq_select_cpu() smp_processor_id()
> +#endif
> +
> /* Serializes the accesses to the list of workqueues. */
> static DEFINE_SPINLOCK(workqueue_lock);
> static LIST_HEAD(workqueues);
> @@ -995,7 +1005,7 @@ static void __queue_work(unsigned int cpu, struct
> workqueue_struct *wq,
> struct global_cwq *last_gcwq;
>
> if (unlikely(cpu == WORK_CPU_UNBOUND))
> - cpu = raw_smp_processor_id();
> + cpu = wq_select_cpu();
>
> /*
>* It's multi cpu. If @wq is non-reentrant and @work
> @@ -1066,8 +1076,9 @@ int queue_work(struct workqueue_struct *wq, struct
> work_struct *work)
> {
> int ret;
>
> - ret = queue_work_on(get_cpu(), wq, work);
> - put_cpu();
> + preempt_disable();
> + ret = queue_work_on(wq_select_cpu(), wq, work);
> + preempt_enable();
First of all, I'm not entirely sure this is safe. queue_work() used
to *guarantee* that the work item would execute on the local CPU. I
don't think there are many which depend on that but I'd be surprised
if this doesn't lead to some subtle problems somewhere. It might not
be realistic to audit all users and we might have to just let it
happen and watch for the fallouts. Dunno, still wanna see some level
of auditing.
Also, I'm wondering why this is necessary at all for workqueues. For
schedule/queue_work(), you pretty much know the current cpu is not
idle. For delayed workqueue, sure but for immediate scheduling, why?
Thanks.
--
tejun
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
2012-09-26 Android Platform Team Meeting Agenda Posted
Please take a look at: https://wiki.linaro.org/Platform/Android/Meetings/2012-09-26 Feel free to add to the agenda and join us in #linaro-meeting on irc.freenode.net at 13:00 UTC on 2012/9/26. -- Zach Pfeffer Android Platform Team Lead, Linaro Platform Teams Linaro.org | Open source software for ARM SoCs Follow Linaro: http://www.facebook.com/pages/Linaro http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 2/3] workqueue: create __flush_delayed_work to avoid duplicating code
On 25 September 2012 23:17, Tejun Heo wrote: > On Tue, Sep 25, 2012 at 04:06:07PM +0530, Viresh Kumar wrote: >> flush_delayed_work() and flush_delayed_work_sync() had major portion of code >> similar. This patch introduces another routine __flush_delayed_work() which >> contains the common part to avoid code duplication. > > This part has seen a lot of update in pending wq/for-3.7 branch. > Please rebase on top of that. > > git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-3.7 So, this patch is not required anymore. As they are already merged :) ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
