[PATCH][RFC] consider vcpu-pin weight on Credit Scheduler TAKE2

[PATCH][RFC] consider vcpu-pin weight on Credit Scheduler TAKE2

[Atsushi SAKAI]

[-- Attachment #1: Type: text/plain, Size: 1367 bytes --]

Hi, Keir

This patch intends 
to consider vcpu-pin weight on credit scheduler TAKE2.
http://lists.xensource.com/archives/html/xen-devel/2007-06/msg00359.html

The difference from previous one is
1) Coding style clean up
2) Skip loop for unused vcpu-pin-count.
3) Remove if pin_count ==1 in multiple loop.
   Then pin_count ==1 is another loop.

Signed-off-by: Atsushi SAKAI <sakaia@jp.fujitsu.com>

And one question, 
Does this patch need following tune up for reducing multiple loop?

>From following

-  /* sort weight */
-  for(j=0;j<pin_count;j++)
-  {
-      sortflag = 0;
-      for(k=1;k<pin_count;k++)
-      {
-          if ( pcpu_weight[pcpu_id_list[k-1]] > pcpu_weight[pcpu_id_list[k]] 
)
-          {
-              sortflag = 1;
-              pcpu_id_handle  = pcpu_id_list[k-1];
-              pcpu_id_list[k-1] = pcpu_id_list[k];
-              pcpu_id_list[k]   = pcpu_id_handle;
-          }
-      }
-      if( sortflag == 0)break;
-  }

To following

+     /* sort weight */
+     for(k=1;k<pin_count;k++)
+     {
+          if ( pcpu_weight[pcpu_id_list[k-1]] > pcpu_weight[pcpu_id_list[k]] 
)
+          {
+              pcpu_id_handle  = pcpu_id_list[k-1];
+              pcpu_id_list[k-1] = pcpu_id_list[k];
+              pcpu_id_list[k]   = pcpu_id_handle;
+              if (k > 1) k -= 2;  
+           }
+     }


Thanks
Atsushi SAKAI

 

[-- Attachment #2: vcpupinweight0627.patch --]
[-- Type: application/octet-stream, Size: 17920 bytes --]

diff -r 96331db61e47 xen/common/sched_credit.c
--- a/xen/common/sched_credit.c	Wed Jun 06 09:30:01 2007 -0600
+++ b/xen/common/sched_credit.c	Wed Jun 27 15:16:50 2007 +0900
@@ -201,6 +201,9 @@ struct csched_vcpu {
     atomic_t credit;
     uint16_t flags;
     int16_t pri;
+    uint16_t factor;
+    uint16_t pcpu_id_weight;
+    uint16_t pcpu_id_credit;
 #ifdef CSCHED_STATS
     struct {
         int credit_last;
@@ -214,6 +217,12 @@ struct csched_vcpu {
 };
 
 /*
+ * Default factor correction for vcpu-pin
+ */
+
+#define CSCHED_DEFAULT_FACTOR       256
+
+/*
  * Domain
  */
 struct csched_dom {
@@ -237,6 +246,7 @@ struct csched_private {
     uint32_t weight;
     uint32_t credit;
     int credit_balance;
+    int pcpu_credit_balance[NR_CPUS];
     uint32_t runq_sort;
     CSCHED_STATS_DEFINE()
 };
@@ -818,6 +828,339 @@ csched_runq_sort(unsigned int cpu)
     spin_unlock_irqrestore(&per_cpu(schedule_data, cpu).schedule_lock, flags);
 }
 
+/*
+ * csched_consider_vcpupin()
+ *
+ * IN  sdom->weight
+ *     sdom->active_vcpu_count
+ *     svc->vcpu->cpu_affinity
+ *
+ * OUT svc->factor
+ *     svc->pcpu_id_weight
+ *
+ * If pcpu.credit <= TSLICE && vcpu.credit <= TSLICE satisfies,
+ * credit scheduler works fine.
+ * This case violates when vcpu-pin with not appropriate weight.
+ *
+ * This function consider pcpu.credit >= TSLICE.
+ * In this case, proportionally reduce credit.
+ * This makes each vcpu runs on pcpu with specified proportion.
+ */
+
+static void
+csched_consider_vcpupin(void)
+{
+    unsigned int pcpu_weight[NR_CPUS];
+    unsigned int pcpu_weight_limit;
+    unsigned int pcpu_weight_summing;
+    struct list_head *iter_vcpu, *next_vcpu;
+    struct list_head *iter_sdom, *next_sdom;
+    struct csched_vcpu *svc;
+    struct csched_dom *sdom;
+    cpumask_t cpus;
+    cpumask_t npin_list;
+    int pcpu_id_list[NR_CPUS];
+    int pcpu_id_handle;
+    int pin_count;
+    int vcpu_weight;
+    int weight_add_pcpu_count;
+    int i, j, k;
+    bool_t sortflag;
+    unsigned long factor_multi, factor_divide;
+
+    /* Summming vcpu-pinned weight */
+    for ( i = 0; i < NR_CPUS; i++) pcpu_weight[i]=0;
+    cpus_clear(npin_list);
+
+    /* Consider vcpu-pin for vcpupin == 1*/
+    list_for_each_safe( iter_sdom, next_sdom, &csched_priv.active_sdom )
+    {
+        sdom = list_entry(iter_sdom, struct csched_dom, active_sdom_elem);
+        vcpu_weight = (sdom->weight + sdom->active_vcpu_count - 1) 
+                    / sdom->active_vcpu_count;
+        list_for_each_safe( iter_vcpu, next_vcpu, &sdom->active_vcpu )
+        {
+            svc = list_entry(iter_vcpu, struct csched_vcpu,
+                             active_vcpu_elem);
+            pin_count = cpus_weight(svc->vcpu->cpu_affinity);
+            cpus_and(cpus, cpu_online_map, svc->vcpu->cpu_affinity);
+            cpu_set(pin_count - 1, npin_list);
+            if ( pin_count == 1 )
+            {
+                pcpu_id_handle = NR_CPUS;
+                svc->pcpu_id_weight =
+                    __cycle_cpu(pcpu_id_handle, &cpus);
+                pcpu_weight[svc->pcpu_id_weight] += vcpu_weight;
+            }
+        }
+    }
+
+    /* Consider vcpu-pin for vcpupin >= 2*/
+    for ( i = 2; i < csched_priv.ncpus; i++)
+    {
+        if( cpu_isset(i - 1, npin_list) )
+        {
+            list_for_each_safe( iter_sdom, next_sdom, &csched_priv.active_sdom )
+            {
+                sdom = list_entry(iter_sdom, struct csched_dom, active_sdom_elem);
+                vcpu_weight = (sdom->weight + sdom->active_vcpu_count - 1) 
+                        / sdom->active_vcpu_count;
+                list_for_each_safe( iter_vcpu, next_vcpu, &sdom->active_vcpu )
+                {
+                    svc = list_entry(iter_vcpu, struct csched_vcpu,
+                                     active_vcpu_elem);
+                    pin_count = cpus_weight(svc->vcpu->cpu_affinity);
+                    cpus_and(cpus, cpu_online_map, svc->vcpu->cpu_affinity);
+                    if ( pin_count == i )
+                    {
+                        /* search pinned pcpus */
+                        pcpu_id_handle = NR_CPUS;
+                        for ( j = 0; j < pin_count; j++)
+                        {
+                            pcpu_id_handle =
+                                __cycle_cpu(pcpu_id_handle, &cpus);
+                            pcpu_id_list[j]  = pcpu_id_handle;
+                        }
+                        /* sort weight */
+                        for ( j = 0; j < pin_count; j++)
+                        {
+                            sortflag = 0;
+                            for ( k = 1; k < pin_count; k++)
+                            {
+                                if ( pcpu_weight[pcpu_id_list[k-1]] >
+                                    pcpu_weight[pcpu_id_list[k]] )
+                                {
+                                    sortflag = 1;
+                                    pcpu_id_handle  = pcpu_id_list[k-1];
+                                    pcpu_id_list[k-1] = pcpu_id_list[k];
+                                    pcpu_id_list[k]   = pcpu_id_handle;
+                                }
+                            }
+                            if( sortflag == 0)break;
+                        }
+                        /* add weight to each pcpu_weight */
+                        pcpu_weight_summing = vcpu_weight;
+                        svc->pcpu_id_weight = pcpu_id_list[0];
+                        weight_add_pcpu_count = 1;
+                        for ( j = 0; j < pin_count; j++)
+                        {
+                            /* use mul(j+1) instead of div(j+1)*/
+                            if ( pcpu_weight[pcpu_id_list[j]] * (j+1) <
+                                (pcpu_weight_summing+
+                                     pcpu_weight[pcpu_id_list[j]]) )
+                            {
+                                pcpu_weight_summing +=
+                                    pcpu_weight[pcpu_id_list[j]];
+                                weight_add_pcpu_count  = j+1;
+                            }
+                        }
+                        pcpu_weight_summing = 
+                            (pcpu_weight_summing+weight_add_pcpu_count - 1)
+                            / weight_add_pcpu_count;
+                        for ( j = 0; j < weight_add_pcpu_count; j++)
+                            pcpu_weight[pcpu_id_list[j]] = pcpu_weight_summing;
+                    }
+                }
+            }
+        }
+    }
+
+    /* Calculate vcpu pinned correction factor */
+    pcpu_weight_limit = csched_priv.weight/csched_priv.ncpus;
+    list_for_each_safe( iter_sdom, next_sdom, &csched_priv.active_sdom )
+    {
+        sdom = list_entry(iter_sdom, struct csched_dom, active_sdom_elem);
+        list_for_each_safe( iter_vcpu, next_vcpu, &sdom->active_vcpu )
+        {
+            svc = list_entry(iter_vcpu, struct csched_vcpu, active_vcpu_elem);
+            svc->factor = CSCHED_DEFAULT_FACTOR;
+            pin_count = cpus_weight(svc->vcpu->cpu_affinity);
+            if ( pin_count < csched_priv.ncpus )
+            {
+                if ( pcpu_weight[svc->pcpu_id_weight] > pcpu_weight_limit )
+                {
+                    factor_multi = pcpu_weight_limit;
+                    factor_divide= pcpu_weight[svc->pcpu_id_weight];
+                    if ( csched_priv.credit_balance < 0 &&
+                         csched_priv.credit > 0)
+                    {
+                        factor_multi *= csched_priv.credit;
+                        factor_divide*= 
+                            (csched_priv.credit - csched_priv.credit_balance);
+                    }
+                    if ( svc->pcpu_id_credit <= NR_CPUS )
+                        if ( 
+                    csched_priv.pcpu_credit_balance[svc->pcpu_id_credit] < 0 )
+                        {
+                            factor_multi *= (CSCHED_CREDITS_PER_TSLICE
+                       - csched_priv.pcpu_credit_balance[svc->pcpu_id_credit]);
+                            factor_divide*= CSCHED_CREDITS_PER_TSLICE;
+                        }
+                    svc->factor = svc->factor * factor_multi / factor_divide;
+                }
+            }
+        }
+    }
+}
+
+/*
+ * csched_consider_vcpupin_credit_balance()
+ *
+ * IN 
+ *     svc->credit
+ *     svc->vcpu->cpu_affinity
+ *
+ * OUT
+ *     csched_priv.pcpu_credit_balance
+ *     svc->pcpu_id_credit
+ *
+ * This function calculates the credit balance of each pcpu,
+ * based on vcpupin information.
+ */
+static void
+csched_consider_vcpupin_credit_balance(void)
+{
+    struct list_head *iter_vcpu, *next_vcpu;
+    struct list_head *iter_sdom, *next_sdom;
+    struct csched_vcpu *svc;
+    struct csched_dom *sdom;
+    cpumask_t cpus;
+    cpumask_t npin_list;
+    int pcpu_id_list[NR_CPUS];
+    int pcpu_id_handle;
+    int pin_count;
+    int pcpu_credit[NR_CPUS];
+    int pcpu_credit_summing;
+    int credit_add_pcpu_count;
+    int i, j, k;
+    bool_t sortflag;
+
+    /* Calculate each pcpu_credit_balance for vcpu pinned correction factor */
+    for ( i = 0; i < NR_CPUS; i++) pcpu_credit[i]=0;
+    cpus_clear(npin_list);
+
+    list_for_each_safe( iter_sdom, next_sdom, &csched_priv.active_sdom )
+    {
+        sdom = list_entry(iter_sdom, struct csched_dom, active_sdom_elem);
+        list_for_each_safe( iter_vcpu, next_vcpu, &sdom->active_vcpu )
+        {
+            svc = list_entry(iter_vcpu, struct csched_vcpu,
+                            active_vcpu_elem);
+            pin_count = cpus_weight(svc->vcpu->cpu_affinity);
+            cpus_and(cpus, cpu_online_map, svc->vcpu->cpu_affinity);
+            cpu_set(pin_count - 1, npin_list);
+            if ( pin_count == 1 )
+            {
+                pcpu_id_handle = NR_CPUS;
+                svc->pcpu_id_credit =
+                    __cycle_cpu(pcpu_id_handle, &cpus);
+                pcpu_credit[svc->pcpu_id_credit] +=
+                    atomic_read(&svc->credit);
+            }
+        }
+    }
+
+    for ( i = 2; i < csched_priv.ncpus; i++)
+    {
+        if( cpu_isset(i - 1, npin_list) )
+        {
+            list_for_each_safe( iter_sdom, next_sdom, &csched_priv.active_sdom )
+            {
+                sdom = list_entry(iter_sdom, struct csched_dom, active_sdom_elem);
+                list_for_each_safe( iter_vcpu, next_vcpu, &sdom->active_vcpu )
+                {
+                    svc = list_entry(iter_vcpu, struct csched_vcpu,
+                                    active_vcpu_elem);
+                    pin_count = cpus_weight(svc->vcpu->cpu_affinity);
+                    cpus_and(cpus, cpu_online_map, svc->vcpu->cpu_affinity);
+                    if ( pin_count == i )
+                    {
+                        /* search pinned pcpus */
+                        pcpu_id_handle = NR_CPUS;
+                        for ( j = 0; j < pin_count; j++)
+                        {
+                            pcpu_id_handle = 
+                                __cycle_cpu(pcpu_id_handle, &cpus);
+                            pcpu_id_list[j]  = pcpu_id_handle;
+                        }
+                        /* sort credit */
+                        pcpu_credit_summing = atomic_read(&svc->credit);
+                        for ( j = 0; j < pin_count; j++)
+                        {
+                            sortflag = 0;
+                            for ( k = 1; k < pin_count; k++)
+                            {
+                                if ( pcpu_credit_summing >0 )
+                                {
+                                    if ( pcpu_credit[pcpu_id_list[k-1]] <
+                                             pcpu_credit[pcpu_id_list[k]] )
+                                        sortflag = 1;
+                                } else {
+                                    if ( pcpu_credit[pcpu_id_list[k-1]] >
+                                             pcpu_credit[pcpu_id_list[k]] )
+                                        sortflag = 1;
+                                }
+                                if ( sortflag == 1 )
+                                {
+                                    pcpu_id_handle  = pcpu_id_list[k-1];
+                                    pcpu_id_list[k-1] = pcpu_id_list[k];
+                                    pcpu_id_list[k]   = pcpu_id_handle;
+                                }
+                            }
+                            if( sortflag == 0)break;
+                        }
+                        /* add credit to each pcpu_credit */
+                        svc->pcpu_id_credit = pcpu_id_list[0];
+                        credit_add_pcpu_count = 1;
+                        if ( pcpu_credit_summing != 0 )
+                        {
+                            if ( pcpu_credit_summing > 0 )
+                            {
+                                for ( j = 0; j < pin_count; j++)
+                                {
+                                    /* use mul (j+1) instead of div(j+1)*/
+                                    if ( pcpu_credit[pcpu_id_list[j]] * (j+1) <
+                                        (pcpu_credit_summing +
+                                            pcpu_credit[pcpu_id_list[j]]) )
+                                    {
+                                        pcpu_credit_summing +=
+                                            pcpu_credit[pcpu_id_list[j]];
+                                        credit_add_pcpu_count = j+1;
+                                    }
+                                }
+                            } else {
+                                for ( j = 0; j < pin_count; j++)
+                                {
+                                    /* use mul (j+1) instead of div(j+1)*/
+                                    if ( pcpu_credit[pcpu_id_list[j]] * (j+1) >
+                                        (pcpu_credit_summing +
+                                             pcpu_credit[pcpu_id_list[j]]) )
+                                    {
+                                        pcpu_credit_summing +=
+                                            pcpu_credit[pcpu_id_list[j]];
+                                        credit_add_pcpu_count = j+1;
+                                    }
+                                }
+                            }
+                            pcpu_credit_summing =
+                                (pcpu_credit_summing+credit_add_pcpu_count-1)
+                                                 / credit_add_pcpu_count;
+                            for ( j = 0; j < credit_add_pcpu_count; j++)
+                                pcpu_credit[pcpu_id_list[j]] =
+                                    pcpu_credit_summing;
+                        } else {
+                            svc->pcpu_id_credit = 0xffff;
+                        }
+                    }
+                }
+            }
+        }
+    }  /* csched_priv.ncpus loop */
+    
+    for ( i = 0; i < NR_CPUS; i++)
+        csched_priv.pcpu_credit_balance[i] = pcpu_credit[i];
+}
+
 static void
 csched_acct(void)
 {
@@ -832,6 +1175,9 @@ csched_acct(void)
     uint32_t credit_fair;
     uint32_t credit_peak;
     uint32_t credit_cap;
+    uint32_t credit_corr_factor;
+    uint32_t credit_total_corr_factor;
+    uint32_t credit_total_old;
     int credit_balance;
     int credit_xtra;
     int credit;
@@ -863,6 +1209,10 @@ csched_acct(void)
     credit_balance = 0;
     credit_xtra = 0;
     credit_cap = 0U;
+    credit_total_corr_factor = CSCHED_DEFAULT_FACTOR;
+
+    /* consider vcpu-pin effect */
+    csched_consider_vcpupin();
 
     list_for_each_safe( iter_sdom, next_sdom, &csched_priv.active_sdom )
     {
@@ -903,6 +1253,7 @@ csched_acct(void)
 
         credit_fair = ( ( credit_total * sdom->weight) + (weight_total - 1)
                       ) / weight_total;
+        credit_corr_factor = credit_total_corr_factor;
 
         if ( credit_fair < credit_peak )
         {
@@ -912,11 +1263,14 @@ csched_acct(void)
         {
             if ( weight_left != 0U )
             {
+                credit_total_old = credit_total;
                 /* Give other domains a chance at unused credits */
                 credit_total += ( ( ( credit_fair - credit_peak
                                     ) * weight_total
                                   ) + ( weight_left - 1 )
                                 ) / weight_left;
+                credit_total_corr_factor = credit_total_corr_factor 
+                                         * credit_total / credit_total_old;
             }
 
             if ( credit_xtra )
@@ -930,6 +1284,8 @@ csched_acct(void)
                 list_del(&sdom->active_sdom_elem);
                 list_add(&sdom->active_sdom_elem, &csched_priv.active_sdom);
             }
+            credit_corr_factor = credit_corr_factor * credit_peak
+                                                    / credit_fair;
 
             credit_fair = credit_peak;
         }
@@ -945,7 +1301,13 @@ csched_acct(void)
             BUG_ON( sdom != svc->sdom );
 
             /* Increment credit */
-            atomic_add(credit_fair, &svc->credit);
+            if ( svc->factor == CSCHED_DEFAULT_FACTOR )
+                atomic_add(credit_fair, &svc->credit);
+            else
+                /* increment credit for vcpupin case */
+                atomic_add(credit_fair * svc->factor / credit_corr_factor,
+                           &svc->credit);
+
             credit = atomic_read(&svc->credit);
 
             /*
@@ -1007,6 +1369,8 @@ csched_acct(void)
     }
 
     csched_priv.credit_balance = credit_balance;
+
+    csched_consider_vcpupin_credit_balance();
 
     spin_unlock_irqrestore(&csched_priv.lock, flags);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH][RFC] consider vcpu-pin weight on Credit Scheduler TAKE2

[Emmanuel Ackaouy]

I think this patch is too large and intrusive in the common paths.
I understand the problem you are trying to fix. I don't think it is
serious enough to call for such a large change. The accounting
code is already tricky enough, don't you think? If you reduce the
scope of the problem you're addressing, I think we should be
able to get a much smaller, cleaner and robust change in place.

There are many different scenarios when using pinning that
screws with set weights. Have you considered them all?

For example:

VCPU0.0:0-1, VCPU0.1:1-2 weight 256
VCPU1.0-0-2, VCPU1.1:0-2 weight 512

Does your patch deal with cases when there are multiple
domains with multiple VCPUs each and not all sharing the
same cpu affinity mask? I'm not even sure myself what
should happen in some of these situations...

I argue that the general problem isn't important to solve. The
interesting problem is a small subset: When a set of physical
CPUs are set aside for a specific group of domains, setting
weights for those domains should behave as expected. For
example, on an 8way host, you could set aside 2CPUs for
development work and assign different weights to domains
running in that dev group. You would expect the weights to
work normally.

The best way to do this though is not to screw around with
weights and credit when VCPUs are pinned. The cleanest
modification is to run distinct credit schedulers: 1 for dev on
2CPUs, and 1 for the rest.

You could probably achieve this in a much smaller patch which
would include administrative interfaces for creating and destroying
these dynamic CPU partition groups as well assigning domains to
them.

On Jun 27, 2007, at 9:58, Atsushi SAKAI wrote:

> Hi, Keir
>
> This patch intends
> to consider vcpu-pin weight on credit scheduler TAKE2.
> http://lists.xensource.com/archives/html/xen-devel/2007-06/ 
> msg00359.html
>
> The difference from previous one is
> 1) Coding style clean up
> 2) Skip loop for unused vcpu-pin-count.
> 3) Remove if pin_count ==1 in multiple loop.
>    Then pin_count ==1 is another loop.
>
> Signed-off-by: Atsushi SAKAI <sakaia@jp.fujitsu.com>
>
> And one question,
> Does this patch need following tune up for reducing multiple loop?
>
>> From following
>
> -  /* sort weight */
> -  for(j=0;j<pin_count;j++)
> -  {
> -      sortflag = 0;
> -      for(k=1;k<pin_count;k++)
> -      {
> -          if ( pcpu_weight[pcpu_id_list[k-1]] >  
> pcpu_weight[pcpu_id_list[k]]
> )
> -          {
> -              sortflag = 1;
> -              pcpu_id_handle  = pcpu_id_list[k-1];
> -              pcpu_id_list[k-1] = pcpu_id_list[k];
> -              pcpu_id_list[k]   = pcpu_id_handle;
> -          }
> -      }
> -      if( sortflag == 0)break;
> -  }
>
> To following
>
> +     /* sort weight */
> +     for(k=1;k<pin_count;k++)
> +     {
> +          if ( pcpu_weight[pcpu_id_list[k-1]] >  
> pcpu_weight[pcpu_id_list[k]]
> )
> +          {
> +              pcpu_id_handle  = pcpu_id_list[k-1];
> +              pcpu_id_list[k-1] = pcpu_id_list[k];
> +              pcpu_id_list[k]   = pcpu_id_handle;
> +              if (k > 1) k -= 2;
> +           }
> +     }
>
>
> Thanks
> Atsushi SAKAI
>
>
> <vcpupinweight0627.patch>______________________________________________ 
> _
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH][RFC] consider vcpu-pin weight on CreditScheduler TAKE2

[Atsushi SAKAI]

Hi, Emmanuel

 Thank you for commenting my patch.
 I am waiting for this kind of discussion.

My patch is a kind of full-set solution for this vcpu-pin weight issue.
Of couse, your suggested complex configuration considers it.
Since my patch is calculate each pcpu credit.
(It makes a kind of long code((+)365line)).

Anyway your suggested, 
Segmenting pcpu is another useful solution.
Since in typical use, seg-1 for dev seg-2 for rest.
But flexibility is reduced from previous one.
In this case vcpu-pin cannot define over the multiple segmentation.

Which is the best way to solve?

Thanks
Atsushi SAKAI


Emmanuel Ackaouy <ackaouy@gmail.com> wrote:

> I think this patch is too large and intrusive in the common paths.
> I understand the problem you are trying to fix. I don't think it is
> serious enough to call for such a large change. The accounting
> code is already tricky enough, don't you think? If you reduce the
> scope of the problem you're addressing, I think we should be
> able to get a much smaller, cleaner and robust change in place.
> 
> There are many different scenarios when using pinning that
> screws with set weights. Have you considered them all?
> 
> For example:
> 
> VCPU0.0:0-1, VCPU0.1:1-2 weight 256
> VCPU1.0-0-2, VCPU1.1:0-2 weight 512
> 
> Does your patch deal with cases when there are multiple
> domains with multiple VCPUs each and not all sharing the
> same cpu affinity mask? I'm not even sure myself what
> should happen in some of these situations...
> 
> I argue that the general problem isn't important to solve. The
> interesting problem is a small subset: When a set of physical
> CPUs are set aside for a specific group of domains, setting
> weights for those domains should behave as expected. For
> example, on an 8way host, you could set aside 2CPUs for
> development work and assign different weights to domains
> running in that dev group. You would expect the weights to
> work normally.
> 
> The best way to do this though is not to screw around with
> weights and credit when VCPUs are pinned. The cleanest
> modification is to run distinct credit schedulers: 1 for dev on
> 2CPUs, and 1 for the rest.
> 
> You could probably achieve this in a much smaller patch which
> would include administrative interfaces for creating and destroying
> these dynamic CPU partition groups as well assigning domains to
> them.
> 
> On Jun 27, 2007, at 9:58, Atsushi SAKAI wrote:
> 
> > Hi, Keir
> >
> > This patch intends
> > to consider vcpu-pin weight on credit scheduler TAKE2.
> > http://lists.xensource.com/archives/html/xen-devel/2007-06/ 
> > msg00359.html
> >
> > The difference from previous one is
> > 1) Coding style clean up
> > 2) Skip loop for unused vcpu-pin-count.
> > 3) Remove if pin_count ==1 in multiple loop.
> >    Then pin_count ==1 is another loop.
> >
> > Signed-off-by: Atsushi SAKAI <sakaia@jp.fujitsu.com>
> >
> > And one question,
> > Does this patch need following tune up for reducing multiple loop?
> >
> >> From following
> >
> > -  /* sort weight */
> > -  for(j=0;j<pin_count;j++)
> > -  {
> > -      sortflag = 0;
> > -      for(k=1;k<pin_count;k++)
> > -      {
> > -          if ( pcpu_weight[pcpu_id_list[k-1]] >  
> > pcpu_weight[pcpu_id_list[k]]
> > )
> > -          {
> > -              sortflag = 1;
> > -              pcpu_id_handle  = pcpu_id_list[k-1];
> > -              pcpu_id_list[k-1] = pcpu_id_list[k];
> > -              pcpu_id_list[k]   = pcpu_id_handle;
> > -          }
> > -      }
> > -      if( sortflag == 0)break;
> > -  }
> >
> > To following
> >
> > +     /* sort weight */
> > +     for(k=1;k<pin_count;k++)
> > +     {
> > +          if ( pcpu_weight[pcpu_id_list[k-1]] >  
> > pcpu_weight[pcpu_id_list[k]]
> > )
> > +          {
> > +              pcpu_id_handle  = pcpu_id_list[k-1];
> > +              pcpu_id_list[k-1] = pcpu_id_list[k];
> > +              pcpu_id_list[k]   = pcpu_id_handle;
> > +              if (k > 1) k -= 2;
> > +           }
> > +     }
> >
> >
> > Thanks
> > Atsushi SAKAI
> >
> >
> > <vcpupinweight0627.patch>______________________________________________ 
> > _
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH][RFC] consider vcpu-pin weight on CreditScheduler TAKE2

[Emmanuel Ackaouy]

On Jun 27, 2007, at 14:07, Atsushi SAKAI wrote:
>  Thank you for commenting my patch.
>  I am waiting for this kind of discussion.
>
> My patch is a kind of full-set solution for this vcpu-pin weight issue.
> Of couse, your suggested complex configuration considers it.
> Since my patch is calculate each pcpu credit.
> (It makes a kind of long code((+)365line)).
>
> Anyway your suggested,
> Segmenting pcpu is another useful solution.
> Since in typical use, seg-1 for dev seg-2 for rest.
> But flexibility is reduced from previous one.
> In this case vcpu-pin cannot define over the multiple segmentation.
>
> Which is the best way to solve?

If you could solve the generic problem in a simpler way, I would
not be opposed to it. But +365 lines in what is already a fairly
complex accounting code path doesn't seem right to me.

I can't even understand what weights mean when every CPU
has a different pin cpumask. Weights only make sense to me
when VMs compete for resources.

In my opinion, adding the concept of dynamic partitioning (or
segmentation) of the host system would allow a bunch of people
to no longer have to pin their VCPUs. This is desirable.

Re: [PATCH][RFC] consider vcpu-pin weight on CreditScheduler TAKE2

[Keir Fraser]

On 27/6/07 13:21, "Emmanuel Ackaouy" <ackaouy@gmail.com> wrote:

>> Which is the best way to solve?
> 
> If you could solve the generic problem in a simpler way, I would
> not be opposed to it. But +365 lines in what is already a fairly
> complex accounting code path doesn't seem right to me.
> 
> I can't even understand what weights mean when every CPU
> has a different pin cpumask. Weights only make sense to me
> when VMs compete for resources.
> 
> In my opinion, adding the concept of dynamic partitioning (or
> segmentation) of the host system would allow a bunch of people
> to no longer have to pin their VCPUs. This is desirable.

Partitioning is a very sensible simplification in many (most?) cases, but
would need plumbing all the way up through xen-api, which is a pain. I still
suspect that the patch could be simplified even without interface changes. I
don't understand the need to add extra complexity on every accounting
period.

 -- Keir

Re: [PATCH][RFC] consider vcpu-pin weight on CreditScheduler TAKE2

[Emmanuel Ackaouy]

On Jun 27, 2007, at 14:31, Keir Fraser wrote:
> Partitioning is a very sensible simplification in many (most?) cases, 
> but
> would need plumbing all the way up through xen-api, which is a pain. I 
> still
> suspect that the patch could be simplified even without interface 
> changes. I
> don't understand the need to add extra complexity on every accounting
> period.

If you don't want to change the API, the partitioning could just
automagically happen when all distinct cpu affinity masks set
using the existing interface are non-overlapping.

Re: [PATCH][RFC] consider vcpu-pin weight onCreditScheduler TAKE2

[Atsushi SAKAI]

Hi, Keir and Emmanuel

 Thank you for your discussions.
It is very meaningful for me.
I try it again for simplify the code.

Anyway the reason of this kind of complex is come from.

Following 5 conditions should consider for this issue(vcpu-pin-weight).

1)Domain    1-n
2)vcpu      1-n
3)vcpu-pin  1-n
4)Each domain has weight, but it should be treated as vcpu-weight.
since each vcpu pin-map is not same for each domain.
5)Each vcpu has credit, but it is changed each time slice.
(Sometimes accounted, and othertimes are not.)


Thanks
Atsushi SAKAI



Keir Fraser <keir@xensource.com> wrote:

> On 27/6/07 13:21, "Emmanuel Ackaouy" <ackaouy@gmail.com> wrote:
> 
> >> Which is the best way to solve?
> > 
> > If you could solve the generic problem in a simpler way, I would
> > not be opposed to it. But +365 lines in what is already a fairly
> > complex accounting code path doesn't seem right to me.
> > 
> > I can't even understand what weights mean when every CPU
> > has a different pin cpumask. Weights only make sense to me
> > when VMs compete for resources.
> > 
> > In my opinion, adding the concept of dynamic partitioning (or
> > segmentation) of the host system would allow a bunch of people
> > to no longer have to pin their VCPUs. This is desirable.
> 
> Partitioning is a very sensible simplification in many (most?) cases, but
> would need plumbing all the way up through xen-api, which is a pain. I still
> suspect that the patch could be simplified even without interface changes. I
> don't understand the need to add extra complexity on every accounting
> period.
> 
>  -- Keir