diff mbox

[01/14] xen: sched: __runq_tickle takes a useless cpu parameter

Message ID 20160205183334.4543.50511.stgit@Solace.station
State New, archived
Headers show

Commit Message

Dario Faggioli Feb. 5, 2016, 6:33 p.m. UTC
as it is always acts on v->processor of the vcpu because
of which we are tickling.

Getting rid of it makes the code easier to understand
and better looking.

While there, remove a spurious blank line.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@citrix.com>
---
 xen/common/sched_credit.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Konrad Rzeszutek Wilk Feb. 15, 2016, 4:14 p.m. UTC | #1
On Fri, Feb 05, 2016 at 07:33:34PM +0100, Dario Faggioli wrote:
> as it is always acts on v->processor of the vcpu because
> of which we are tickling.

s/because of which// ?

> 
> Getting rid of it makes the code easier to understand
> and better looking.
> 
> While there, remove a spurious blank line.
> 

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> ---
> Cc: George Dunlap <george.dunlap@citrix.com>
> ---
>  xen/common/sched_credit.c |   10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> index 671bbee..5279b92 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -360,9 +360,9 @@ boolean_param("tickle_one_idle_cpu", opt_tickle_one_idle);
>  
>  DEFINE_PER_CPU(unsigned int, last_tickle_cpu);
>  
> -static inline void
> -__runq_tickle(unsigned int cpu, struct csched_vcpu *new)
> +static inline void __runq_tickle(struct csched_vcpu *new)
>  {
> +    unsigned int cpu = new->vcpu->processor;
>      struct csched_vcpu * const cur = CSCHED_VCPU(curr_on_cpu(cpu));
>      struct csched_private *prv = CSCHED_PRIV(per_cpu(scheduler, cpu));
>      cpumask_t mask, idle_mask, *online;
> @@ -375,7 +375,6 @@ __runq_tickle(unsigned int cpu, struct csched_vcpu *new)
>      cpumask_and(&idle_mask, prv->idlers, online);
>      idlers_empty = cpumask_empty(&idle_mask);
>  
> -
>      /*
>       * If the pcpu is idle, or there are no idlers and the new
>       * vcpu is a higher priority than the old vcpu, run it here.
> @@ -980,11 +979,10 @@ static void
>  csched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
>  {
>      struct csched_vcpu * const svc = CSCHED_VCPU(vc);
> -    const unsigned int cpu = vc->processor;
>  
>      BUG_ON( is_idle_vcpu(vc) );
>  
> -    if ( unlikely(curr_on_cpu(cpu) == vc) )
> +    if ( unlikely(curr_on_cpu(vc->processor) == vc) )
>      {
>          SCHED_STAT_CRANK(vcpu_wake_running);
>          return;
> @@ -1028,7 +1026,7 @@ csched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
>  
>      /* Put the VCPU on the runq and tickle CPUs */
>      __runq_insert(svc);
> -    __runq_tickle(cpu, svc);
> +    __runq_tickle(svc);
>  }
>  
>  static void
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Dario Faggioli Feb. 15, 2016, 4:29 p.m. UTC | #2
On Mon, 2016-02-15 at 11:14 -0500, Konrad Rzeszutek Wilk wrote:
> On Fri, Feb 05, 2016 at 07:33:34PM +0100, Dario Faggioli wrote:
> > as it is always acts on v->processor of the vcpu because
> > of which we are tickling.
> 
> s/because of which// ?
> 
Yeah, well, technically, as tickling means "raising the
SCHEDULE_SOFTIRQ on a pcpu", I tend to see and describe it as "we
tickled pcpu X because of vcpu k woke up".

But this may well be me, and I'm fine both keeping or killing that
"because of which".

> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
Thanks,
Dario
diff mbox

Patch

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 671bbee..5279b92 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -360,9 +360,9 @@  boolean_param("tickle_one_idle_cpu", opt_tickle_one_idle);
 
 DEFINE_PER_CPU(unsigned int, last_tickle_cpu);
 
-static inline void
-__runq_tickle(unsigned int cpu, struct csched_vcpu *new)
+static inline void __runq_tickle(struct csched_vcpu *new)
 {
+    unsigned int cpu = new->vcpu->processor;
     struct csched_vcpu * const cur = CSCHED_VCPU(curr_on_cpu(cpu));
     struct csched_private *prv = CSCHED_PRIV(per_cpu(scheduler, cpu));
     cpumask_t mask, idle_mask, *online;
@@ -375,7 +375,6 @@  __runq_tickle(unsigned int cpu, struct csched_vcpu *new)
     cpumask_and(&idle_mask, prv->idlers, online);
     idlers_empty = cpumask_empty(&idle_mask);
 
-
     /*
      * If the pcpu is idle, or there are no idlers and the new
      * vcpu is a higher priority than the old vcpu, run it here.
@@ -980,11 +979,10 @@  static void
 csched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
 {
     struct csched_vcpu * const svc = CSCHED_VCPU(vc);
-    const unsigned int cpu = vc->processor;
 
     BUG_ON( is_idle_vcpu(vc) );
 
-    if ( unlikely(curr_on_cpu(cpu) == vc) )
+    if ( unlikely(curr_on_cpu(vc->processor) == vc) )
     {
         SCHED_STAT_CRANK(vcpu_wake_running);
         return;
@@ -1028,7 +1026,7 @@  csched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
 
     /* Put the VCPU on the runq and tickle CPUs */
     __runq_insert(svc);
-    __runq_tickle(cpu, svc);
+    __runq_tickle(svc);
 }
 
 static void