From owner-freebsd-virtualization@FreeBSD.ORG Thu May 1 14:31:02 2014 Return-Path: Delivered-To: freebsd-virtualization@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 6BA4267E for ; Thu, 1 May 2014 14:31:02 +0000 (UTC) Received: from mail-la0-x22f.google.com (mail-la0-x22f.google.com [IPv6:2a00:1450:4010:c03::22f]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D4B461203 for ; Thu, 1 May 2014 14:31:01 +0000 (UTC) Received: by mail-la0-f47.google.com with SMTP id pn19so2222929lab.34 for ; Thu, 01 May 2014 07:30:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=ffIQzEbPFZY5Pxw+UH6fGp5Q4a4W82siJGKwgDiP5/U=; b=naA+ZcUd0YqlZ/eInUrniOF8uDuyG5/6hQbLuSLqu+DTyFzqx8R0RoaMDpYLyiklkH tbDU/a/a8yVTOUyu2HXGqYg5CR2dbu4CTAbRS1HVbf1gG/NSBZMcsg/JdeC7Tp2PFure vzP6fiiRhUa/R/gtMcy3wpep2LLBCXKpBeSvMHcmSfM658Dz3jH+5SemFHBHyZA5oclc /8gag3G1OcGQ0lvVDRrHngfVVJDeQNzorqDJYrOrjdrcaaqZkf7maXSEvGXEoGHgiM/C uTTrMRljm0utZXQoSp9XoohVoKz+0hGAEmVITl/TtoH3epXVTF5WT4jPLxHsYTOo/CiM mVvQ== X-Received: by 10.152.1.8 with SMTP id 8mr7790666lai.1.1398954659735; Thu, 01 May 2014 07:30:59 -0700 (PDT) Received: from kloomba ([95.104.133.236]) by mx.google.com with ESMTPSA id jp6sm30073349lbc.15.2014.05.01.07.30.58 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 01 May 2014 07:30:58 -0700 (PDT) Sender: Roman Bogorodskiy Date: Thu, 1 May 2014 18:30:06 +0400 From: Roman Bogorodskiy To: Neel Natu Subject: Re: [PATCH] Flexible vcpu pinning configuration Message-ID: <20140501143005.GA91029@kloomba> References: <20140427104511.GA7804@kloomba> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="tjCHc7DPkfUGtrlw" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "freebsd-virtualization@freebsd.org" X-BeenThere: freebsd-virtualization@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Discussion of various virtualization techniques FreeBSD supports." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 May 2014 14:31:02 -0000 --tjCHc7DPkfUGtrlw Content-Type: multipart/mixed; boundary="YiEDa0DAkWCtVeE4" Content-Disposition: inline --YiEDa0DAkWCtVeE4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Neel Natu wrote: > Hi Roman, >=20 > On Sun, Apr 27, 2014 at 3:45 AM, Roman Bogorodskiy wr= ote: > > I've created an initial version of the patch which allows more flexible > > vcpu pinning configuration. > > > > Current schema is: > > > > bhyve -p N > > > > pins vcpu i to hostcpu N + i. > > > > The propsed extension is: > > > > bhyve -p N:M .... -p 0:1 -p 3:5 > > > > which pins vcpu N to host pcpu M. Option needs to be specified > > individually for each vcpu. > > > > So it works like that for me: > > > > sudo /usr/sbin/bhyve -p 0:0 -p 1:3 -c 2 ... > > > > # sudo cpuset -g -t 100262 > > tid 100262 mask: 0 > > # sudo cpuset -g -t 100264 > > tid 100264 mask: 3 > > > > PS I used cpumat_t* array to store these values instead of int, because > > if the idea is OK, I'll extend it to support ranges like e.g. cpuset(1) > > supports, e.g.: "1:2-5". > > > > The questions are: > > > > - Is it OK to chance '-p' arg syntax or it's better to introduce a new > > one? > > >=20 > I think we can reuse the "-p" option unless anybody objects vociferously. >=20 > > - Is the syntax OK (currently: 'vcpu:pcpu', later > > 'vcpu:pcpuN-pcpuM,pcpuX")? >=20 > Yup, I think that works fine. >=20 > The patch looks good in general but I have a few comments: >=20 > - Scope of 'vcpupmap[]' should be restricted to 'static'. >=20 > - usage() and man page need to be updated. >=20 > - pincpu_parse(): > The option parsing can be made much easier by using: >=20 > if (sscanf(str, "%d:%d", &vcpu, &pcpu) =3D=3D 2) { > /* success */ > } else { > return (-1); > } >=20 > If the same vcpu is specified multiple times then we should > malloc(sizeof(cpuset_t)) only the first time: >=20 > if (vcpumap[vcpu] !=3D NULL) > mask =3D vcpumap[vcpu]; > else > mask =3D malloc(sizeof(cpuset_t)); >=20 > We need to range-check 'vcpu' before using it as an index into the > 'vcpumap[]' array. >=20 > best > Neel Attached an updated patch. I'm still inclined to use something like parselist() from usr.bin/cpuset/cpuset.c, but I don't want to copy/paste and I don't know where it'd make sense to move it so it was usable outside of cpuset? PS While reading bhyverun.c, I think I spotted a typo: in fbsdrun_deletecpu= () error message says fprintf(stderr, "addcpu: .... Should it be "deletecpu:" = instead? Roman Bogorodskiy --YiEDa0DAkWCtVeE4 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="bhyve_vcpupin2.diff" Content-Transfer-Encoding: quoted-printable Index: bhyve.8 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- bhyve.8 (revision 264921) +++ bhyve.8 (working copy) @@ -78,12 +78,14 @@ allow a remote kernel kgdb to be relayed to the guest kernel gdb stub via a local IPv4 address and this port. This option will be deprecated in a future version. -.It Fl p Ar pinnedcpu +.It Fl p Ar n:m Force guest virtual CPUs to be pinned to host CPUs. Virtual CPU .Em n is pinned to host CPU -.Em pinnedcpu+n . +.Em m . +This option could be specified multiple times to set pinning for each +Virtual CPU and to pin Virtual CPU to more than one host CPU. .It Fl P Force the guest virtual CPU to exit when a PAUSE instruction is detected. .It Fl W Index: bhyverun.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- bhyverun.c (revision 264921) +++ bhyverun.c (working copy) @@ -83,7 +83,6 @@ =20 int guest_ncpus; =20 -static int pincpu =3D -1; static int guest_vmexit_on_hlt, guest_vmexit_on_pause, disable_x2apic; static int virtio_msix =3D 1; =20 @@ -119,6 +118,8 @@ int mt_vcpu;=09 } mt_vmm_info[VM_MAXCPU]; =20 +static cpuset_t *vcpumap[VM_MAXCPU] =3D { NULL }; + static void usage(int code) { @@ -125,12 +126,12 @@ =20 fprintf(stderr, "Usage: %s [-aehwAHIPW] [-g ] [-s ] [-S ]\n" - " %*s [-c vcpus] [-p pincpu] [-m mem] [-l ] \n" + " %*s [-c vcpus] [-p vcpu:hostcpu] [-m mem] [-l ] \n" " -a: local apic is in XAPIC mode (default is X2APIC)\n" " -A: create an ACPI table\n" " -g: gdb port\n" " -c: # cpus (default 1)\n" - " -p: pin vcpu 'n' to host cpu 'pincpu + n'\n" + " -p: pin vcpu 'n' to host cpu 'm'\n" " -H: vmexit from the guest on hlt\n" " -P: vmexit from the guest on pause\n" " -W: force virtio to use single-vector MSI\n" @@ -146,6 +147,53 @@ exit(code); } =20 +static int +pincpu_parse(const char *cpupin) +{ + char *string; + int vcpu =3D -1, pcpu =3D -1, ret =3D -1; + + if ((string =3D strdup(cpupin)) =3D=3D NULL) { + fprintf(stderr, "strdup failed: %d\n", errno); + return -1; + } + + if (sscanf(string, "%d:%d", &vcpu, &pcpu) !=3D 2) { + fprintf(stderr, "invalid format: %s\n", string); + goto cleanup; + } + + if (vcpu < 0 || vcpu > VM_MAXCPU - 1) { + fprintf(stderr, "invalid vcpu value '%d', " + "should be from 0 to %d\n", + vcpu, VM_MAXCPU - 1); + goto cleanup; + } + + if (pcpu < 0 || pcpu > CPU_SETSIZE) { + fprintf(stderr, "invalid pcpu value '%d', " + "should be from 0 to %d\n", + pcpu, CPU_SETSIZE); + goto cleanup; + } + + if (vcpumap[vcpu] =3D=3D NULL) { + if ((vcpumap[vcpu] =3D malloc(sizeof(cpuset_t))) =3D=3D NULL) { + fprintf(stderr, "malloc failed: %d\n", errno); + goto cleanup; + } + CPU_ZERO(vcpumap[vcpu]); + } + CPU_SET(pcpu, vcpumap[vcpu]); + + ret =3D 0; + +cleanup: + free(string); + + return ret; +} + void * paddr_guest2host(struct vmctx *ctx, uintptr_t gaddr, size_t len) { @@ -479,15 +527,13 @@ static void vm_loop(struct vmctx *ctx, int vcpu, uint64_t rip) { - cpuset_t mask; int error, rc, prevcpu; enum vm_exitcode exitcode; =20 - if (pincpu >=3D 0) { - CPU_ZERO(&mask); - CPU_SET(pincpu + vcpu, &mask); + if (vcpumap[vcpu] !=3D NULL) { error =3D pthread_setaffinity_np(pthread_self(), - sizeof(mask), &mask); + sizeof(cpuset_t), + vcpumap[vcpu]); assert(error =3D=3D 0); } =20 @@ -622,7 +668,10 @@ bvmcons =3D 1; break; case 'p': - pincpu =3D atoi(optarg); + if (pincpu_parse(optarg) < 0) { + errx(EX_USAGE, "invalid cpu pin " + "configuration '%s'", optarg); + } break; case 'c': guest_ncpus =3D atoi(optarg); --YiEDa0DAkWCtVeE4-- --tjCHc7DPkfUGtrlw Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (FreeBSD) iQEcBAEBAgAGBQJTYlptAAoJEMltX/4IwiJqEkoH/ixfHJaNREQrMECNKv+B7R1i YJSS3TesSLCiCzKUA887+i+DMgXfbvKu/xMciS6Vllj6ejIONMlrEQgRF9tUO23J p1GRcQRzNXsmhSLlBhZL/HA2LoZN8UStj2vScKYMtAYotFyoXwc024mVy8t6q4wU vllCl21Pa3TAjVYPpzlrKQAadhZXRTIr2U0BDmUWFeMl553U/ZOlT/d/TBDhSeYG 4VVS4eTK7oDkK8/XRPq4bQvswx5KmFGp1Sy0uoVSbUCVYQ6oNg2TEWJtpa+spPl8 SCgiK15s2lHaB/UeAQkSre/sJ/I0Gy5lZ5R/Ncn12tfu0OrCO4YSvZuGPKwKFAA= =Ix2V -----END PGP SIGNATURE----- --tjCHc7DPkfUGtrlw--