Hi Roman,
On Thu, May 1, 2014 at 7:30 AM, Roman Bogorodskiy <no...@freebsd.org> wrote:
> Neel Natu wrote:
>
>> Hi Roman,
>>
>> On Sun, Apr 27, 2014 at 3:45 AM, Roman Bogorodskiy <no...@freebsd.org> wrote:
>> > 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?
>> >
>>
>> I think we can reuse the "-p" option unless anybody objects vociferously.
>>
>> > - Is the syntax OK (currently: 'vcpu:pcpu', later
>> > 'vcpu:pcpuN-pcpuM,pcpuX")?
>>
>> Yup, I think that works fine.
>>
>> The patch looks good in general but I have a few comments:
>>
>> - Scope of 'vcpupmap[]' should be restricted to 'static'.
>>
>> - usage() and man page need to be updated.
>>
>> - pincpu_parse():
>> The option parsing can be made much easier by using:
>>
>> if (sscanf(str, "%d:%d", &vcpu, &pcpu) == 2) {
>> /* success */
>> } else {
>> return (-1);
>> }
>>
>> If the same vcpu is specified multiple times then we should
>> malloc(sizeof(cpuset_t)) only the first time:
>>
>> if (vcpumap[vcpu] != NULL)
>> mask = vcpumap[vcpu];
>> else
>> mask = malloc(sizeof(cpuset_t));
>>
>> We need to range-check 'vcpu' before using it as an index into the
>> 'vcpumap[]' array.
>>
>> best
>> Neel
>
> Attached an updated patch.
A slightly modified version was submitted:
http://svnweb.freebsd.org/base?view=revision&revision=265376
>
> 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?
>
Hmm, not sure really … but you can get started by copying it into
bhyve and if there is a better place to put it we can do that too.
> 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?
Thanks. This is fixed now:
http://svnweb.freebsd.org/base?view=revision&revision=265366
best
Neel
>
> Roman Bogorodskiy
_______________________________________________
freebsd-virtualization@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to
"freebsd-virtualization-unsubscr...@freebsd.org"
