From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH v5 for Xen 4.7 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler Date: Tue, 9 Feb 2016 10:19:59 +0000 Message-ID: <20160209101959.GR23178@citrix.com> References: <1454626244-5511-1-git-send-email-lichong659@gmail.com> <1454626244-5511-4-git-send-email-lichong659@gmail.com> <20160205144439.GA23178@citrix.com> <20160208110717.GM23178@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Chong Li Cc: Chong Li , Wei Liu , Sisu Xi , George Dunlap , "dario.faggioli" , Ian Jackson , xen-devel , Ian Campbell , Meng Xu , Dagaen Golomb List-Id: xen-devel@lists.xenproject.org On Mon, Feb 08, 2016 at 04:59:46PM -0600, Chong Li wrote: > On Mon, Feb 8, 2016 at 5:07 AM, Wei Liu wrote: > > > > > > [...] > > > >> + num_vcpus = max_vcpuid + 1; > > > >> + GCNEW_ARRAY(vcpus, num_vcpus); > > > >> + if (sched_rtds_validate_params(gc, scinfo->vcpus[0].period, > > > >> + scinfo->vcpus[0].budget, > > > > > > > > This doesn't make sense. You take this path because scinfo->num_vcpus is > > > > 0 but now you're dereferencing scinfo->vcpus[0]. Do I miss anything? > > > For commands like " xl sched-rtds -d vm1 -v all -p 1000 -b 1000" > > > (which sets all vcpus with > > > the same scheduling parameters), we pass the budget and period via > > > scinfo->vcpus[0]. > > > > > > I'll add more explanation here. > > > > No, adding more explanation won't help. > > > > Let me explain a bit. Libxl is the library that can be used by multiple > > applications. Xl is just one of the applications. The other application > > that I know of is libvirt. > > > > So, the incarnation of a particular xl command is of no concern how we > > define the semantics of a libxl API. That is, you can come up with an > > unambiguous API but still support the same xl command. > > > > Currently the semantics of this (new?) libxl API seems to be broken, > > because you're (ab)using num_vcpus to represent a special case. In > > effect you can't really whether the array is empty. When num_vcpus is 0, > > you shouldn't dereference vcpus array, at all, because the semantics of > > num_vcpus == 0 is that the array is empty. > > > > Wei. > > I see. I'll think about re-designing the data structure of > libxl_vcpu_sched_params. > Or you can come up with a new API (function) that sets parameter for all vcpus at once? Just a random thought. You can post your proposed data structure and / or the API here. We can discuss this a bit before you actually start writing code, so that you avoid wasting effort. Wei. > Chong > > > > > > > > > >> + &vcpus[0].s.rtds.period, > > > >> + &vcpus[0].s.rtds.budget)) > > > > > > > > > > -- > > > Chong Li > > > Department of Computer Science and Engineering > > > Washington University in St.louis > > > > > -- > Chong Li > Department of Computer Science and Engineering > Washington University in St.louis