From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chong Li Subject: Re: [PATCH v4 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler Date: Sun, 9 Aug 2015 10:45:59 -0500 Message-ID: References: <1436590356-3706-1-git-send-email-chong.li@wustl.edu> <1436590356-3706-2-git-send-email-chong.li@wustl.edu> <55A394D0020000780008FFC3@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55A394D0020000780008FFC3@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Chong Li , Sisu Xi , George Dunlap , "dario.faggioli" , xen-devel , Meng Xu , Dagaen Golomb List-Id: xen-devel@lists.xenproject.org On Mon, Jul 13, 2015 at 3:37 AM, Jan Beulich wrote: >>>> On 11.07.15 at 06:52, wrote: >> @@ -1162,8 +1176,82 @@ rt_dom_cntl( >> } >> spin_unlock_irqrestore(&prv->lock, flags); >> break; >> + case XEN_DOMCTL_SCHEDOP_getvcpuinfo: >> + spin_lock_irqsave(&prv->lock, flags); >> + for ( index = 0; index < op->u.v.nr_vcpus; index++ ) >> + { >> + if ( copy_from_guest_offset(&local_sched, >> + op->u.v.vcpus, index, 1) ) >> + { >> + rc = -EFAULT; >> + break; >> + } >> + if ( local_sched.vcpuid >= d->max_vcpus || >> + d->vcpu[local_sched.vcpuid] == NULL ) >> + { >> + rc = -EINVAL; >> + break; >> + } >> + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]); >> + >> + local_sched.s.rtds.budget = svc->budget / MICROSECS(1); >> + local_sched.s.rtds.period = svc->period / MICROSECS(1); >> + >> + if ( __copy_to_guest_offset(op->u.v.vcpus, index, >> + &local_sched, 1) ) >> + { >> + rc = -EFAULT; >> + break; >> + } >> + if( hypercall_preempt_check() ) >> + { >> + rc = -ERESTART; >> + break; >> + } > > I still don't see how this is supposed to work. I return -ERESTART here, and the upper layer function (do_domctl) will handle this error code by calling hypercall_create_continuation. >> +} xen_domctl_schedparam_vcpu_t; >> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_schedparam_vcpu_t); >> + >> /* Set or get info? */ >> #define XEN_DOMCTL_SCHEDOP_putinfo 0 >> #define XEN_DOMCTL_SCHEDOP_getinfo 1 >> +#define XEN_DOMCTL_SCHEDOP_putvcpuinfo 2 >> +#define XEN_DOMCTL_SCHEDOP_getvcpuinfo 3 >> struct xen_domctl_scheduler_op { >> uint32_t sched_id; /* XEN_SCHEDULER_* */ >> uint32_t cmd; /* XEN_DOMCTL_SCHEDOP_* */ >> union { >> - struct xen_domctl_sched_sedf { >> - uint64_aligned_t period; >> - uint64_aligned_t slice; >> - uint64_aligned_t latency; >> - uint32_t extratime; >> - uint32_t weight; >> - } sedf; >> - struct xen_domctl_sched_credit { >> - uint16_t weight; >> - uint16_t cap; >> - } credit; >> - struct xen_domctl_sched_credit2 { >> - uint16_t weight; >> - } credit2; >> - struct xen_domctl_sched_rtds { >> - uint32_t period; >> - uint32_t budget; >> - } rtds; >> + xen_domctl_sched_sedf_t sedf; >> + xen_domctl_sched_credit_t credit; >> + xen_domctl_sched_credit2_t credit2; >> + xen_domctl_sched_rtds_t rtds; >> + struct { >> + XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus; >> + uint16_t nr_vcpus; >> + } v; > > And there's still no explicit padding here at all (nor am I convinced > that uint16_t is really a good choice for nr_vcpus - uint32_t would > seem more natural without causing any problems or structure > growth). I think the size of union u is equal to the size of xen_domctl_sched_sedf_t, which is 64*4 bits (if "vcpus" in struct v is just a pointer). The "nr_vcpus" is indeed better to be uint32_t. I'll change it in the next version. Chong > > Jan -- Chong Li Department of Computer Science and Engineering Washington University in St.louis