Re: [Xen-devel] [PATCH 05/16] xen: sched: move pCPU initialization in an helper

2016-03-24 Thread Dario Faggioli 
On Wed, 2016-03-23 at 17:51 +, George Dunlap wrote:
> On 18/03/16 19:04, Dario Faggioli wrote:
> > --- a/xen/common/sched_credit.c
> > +++ b/xen/common/sched_credit.c
> > @@ -542,16 +542,11 @@ csched_alloc_pdata(const struct scheduler
> > *ops, int cpu)
> >  }
> >  
> >  static void
> > -csched_init_pdata(const struct scheduler *ops, void *pdata, int
> > cpu)
> > +init_pdata(struct csched_private *prv, struct csched_pcpu *spc,
> > int cpu)
> >  {
> > -struct csched_private *prv = CSCHED_PRIV(ops);
> > -struct csched_pcpu * const spc = pdata;
> > -unsigned long flags;
> > -
> > -/* cpu data needs to be allocated, but STILL uninitialized */
> > -ASSERT(spc && spc->runq.next == spc->runq.prev && spc-
> > >runq.next == NULL);
> > -
> > -spin_lock_irqsave(&prv->lock, flags);
> > +ASSERT(spin_is_locked(&prv->lock));
> > +/* cpu data needs to be allocated, but STILL uninitialized. */
> > +ASSERT(spc && spc->runq.next == NULL && spc->runq.prev ==
> > NULL);
> Actually, Juergen, looks like Dario already agrees with us. ;-)
> 
Indeed I do! :-)

> Obviously this should be updated in the previous patch instead.
> 
Yeah, sorry. Rebasing / patch-splitting gone wrong! :-/

> With that done:
> 
> Reviewed-by: George Dunlap 
> 
I'll do that.

Thanks and Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 05/16] xen: sched: move pCPU initialization in an helper

2016-03-23 Thread George Dunlap 
On 23/03/16 17:51, George Dunlap wrote:
> On 18/03/16 19:04, Dario Faggioli wrote:
>> That will turn out useful in following patches, where such
>> code will need to be called more than just once. Create an
>> helper now, and move the code there, to avoid mixing code
>> motion and functional changes later.
>>
>> In Credit2, some style cleanup is also done.
>>
>> No functional change intended.
>>
>> Signed-off-by: Dario Faggioli 
>> ---
>> Cc: George Dunlap 
>> ---
>>  xen/common/sched_credit.c  |   22 +-
>>  xen/common/sched_credit2.c |   26 --
>>  2 files changed, 29 insertions(+), 19 deletions(-)
>>
>> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
>> index d4a0f5e..4488d7c 100644
>> --- a/xen/common/sched_credit.c
>> +++ b/xen/common/sched_credit.c
>> @@ -542,16 +542,11 @@ csched_alloc_pdata(const struct scheduler *ops, int 
>> cpu)
>>  }
>>  
>>  static void
>> -csched_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
>> +init_pdata(struct csched_private *prv, struct csched_pcpu *spc, int cpu)
>>  {
>> -struct csched_private *prv = CSCHED_PRIV(ops);
>> -struct csched_pcpu * const spc = pdata;
>> -unsigned long flags;
>> -
>> -/* cpu data needs to be allocated, but STILL uninitialized */
>> -ASSERT(spc && spc->runq.next == spc->runq.prev && spc->runq.next == 
>> NULL);
>> -
>> -spin_lock_irqsave(&prv->lock, flags);
>> +ASSERT(spin_is_locked(&prv->lock));
>> +/* cpu data needs to be allocated, but STILL uninitialized. */
>> +ASSERT(spc && spc->runq.next == NULL && spc->runq.prev == NULL);
> 
> Actually, Juergen, looks like Dario already agrees with us. ;-)
> 
> Obviously this should be updated in the previous patch instead.
> 
> With that done:
> 
> Reviewed-by: George Dunlap 

Oops -- somehow still have Juergen's Fujitsu address in my addressbook...

 -G

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 05/16] xen: sched: move pCPU initialization in an helper

2016-03-23 Thread George Dunlap 
On 18/03/16 19:04, Dario Faggioli wrote:
> That will turn out useful in following patches, where such
> code will need to be called more than just once. Create an
> helper now, and move the code there, to avoid mixing code
> motion and functional changes later.
> 
> In Credit2, some style cleanup is also done.
> 
> No functional change intended.
> 
> Signed-off-by: Dario Faggioli 
> ---
> Cc: George Dunlap 
> ---
>  xen/common/sched_credit.c  |   22 +-
>  xen/common/sched_credit2.c |   26 --
>  2 files changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> index d4a0f5e..4488d7c 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -542,16 +542,11 @@ csched_alloc_pdata(const struct scheduler *ops, int cpu)
>  }
>  
>  static void
> -csched_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
> +init_pdata(struct csched_private *prv, struct csched_pcpu *spc, int cpu)
>  {
> -struct csched_private *prv = CSCHED_PRIV(ops);
> -struct csched_pcpu * const spc = pdata;
> -unsigned long flags;
> -
> -/* cpu data needs to be allocated, but STILL uninitialized */
> -ASSERT(spc && spc->runq.next == spc->runq.prev && spc->runq.next == 
> NULL);
> -
> -spin_lock_irqsave(&prv->lock, flags);
> +ASSERT(spin_is_locked(&prv->lock));
> +/* cpu data needs to be allocated, but STILL uninitialized. */
> +ASSERT(spc && spc->runq.next == NULL && spc->runq.prev == NULL);

Actually, Juergen, looks like Dario already agrees with us. ;-)

Obviously this should be updated in the previous patch instead.

With that done:

Reviewed-by: George Dunlap 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 05/16] xen: sched: move pCPU initialization in an helper

2016-03-19 Thread Dario Faggioli 
That will turn out useful in following patches, where such
code will need to be called more than just once. Create an
helper now, and move the code there, to avoid mixing code
motion and functional changes later.

In Credit2, some style cleanup is also done.

No functional change intended.

Signed-off-by: Dario Faggioli 
---
Cc: George Dunlap 
---
 xen/common/sched_credit.c  |   22 +-
 xen/common/sched_credit2.c |   26 --
 2 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index d4a0f5e..4488d7c 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -542,16 +542,11 @@ csched_alloc_pdata(const struct scheduler *ops, int cpu)
 }
 
 static void
-csched_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
+init_pdata(struct csched_private *prv, struct csched_pcpu *spc, int cpu)
 {
-struct csched_private *prv = CSCHED_PRIV(ops);
-struct csched_pcpu * const spc = pdata;
-unsigned long flags;
-
-/* cpu data needs to be allocated, but STILL uninitialized */
-ASSERT(spc && spc->runq.next == spc->runq.prev && spc->runq.next == NULL);
-
-spin_lock_irqsave(&prv->lock, flags);
+ASSERT(spin_is_locked(&prv->lock));
+/* cpu data needs to be allocated, but STILL uninitialized. */
+ASSERT(spc && spc->runq.next == NULL && spc->runq.prev == NULL);
 
 /* Initialize/update system-wide config */
 prv->credit += prv->credits_per_tslice;
@@ -575,7 +570,16 @@ csched_init_pdata(const struct scheduler *ops, void 
*pdata, int cpu)
 /* Start off idling... */
 BUG_ON(!is_idle_vcpu(curr_on_cpu(cpu)));
 cpumask_set_cpu(cpu, prv->idlers);
+}
 
+static void
+csched_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
+{
+unsigned long flags;
+struct csched_private *prv = CSCHED_PRIV(ops);
+
+spin_lock_irqsave(&prv->lock, flags);
+init_pdata(prv, pdata, cpu);
 spin_unlock_irqrestore(&prv->lock, flags);
 }
 
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index b1642a8..919ca13 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1969,16 +1969,13 @@ static void deactivate_runqueue(struct csched2_private 
*prv, int rqi)
 }
 
 static void
-csched2_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
+init_pdata(struct csched2_private *prv, unsigned int cpu)
 {
 unsigned rqi;
-unsigned long flags;
-struct csched2_private *prv = CSCHED2_PRIV(ops);
 struct csched2_runqueue_data *rqd;
 spinlock_t *old_lock;
 
-spin_lock_irqsave(&prv->lock, flags);
-
+ASSERT(spin_is_locked(&prv->lock));
 ASSERT(!cpumask_test_cpu(cpu, &prv->initialized));
 
 /* Figure out which runqueue to put it in */
@@ -1998,7 +1995,7 @@ csched2_init_pdata(const struct scheduler *ops, void 
*pdata, int cpu)
 BUG();
 }
 
-rqd=prv->rqd + rqi;
+rqd = prv->rqd + rqi;
 
 printk("Adding cpu %d to runqueue %d\n", cpu, rqi);
 if ( ! cpumask_test_cpu(rqi, &prv->active_queues) )
@@ -2008,13 +2005,13 @@ csched2_init_pdata(const struct scheduler *ops, void 
*pdata, int cpu)
 }
 
 /* IRQs already disabled */
-old_lock=pcpu_schedule_lock(cpu);
+old_lock = pcpu_schedule_lock(cpu);
 
 /* Move spinlock to new runq lock.  */
 per_cpu(schedule_data, cpu).schedule_lock = &rqd->lock;
 
 /* Set the runqueue map */
-prv->runq_map[cpu]=rqi;
+prv->runq_map[cpu] = rqi;
 
 cpumask_set_cpu(cpu, &rqd->idle);
 cpumask_set_cpu(cpu, &rqd->active);
@@ -2024,12 +2021,21 @@ csched2_init_pdata(const struct scheduler *ops, void 
*pdata, int cpu)
 
 cpumask_set_cpu(cpu, &prv->initialized);
 
-spin_unlock_irqrestore(&prv->lock, flags);
-
 return;
 }
 
 static void
+csched2_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
+{
+struct csched2_private *prv = CSCHED2_PRIV(ops);
+unsigned long flags;
+
+spin_lock_irqsave(&prv->lock, flags);
+init_pdata(prv, cpu);
+spin_unlock_irqrestore(&prv->lock, flags);
+}
+
+static void
 csched2_free_pdata(const struct scheduler *ops, void *pcpu, int cpu)
 {
 unsigned long flags;


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel