From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it Date: Mon, 23 Apr 2012 16:22:42 +0200 Message-ID: <1335190962.3122.10.camel@Abyss> References: <20120420150012.GB3720@bloms.de> <1334934791.28331.101.camel@zakaz.uk.xensource.com> <20120423094623.GA13565@bloms.de> <1335182682.4347.15.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2488772583394686957==" Return-path: In-Reply-To: <1335182682.4347.15.camel@zakaz.uk.xensource.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: Ian Campbell Cc: George Dunlap , Dieter Bloms , Dieter Bloms , xen-devel List-Id: xen-devel@lists.xenproject.org --===============2488772583394686957== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-O/1aVUApPgk2DxkScTqr" --=-O/1aVUApPgk2DxkScTqr Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2012-04-23 at 13:04 +0100, Ian Campbell wrote:=20 > > I've made a little patch to set the cpu_weight for credit, credit2 and > > sedf scheduler from the config file. >=20 > Awesome, thank you! >=20 Agreed, nice work and thank you for doing it! > > Btw.: for the sedf scheduler there seems to be some type mismatch. =20 > > The functions xc_sedf_domain_set and xc_sedf_domain_get expect the type > > 'uint16_t' for variables 'extratime' and 'weight' while the structure > > 'xen_domctl_sched_sedf' defines the type uint32_t for them. > > I think they should be the same, or not ? >=20 > I'm not clear enough on the scheduling stuff to be able to say one way > or another. I'd be inclined to suggest that if this needs to change for > some reason then we should leave it for post 4.2. >=20 In Xen's SEDF code weight is a 'short', while extratime is nothing more than a boolean flag (it can store only the EXTRA_AWARE flag). So I think it would be worth looking at this, but I agree we ca defer that for now, especially considering the whole SEDF interface might need some restructuring! :-/ > > xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus); > > libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, > > &info->cpumap); > > xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + > > LIBXL_MAXMEM_CONSTANT); > > + > > + sched =3D libxl_get_scheduler (ctx); > > + =20 > > + switch (sched) { > > + case LIBXL_SCHEDULER_SEDF: > > + xc_sedf_domain_get (ctx->xch, domid, &(sched_op.u.sedf.period), > > &(sched_op.u.sedf.slice), &(sched_op.u.sedf.latency), (uint16_t *) > > &(sched_op.u.sedf.extratime), (uint16_t *) &(sched_op.u.sedf.weight)); > > + sched_op.u.sedf.weight =3D info->weight; > > + xc_sedf_domain_set (ctx->xch, domid, sched_op.u.sedf.period, > > sched_op.u.sedf.slice, sched_op.u.sedf.latency, (uint16_t) > > sched_op.u.sedf.extratime, (uint16_t) sched_op.u.sedf.weight); >=20 > Wow, that's a pretty sucky interface at the libxc level! Anyway no need > for you to fix it here but please do wrap your lines to <80 columns for > readability. >=20 Indeed! I really think we have to do something about this, but that definitely will happen post-release. :-) > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > > index 5cf9708..f185d4c 100644 > > --- a/tools/libxl/libxl_types.idl > > +++ b/tools/libxl/libxl_types.idl > > @@ -232,6 +232,8 @@ MemKB =3D UInt(64, init_val =3D "LIBXL_MEMKB_DEFAUL= T") > > libxl_domain_build_info =3D Struct("domain_build_info",[ > > ("max_vcpus", integer), > > ("cur_vcpus", integer), > > + ("weight", integer), > > + ("cap", integer), >=20 > Are these values common parameters to all schedulers? Do they always > have the same meaning/units/etc? >=20 Maybe weight is, but still, I think having some mechanism for specifying the full set of parameter of a specific scheduler is to be preferred... > I wonder if perhaps including each of libxl_sched_*_params in build_info > might be a preferable interface? I would probably cleanup the above code > in build_pre too since you could just call the appropriate > libxl_sched_*_params_set. >=20 ... Exactly, that's much better looking to me. > Ideally libxl_sched_*_params would be in a union in > libxl_domain_build_info but that would require that xl could easily > determine which scheduler was going to be used for the domain, having a > non-union here would keep things somewhat simpler from that PoV. >=20 Yes, again, I agree that a union will be even better, but maybe not so much a big deal for now (we can turn it into an union later, right? Or you think there will be some API implications?) Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-O/1aVUApPgk2DxkScTqr Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEABECAAYFAk+VZbIACgkQk4XaBE3IOsQr6QCfRH22XlfzvX0wMWINceq9gmXP RpgAoIaWN32GxCjiMtvIVCRq8gGWxlFg =U+kJ -----END PGP SIGNATURE----- --=-O/1aVUApPgk2DxkScTqr-- --===============2488772583394686957== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============2488772583394686957==--