Re: Fix for HMP scheduler crash [ Re: [GIT PULL]: big LITTLE MP v10]

2012-10-12 Thread Jon Medhurst (Tixy) 
On Fri, 2012-10-12 at 14:19 +0100, Jon Medhurst (Tixy) wrote:
 The attached patch fixes the immediate problem by avoiding the empty
 domain (which is probably a good thing anyway)

Oops, my last patch included some extra junk, the one attached to this
mail fixes this...
From 7365076675b851355d48e9b1157e223d7719e3ac Mon Sep 17 00:00:00 2001
From: Jon Medhurst t...@linaro.org
Date: Fri, 12 Oct 2012 13:45:35 +0100
Subject: [PATCH] ARM: sched: Avoid empty 'slow' HMP domain

On homogeneous (non-heterogeneous) systems all CPUs will be declared
'fast' and the slow cpu list will be empty. In this situation we need to
avoid adding an empty slow HMP domain otherwise the scheduler code will
blow up when it attempts to move a task to the slow domain.

Signed-off-by: Jon Medhurst t...@linaro.org
---
 arch/arm/kernel/topology.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 58dac7a..0b51233 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -396,10 +396,12 @@ void __init arch_get_hmp_domains(struct list_head *hmp_domains_list)
 	 * Must be ordered with respect to compute capacity.
 	 * Fastest domain at head of list.
 	 */
-	domain = (struct hmp_domain *)
-		kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
-	cpumask_copy(domain-cpus, hmp_slow_cpu_mask);
-	list_add(domain-hmp_domains, hmp_domains_list);
+	if(!cpumask_empty(hmp_slow_cpu_mask)) {
+		domain = (struct hmp_domain *)
+			kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
+		cpumask_copy(domain-cpus, hmp_slow_cpu_mask);
+		list_add(domain-hmp_domains, hmp_domains_list);
+	}
 	domain = (struct hmp_domain *)
 		kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
 	cpumask_copy(domain-cpus, hmp_fast_cpu_mask);
-- 
1.7.10.4

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

Re: Fix for HMP scheduler crash [ Re: [GIT PULL]: big LITTLE MP v10]

2012-10-12 Thread Viresh Kumar 
On 12 October 2012 19:03, Jon Medhurst (Tixy) t...@linaro.org wrote:
 On Fri, 2012-10-12 at 14:19 +0100, Jon Medhurst (Tixy) wrote:
 The attached patch fixes the immediate problem by avoiding the empty
 domain (which is probably a good thing anyway)

 Oops, my last patch included some extra junk, the one attached to this
 mail fixes this...

Tixy, do let me know if i should send another pull request with this
one applied.

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

Re: Fix for HMP scheduler crash [ Re: [GIT PULL]: big LITTLE MP v10]

2012-10-12 Thread Andrey Konovalov 

On 10/12/2012 05:41 PM, Viresh Kumar wrote:

On 12 October 2012 19:03, Jon Medhurst (Tixy) t...@linaro.org wrote:

On Fri, 2012-10-12 at 14:19 +0100, Jon Medhurst (Tixy) wrote:

The attached patch fixes the immediate problem by avoiding the empty
domain (which is probably a good thing anyway)


Oops, my last patch included some extra junk, the one attached to this
mail fixes this...


Tixy, do let me know if i should send another pull request with this
one applied.


I am going to apply the latest Tixy's patch to llct anyway. (so it will 
get into the ll too). Would be easier for me to get it as part of 
big.LITTLE-MP topic though. If it woun't be in big.LITTLE-MP, will apply 
to llct myself.
Today's linux-linaro trees are not RCs yet, so we should better try it 
now then right before the release. If there are issues there, there is 
one week (till Oct 18) to adjust the patch.


Thanks,
Andrey


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

Re: Fix for HMP scheduler crash [ Re: [GIT PULL]: big LITTLE MP v10]

2012-10-12 Thread Jon Medhurst (Tixy) 
On Fri, 2012-10-12 at 19:11 +0530, Viresh Kumar wrote:
 On 12 October 2012 19:03, Jon Medhurst (Tixy) t...@linaro.org wrote:
  On Fri, 2012-10-12 at 14:19 +0100, Jon Medhurst (Tixy) wrote:
  The attached patch fixes the immediate problem by avoiding the empty
  domain (which is probably a good thing anyway)
 
  Oops, my last patch included some extra junk, the one attached to this
  mail fixes this...
 
 Tixy, do let me know if i should send another pull request with this
 one applied.

It will need fixing one way or the other, and it makes more sense to be
part of the MP branch. I was hoping for a second opinion from someone,
but if the fix looks OK to you, then yes, please add it to your branch.

I've boot tested this patch with an Android kernel running on TC2 and A9
CoreTiles.

--- 
Tixy


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

Re: Fix for HMP scheduler crash [ Re: [GIT PULL]: big LITTLE MP v10]

2012-10-12 Thread Viresh Kumar 
On 12 October 2012 19:30, Jon Medhurst (Tixy) t...@linaro.org wrote:
 It will need fixing one way or the other, and it makes more sense to be
 part of the MP branch. I was hoping for a second opinion from someone,
 but if the fix looks OK to you, then yes, please add it to your branch.

Ok I have applied this patch to my tree now.

@Andrey: Please pull my branch now. :)

 I've boot tested this patch with an Android kernel running on TC2 and A9
 CoreTiles.

I couldn't test it as am not in office.

--
viresh

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

Re: Fix for HMP scheduler crash [ Re: [GIT PULL]: big LITTLE MP v10]

2012-10-12 Thread Morten Rasmussen 
Hi Tixy,

Thanks for the patch. I think this patch is the right way to solve this
issue.

There is still a problem with the priority filter in
hmp_down_migration() which Viresh pointed out earlier. There is no
checking of whether the task is actually allowed to run on any of the
slower cpus. Solving that would actually also fix the issue that you are
observing as a side effect. I have attached a patch.

I think we should apply both.

Thanks,
Morten

On Fri, Oct 12, 2012 at 02:33:40PM +0100, Jon Medhurst (Tixy) wrote:
 On Fri, 2012-10-12 at 14:19 +0100, Jon Medhurst (Tixy) wrote:
  The attached patch fixes the immediate problem by avoiding the empty
  domain (which is probably a good thing anyway)
 
 Oops, my last patch included some extra junk, the one attached to this
 mail fixes this...

 From 7365076675b851355d48e9b1157e223d7719e3ac Mon Sep 17 00:00:00 2001
 From: Jon Medhurst t...@linaro.org
 Date: Fri, 12 Oct 2012 13:45:35 +0100
 Subject: [PATCH] ARM: sched: Avoid empty 'slow' HMP domain
 
 On homogeneous (non-heterogeneous) systems all CPUs will be declared
 'fast' and the slow cpu list will be empty. In this situation we need to
 avoid adding an empty slow HMP domain otherwise the scheduler code will
 blow up when it attempts to move a task to the slow domain.
 
 Signed-off-by: Jon Medhurst t...@linaro.org
 ---
  arch/arm/kernel/topology.c |   10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)
 
 diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
 index 58dac7a..0b51233 100644
 --- a/arch/arm/kernel/topology.c
 +++ b/arch/arm/kernel/topology.c
 @@ -396,10 +396,12 @@ void __init arch_get_hmp_domains(struct list_head 
 *hmp_domains_list)
* Must be ordered with respect to compute capacity.
* Fastest domain at head of list.
*/
 - domain = (struct hmp_domain *)
 - kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
 - cpumask_copy(domain-cpus, hmp_slow_cpu_mask);
 - list_add(domain-hmp_domains, hmp_domains_list);
 + if(!cpumask_empty(hmp_slow_cpu_mask)) {
 + domain = (struct hmp_domain *)
 + kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
 + cpumask_copy(domain-cpus, hmp_slow_cpu_mask);
 + list_add(domain-hmp_domains, hmp_domains_list);
 + }
   domain = (struct hmp_domain *)
   kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
   cpumask_copy(domain-cpus, hmp_fast_cpu_mask);
 -- 
 1.7.10.4
From 9f241c37bb7316eeea56e6c93541352cf5c9b8a8 Mon Sep 17 00:00:00 2001
From: Morten Rasmussen morten.rasmus...@arm.com
Date: Fri, 12 Oct 2012 15:25:02 +0100
Subject: [PATCH] sched: Only down migrate low priority tasks if allowed by
 affinity mask

Adds an extra check intersection of the task affinity mask and the slower
hmp_domain cpumask before down migrating low priority tasks.

Signed-off-by: Morten Rasmussen morten.rasmus...@arm.com
---
 kernel/sched/fair.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 56cbda1..edcf922 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5562,8 +5562,11 @@ static unsigned int hmp_down_migration(int cpu, struct sched_entity *se)
 
 #ifdef CONFIG_SCHED_HMP_PRIO_FILTER
 	/* Filter by task priority */
-	if (p-prio = hmp_up_prio)
+	if ((p-prio = hmp_up_prio) 
+		cpumask_intersects(hmp_slower_domain(cpu)-cpus,
+	tsk_cpus_allowed(p))) {
 		return 1;
+	}
 #endif
 
 	/* Let the task load settle before doing another down migration */
-- 
1.7.9.5
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Re: Fix for HMP scheduler crash [ Re: [GIT PULL]: big LITTLE MP v10]

2012-10-12 Thread Viresh Kumar 
On 12 October 2012 20:41, Morten Rasmussen morten.rasmus...@arm.com wrote:
 Thanks for the patch. I think this patch is the right way to solve this
 issue.

 There is still a problem with the priority filter in
 hmp_down_migration() which Viresh pointed out earlier. There is no
 checking of whether the task is actually allowed to run on any of the
 slower cpus. Solving that would actually also fix the issue that you are
 observing as a side effect. I have attached a patch.

@Andrey: I have applied this too. Please PULL it when you feel Tixy 
Morten don't have any more patches to send :)

--
viresh

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

Re: Fix for HMP scheduler crash [ Re: [GIT PULL]: big LITTLE MP v10]

2012-10-12 Thread Jon Medhurst (Tixy) 
On Fri, 2012-10-12 at 16:11 +0100, Morten Rasmussen wrote:
 Hi Tixy,
 
 Thanks for the patch. I think this patch is the right way to solve this
 issue.
 
 There is still a problem with the priority filter in
 hmp_down_migration() which Viresh pointed out earlier. There is no
 checking of whether the task is actually allowed to run on any of the
 slower cpus. Solving that would actually also fix the issue that you are
 observing as a side effect. I have attached a patch.

The patch looks reasonable. I've just run it on TC2 and A9 with the
addition of a pr_err($); before the return 1; and can see the
occosional '$' on TC2 and none on A9, as we would expect. So I guess
that counts as:

Reviewed-by: Jon Medhurst t...@linaro.org
Tested-by: Jon Medhurst t...@linaro.org

-- 
Tixy


 I think we should apply both.
 
 Thanks,
 Morten
 
 On Fri, Oct 12, 2012 at 02:33:40PM +0100, Jon Medhurst (Tixy) wrote:
  On Fri, 2012-10-12 at 14:19 +0100, Jon Medhurst (Tixy) wrote:
   The attached patch fixes the immediate problem by avoiding the empty
   domain (which is probably a good thing anyway)
  
  Oops, my last patch included some extra junk, the one attached to this
  mail fixes this...
 
  From 7365076675b851355d48e9b1157e223d7719e3ac Mon Sep 17 00:00:00 2001
  From: Jon Medhurst t...@linaro.org
  Date: Fri, 12 Oct 2012 13:45:35 +0100
  Subject: [PATCH] ARM: sched: Avoid empty 'slow' HMP domain
  
  On homogeneous (non-heterogeneous) systems all CPUs will be declared
  'fast' and the slow cpu list will be empty. In this situation we need to
  avoid adding an empty slow HMP domain otherwise the scheduler code will
  blow up when it attempts to move a task to the slow domain.
  
  Signed-off-by: Jon Medhurst t...@linaro.org
  ---
   arch/arm/kernel/topology.c |   10 ++
   1 file changed, 6 insertions(+), 4 deletions(-)
  
  diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
  index 58dac7a..0b51233 100644
  --- a/arch/arm/kernel/topology.c
  +++ b/arch/arm/kernel/topology.c
  @@ -396,10 +396,12 @@ void __init arch_get_hmp_domains(struct list_head 
  *hmp_domains_list)
   * Must be ordered with respect to compute capacity.
   * Fastest domain at head of list.
   */
  -   domain = (struct hmp_domain *)
  -   kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
  -   cpumask_copy(domain-cpus, hmp_slow_cpu_mask);
  -   list_add(domain-hmp_domains, hmp_domains_list);
  +   if(!cpumask_empty(hmp_slow_cpu_mask)) {
  +   domain = (struct hmp_domain *)
  +   kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
  +   cpumask_copy(domain-cpus, hmp_slow_cpu_mask);
  +   list_add(domain-hmp_domains, hmp_domains_list);
  +   }
  domain = (struct hmp_domain *)
  kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
  cpumask_copy(domain-cpus, hmp_fast_cpu_mask);
  -- 
  1.7.10.4



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

Re: Fix for HMP scheduler crash [ Re: [GIT PULL]: big LITTLE MP v10]

2012-10-12 Thread Jon Medhurst (Tixy) 
On Fri, 2012-10-12 at 21:03 +0530, Viresh Kumar wrote:
 On 12 October 2012 20:41, Morten Rasmussen morten.rasmus...@arm.com wrote:
  Thanks for the patch. I think this patch is the right way to solve this
  issue.
 
  There is still a problem with the priority filter in
  hmp_down_migration() which Viresh pointed out earlier. There is no
  checking of whether the task is actually allowed to run on any of the
  slower cpus. Solving that would actually also fix the issue that you are
  observing as a side effect. I have attached a patch.
 
 @Andrey: I have applied this too. Please PULL it when you feel Tixy 
 Morten don't have any more patches to send :)

Well, I still have one more issue which _may_ be related, but if it is I
don't think I'm going to be producing any more patches before Monday
anyway. The issue? ...

Android doesn't boot on RTSM A15x1-A7x1, some processes are dying for
currently unknown reasons. But the same kernel with the same filesystem
works on RTSM A15x4-A7x4, RTSM A15x1 and RTSM A15x4, and the same kernel
works on real hardware TC2 and A9x4.

Perhaps it's not related to the MP branch, but as the system which fails
is one core in each cluster it's a topology which might be exposing a
bug in the code.

-- 
Tixy 


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

Re: Fix for HMP scheduler crash [ Re: [GIT PULL]: big LITTLE MP v10]

2012-10-12 Thread Morten Rasmussen 
On Fri, Oct 12, 2012 at 04:33:19PM +0100, Jon Medhurst (Tixy) wrote:
 On Fri, 2012-10-12 at 16:11 +0100, Morten Rasmussen wrote:
  Hi Tixy,
  
  Thanks for the patch. I think this patch is the right way to solve this
  issue.
  
  There is still a problem with the priority filter in
  hmp_down_migration() which Viresh pointed out earlier. There is no
  checking of whether the task is actually allowed to run on any of the
  slower cpus. Solving that would actually also fix the issue that you are
  observing as a side effect. I have attached a patch.
 
 The patch looks reasonable. I've just run it on TC2 and A9 with the
 addition of a pr_err($); before the return 1; and can see the
 occosional '$' on TC2 and none on A9, as we would expect. So I guess
 that counts as:
 
 Reviewed-by: Jon Medhurst t...@linaro.org
 Tested-by: Jon Medhurst t...@linaro.org


Thanks for reviewing and testing.

My comments to your patch in the previous reply would count as:

Reviewed-by: Morten Rasmussen morten.rasmus...@arm.com

I have only tested it on TC2.

Morten
 
 -- 
 Tixy
 
 
  I think we should apply both.
  
  Thanks,
  Morten
  
  On Fri, Oct 12, 2012 at 02:33:40PM +0100, Jon Medhurst (Tixy) wrote:
   On Fri, 2012-10-12 at 14:19 +0100, Jon Medhurst (Tixy) wrote:
The attached patch fixes the immediate problem by avoiding the empty
domain (which is probably a good thing anyway)
   
   Oops, my last patch included some extra junk, the one attached to this
   mail fixes this...
  
   From 7365076675b851355d48e9b1157e223d7719e3ac Mon Sep 17 00:00:00 2001
   From: Jon Medhurst t...@linaro.org
   Date: Fri, 12 Oct 2012 13:45:35 +0100
   Subject: [PATCH] ARM: sched: Avoid empty 'slow' HMP domain
   
   On homogeneous (non-heterogeneous) systems all CPUs will be declared
   'fast' and the slow cpu list will be empty. In this situation we need to
   avoid adding an empty slow HMP domain otherwise the scheduler code will
   blow up when it attempts to move a task to the slow domain.
   
   Signed-off-by: Jon Medhurst t...@linaro.org
   ---
arch/arm/kernel/topology.c |   10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
   
   diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
   index 58dac7a..0b51233 100644
   --- a/arch/arm/kernel/topology.c
   +++ b/arch/arm/kernel/topology.c
   @@ -396,10 +396,12 @@ void __init arch_get_hmp_domains(struct list_head 
   *hmp_domains_list)
  * Must be ordered with respect to compute capacity.
  * Fastest domain at head of list.
  */
   - domain = (struct hmp_domain *)
   - kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
   - cpumask_copy(domain-cpus, hmp_slow_cpu_mask);
   - list_add(domain-hmp_domains, hmp_domains_list);
   + if(!cpumask_empty(hmp_slow_cpu_mask)) {
   + domain = (struct hmp_domain *)
   + kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
   + cpumask_copy(domain-cpus, hmp_slow_cpu_mask);
   + list_add(domain-hmp_domains, hmp_domains_list);
   + }
 domain = (struct hmp_domain *)
 kmalloc(sizeof(struct hmp_domain), GFP_KERNEL);
 cpumask_copy(domain-cpus, hmp_fast_cpu_mask);
   -- 
   1.7.10.4
 
 
 


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