Date: Thu, 1 May 2014 18:30:06 +0400 From: Roman Bogorodskiy <novel@FreeBSD.org> To: Neel Natu <neelnatu@gmail.com> Cc: "freebsd-virtualization@freebsd.org" <freebsd-virtualization@freebsd.org> Subject: Re: [PATCH] Flexible vcpu pinning configuration Message-ID: <20140501143005.GA91029@kloomba> In-Reply-To: <CAFgRE9FUHEXbnkTSVmpyOP7EMzQh7QEFeNJikVWw6xHRWbvQSw@mail.gmail.com> References: <20140427104511.GA7804@kloomba> <CAFgRE9FUHEXbnkTSVmpyOP7EMzQh7QEFeNJikVWw6xHRWbvQSw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--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 <novel@freebsd.org> 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 <gdb port>] [-s <pci>] [-S <pc= i>]\n" - " %*s [-c vcpus] [-p pincpu] [-m mem] [-l <lpc>] <vm>\n" + " %*s [-c vcpus] [-p vcpu:hostcpu] [-m mem] [-l <lpc>] <vm>\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: <n:m> 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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140501143005.GA91029>