Re: [Xen-devel] Re: [Xen-changelog] New weighted fair-share CPU scheduler w/ automatic SMP load balancing

[Ewan Mellor <ewan@xxxxxxxxxxxxx>]

On Fri, May 26, 2006 at 10:51:55AM -0500, Anthony Liguori wrote:

> Just some random feedback.
> 
> Xen patchbot-unstable wrote:
> > 
> >+static PyObject *pyxc_csched_domain_set(XcObject *self,
> >+                                        PyObject *args,
> >+                                        PyObject *kwds)
> >+{
> >+    uint32_t domid;
> >+    uint16_t weight;
> >+    uint16_t cap;
> >+    static char *kwd_list[] = { "dom", "weight", "cap", NULL };
> >+    static char kwd_type[] = "I|HH";
> >+    struct csched_domain sdom;
> >+    
> >+    weight = 0;
> >+    cap = (uint16_t)~0U;
> >+    if( !PyArg_ParseTupleAndKeywords(args, kwds, kwd_type, kwd_list, 
> >+                                     &domid, &weight, &cap) )
> >+        return NULL;
> >+
> >+    sdom.weight = weight;
> >+    sdom.cap = cap;
> >+
> >+    if ( xc_csched_domain_set(self->xc_handle, domid, &sdom) != 0 )
> >+        return PyErr_SetFromErrno(xc_error);
> >+
> >+    Py_INCREF(zero);
> >+    return zero;
> >+}
> >  
> 
> It's always seemed odd that we return zero here instead of Py_RETURN_NONE.

Emmanuel will simply have followed the existing practice.  I agree that
there's no sense in what's there at the moment -- feel free to patch all of
these.

> > 
> >+    def domain_csched_get(self, domid):
> >+        """Get credit scheduler parameters for a domain.
> >+        """
> >+        dominfo = self.domain_lookup_by_name_or_id_nr(domid)
> >+        if not dominfo:
> >+            raise XendInvalidDomain(str(domid))
> >+        try:
> >+            return xc.csched_domain_get(dominfo.getDomid())
> >+        except Exception, ex:
> >+            raise XendError(str(ex))
> >+    
> >+    def domain_csched_set(self, domid, weight, cap):
> >+        """Set credit scheduler parameters for a domain.
> >+        """
> >+        dominfo = self.domain_lookup_by_name_or_id_nr(domid)
> >+        if not dominfo:
> >+            raise XendInvalidDomain(str(domid))
> >+        try:
> >+            return xc.csched_domain_set(dominfo.getDomid(), weight, cap)
> >+        except Exception, ex:
> >+            raise XendError(str(ex))
> >+
> >  
> 
> Please don't catch Exception.  The XML-RPC now properly propagates all 
> exceptions so there's no need to rewrap things in XendError.  Just let 
> the normal exception propagate.

Again, feel free to patch these in the existing code as well as
Emmanuel's new code.

> >diff -r b6937b931419 -r e539abd27a0f tools/python/xen/xm/main.py
> >--- a/tools/python/xen/xm/main.py    Fri May 26 09:44:29 2006 +0100
> >+++ b/tools/python/xen/xm/main.py    Fri May 26 11:14:36 2006 +0100
> >@@ -99,6 +99,7 @@ sched_sedf_help = "sched-sedf [DOM] [OPT
> >                                     specifies another way of setting a 
> >                                     domain's\n\
> >                                     cpu period/slice."
> > 
> >+csched_help = "csched                           Set or get credit 
> >scheduler parameters"
> > block_attach_help = """block-attach <DomId> <BackDev> <FrontDev> <Mode>
> >                 [BackDomId]         Create a new virtual block device"""
> > block_detach_help = """block-detach  <DomId> <DevId>    Destroy a 
> > domain's virtual block device,
> >@@ -174,6 +175,7 @@ host_commands = [
> >     ]
> > 
> > scheduler_commands = [
> >+    "csched",
> >     "sched-bvt",
> >     "sched-bvt-ctxallow",
> >     "sched-sedf",
> >@@ -735,6 +737,48 @@ def xm_sched_sedf(args):
> >         else:
> >             print_sedf(sedf_info)
> 
> Seem to be breaking naming convention here.  sched-csched may seem 
> redundant but that's what you get for choosing a non-descriptive name 
> for the scheduler in the first place ;-)

sched-credit seems appropriate.

Ewan.

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