From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2] sched: credit2: respect per-vcpu hard affinity Date: Fri, 13 Mar 2015 17:11:17 +0000 Message-ID: <1426266674.32500.25.camel@citrix.com> References: <1423453550-3526-1-git-send-email-jtweaver@hawaii.edu> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7865376559433283589==" Return-path: In-Reply-To: <1423453550-3526-1-git-send-email-jtweaver@hawaii.edu> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "jtweaver@hawaii.edu" Cc: "xen-devel@lists.xen.org" , George Dunlap , "henric@hawaii.edu" List-Id: xen-devel@lists.xenproject.org --===============7865376559433283589== Content-Language: en-US Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-P0Jd2TNBaJJp9Xgb1YrA" --=-P0Jd2TNBaJJp9Xgb1YrA Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hey, I came across this patch again for other reasons, and I realized I've a few more (minor) comments: On Sun, 2015-02-08 at 17:45 -1000, Justin T. Weaver wrote: > From: "Justin T. Weaver" > diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c > index cf53770..de8fb5a 100644 > --- a/xen/common/sched_credit2.c > +++ b/xen/common/sched_credit2.c > @@ -2127,16 +2212,25 @@ csched2_init(struct scheduler *ops) > =20 > prv->load_window_shift =3D opt_load_window_shift; > =20 > + cpumask =3D xzalloc_bytes(nr_cpu_ids * sizeof(cpumask_t *)); > + if ( cpumask =3D=3D NULL ) > + return -ENOMEM; > + This can probably be xzalloc_array(), or even xmalloc_array(), if we don't care zeroing all elements (see below about why I actually don't think we need). > return 0; > } > =20 > static void > csched2_deinit(const struct scheduler *ops) > { > + int i; > struct csched2_private *prv; > =20 > prv =3D CSCHED2_PRIV(ops); > xfree(prv); > + > + for ( i =3D 0; i < nr_cpu_ids; i++ ) > + free_cpumask_var(cpumask[i]); > + xfree(cpumask); > Do we need the loop? I mean, all the pcpus go through csched2_alloc_pdata(), when being activated under this scheduler, which allocates their scratch masks. They then go through csched2_free_pdata(), when deactivated from this scheduler, which frees their scratch masks. I would then expect that, when we get to here, all the elemtns of the scratch mask array that were allocated, have also been freed, and hence only freeing the array is necessary. Am I missing something? Regards, Dario --=-P0Jd2TNBaJJp9Xgb1YrA 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 v2 iEYEABECAAYFAlUDGjIACgkQk4XaBE3IOsQqgACgiG1sHJXtLrdeTyld4ZQBA+l9 ZsIAn3EPVWBNtNEKCferPoZW8RDyPT5P =iWJc -----END PGP SIGNATURE----- --=-P0Jd2TNBaJJp9Xgb1YrA-- --===============7865376559433283589== 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 --===============7865376559433283589==--