From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH for-4.6 2/3] xl: fix vNUMA vcpus parsing Date: Thu, 13 Aug 2015 11:32:35 +0200 Message-ID: <1439458355.24583.69.camel@citrix.com> References: <1439408161-15015-1-git-send-email-wei.liu2@citrix.com> <1439408161-15015-3-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5666705307307307278==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZPosQ-0004Rz-PK for xen-devel@lists.xenproject.org; Thu, 13 Aug 2015 09:32:46 +0000 In-Reply-To: <1439408161-15015-3-git-send-email-wei.liu2@citrix.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: Wei Liu Cc: Xen-devel , Boris Ostrovsky , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org --===============5666705307307307278== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-IB2X9ltOGN7xl7uJi/XC" --=-IB2X9ltOGN7xl7uJi/XC Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2015-08-12 at 20:36 +0100, Wei Liu wrote: > Originally, if user didn't specify maxvcpus=3D in xl config file, the > maximum size of vcpu bitmap was always equal to maximum number of pcpus. > This might not be what user wants. >=20 So, to understand, the issue here is that the bitmaps may be too bit, and hence we're wasting memory? Or (since you're mentioning valgrind) are (without this patch) also leaking stuff in some way? I'm confused by he "not be what user wants" part, as, IMO, the actual user --i.e., the person writing the config file and then issuing the `xl ceate' command-- wouldn't notice any difference, would it? Of course, that does not mean that we should waste memory... as I said, I'm asking in order to understand what this patch is actually trying to fix... About the code... > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 078acd1..0fcef98 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > Would it make sense to skip this (the first) step if the user actually did specify "maxvcpus=3D"? Or, if not, at least... ... ... > + /* Pass one, get the total number of vcpus */ > + PARSE_VNUMA_SPEC({ > + if (!strcmp("vcpus", option)) { > + split_string_into_string_list(value, ",", &cpu_spec_list= ); > + len =3D libxl_string_list_length(&cpu_spec_list); > =20 > - vnode_spec =3D xlu_cfg_get_listitem2(vnuma, i); > - assert(vnode_spec); > + for (j =3D 0; j < len; j++) { > + parse_range(cpu_spec_list[j], &s, &e); > + for (; s <=3D e; s++) > + max_vcpus++; > + } > + libxl_string_list_dispose(&cpu_spec_list); > + } > + }); > =20 ... ... ... Move the check for that here, i.e., avoiding doing the second pass if there is a mismatch? > + for (i =3D 0; i < num_vnuma; i++) { > + libxl_bitmap_init(&vcpu_parsed[i]); > + if (libxl_cpu_bitmap_alloc(ctx, &vcpu_parsed[i], max_vcpus)) { > + fprintf(stderr, "libxl_node_bitmap_alloc failed.\n"); > exit(1); > } > + } > + /* Pass two, fill in structs */ > + PARSE_VNUMA_SPEC({ > + if (!strcmp("pnode", option)) { > + val =3D parse_ulong(value); > + if (val >=3D nr_nodes) { > + fprintf(stderr, > + "xl: invalid pnode number: %lu\n", val); > /* User has specified maxvcpus=3D */ > if (b_info->max_vcpus !=3D 0 && b_info->max_vcpus !=3D max_vcpus) { Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-IB2X9ltOGN7xl7uJi/XC 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 iEYEABECAAYFAlXMZDYACgkQk4XaBE3IOsQXhACeMuBWqIUcTZcmxbSKZbKjoa83 6VcAn1t1SbIGiF+EDfXyf+x68szWSLt0 =hG/n -----END PGP SIGNATURE----- --=-IB2X9ltOGN7xl7uJi/XC-- --===============5666705307307307278== 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 --===============5666705307307307278==--