[libvirt] [PATCH] RFC: Clarify the relationship between <vcpu>, <vcpupin>, and <emulatorpin>

[Hu Tao]

On Fri, Oct 12, 2012 at 01:27:27PM +0800, Osier Yang wrote:
> These 3 elements conflicts with each other in either the doc
> or the underlying codes are. This is to propse a solution.
> 
> Before writing any codes, I want to see if the principle is
> correct, any advise is welcomed.
> 
> Current problems:
> 
> Problem 1:
> 
> The doc shouldn't simply say "These settings are superseded
> by CPU tuning. " for element <vcpu>. As except the tuning, <vcpu>
> allows to specify the current, maxmum vcpu number. Apart from that,
> <vcpu> also allows to specify the placement as "auto", which binds
> the domain process to the advisory nodeset from numad.
> 
> Problem 2:
> 
> Doc for <vcpu> says its "cpuset" specify the physical CPUs
> that the vcpus can be pinned. But it's not the truth, as
> actually it only pin domain process to the specified physical
> CPUs. So either it's a document bug, or code bug.
> 
> Problem 3:
> 
> Doc for <vcpupin> says it supersed "cpuset" of <vcpu>, it's
> not quite correct, as each <vcpupin> specify the pinning policy
> only for one vcpu. How about the ones which doesn't have
> <vcpupin> specified? it says the vcpu will be pinned to all
> available physical CPUs, but what's the meaning of attribute
> "cpuset" of <vcpu> then?
> 
> Problem 4:
> 
> Doc for <emulatorpin> says it pin the emulator threads (domain
> process in other context, perhaps another follow up patch to
> cleanup the inconsistency is needed) to the physical CPUs
> specified its attribute "cpuset". Which conflicts with
> <vcpu>'s "cpuset". And actually in the underlying codes,
> it set the affinity for domain process twice if both
> "cpuset" for <vcpu> and <emulatorpin> are specified,
> and <emulatorpin>'s pinning will override <vcpu>'s.
> 
> Problem 5:
> 
> When "placement" of <vcpu> is "auto" (I.e. uses numad to
> get the advisory nodeset to which the domain process is
> pinned to), it will also be overridden by <emulatorpin>,
> 
> This patch is trying to sort out the conflicts or bugs by:
> 
> 1) Don't say <vcpu> is superseded by <cputune>

You mean in the documentation of XML format?

Acutally the VCPUs placement settings of <vcpu> will be overrided 
by those of <cputune>. So I think it's better to keep the words in
doc to make users aware of this.

> 
> 2) Keep the semanteme for "cpuset" of <vcpu> (I.e. Still says it
>    specify the physical CPUs the virtual CPUs). But modifying it
>    to mention it also set the pinning policy for domain process,
>    and the CPU placement of domain process specified by "cpuset"
>    of <vcpu> will be ingored if <emulatorpin> specified, and
>    similary, the CPU placement of vcpu thread will be ignored
>    if it has <vcpupin> specified, for vcpu which doesn't have
>    <vcpupin> specified, it inherits "cpuset" of <vcpu>.

OK.

> 
> 3) Don't say <vcpu> is supersed by <vcpupin>. If neither <vcpupin>
>    nor "cpuset" of <vcpu> is specified, the vcpu will be pinned
>    to all available pCPUs.

OK.

> 
> 4) If neither <emulatorpin> nor "cpuset" of <vcpu> is specified,
>    the domain process (emulator threads in the context) will be
>    pinned to all available pCPUs.

OK.

> 
> 5) If "placement" of <vcpu> is "auto", <emulatorpin> is not allowed.

Conflicts with 2). Why not just override the emulator part? for vcpu
threads the "placement" is still "auto".

> 
> 6) hotplugged vcpus will also inherit "cpuset" of <vcpu>

OK.

> 
> Codes changes above document changes will cause:
> 
> 1) Inherit def->cpumask for each vcpu which doesn't have <vcpupin>
>    specified, during parsing.

OK.

> 
> 2) ping the vcpu which doesn't have <vcpupin> specified to def->cpumask

s/ping/pin/

>    either by cgroup for sched_setaffinity(2), which is actually done
>    by 1).

pin vcpu according to this_vcpu->cpumask, since the cpumask either
inherits from def->cpumask(<vcpu>), or is set by <vcpupin>.

> 
> 3) Error out if "placement" == "auto", and <emulatorpin> is specified.
>    Otherwise, <emulatorpin> is honored, and "cpuset" of <cpuset> is
>    ignored.

You mean "cpuset" of <vcpu> here?

But I still don't understand why "placement" = "auto" and <emulatorpin>
can not both exist, but the latter overides the former.

> 
> 4) Setup cgroup for each hotplugged vcpu, and setup the pinning policy
>    by either cgroup or sched_setaffinity(2).

OK.

> 
> 5) Remove cgroup and <vcpupin> for each hot unplugged vcpu.
> ---
>  docs/formatdomain.html.in |   42 +++++++++++++++++++++++++++---------------
>  1 files changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index d664e7e..1ae8cf4 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -357,8 +357,18 @@
>          the maximum supported by the hypervisor.  <span class="since">Since
>          0.4.4</span>, this element can contain an optional
>          <code>cpuset</code> attribute, which is a comma-separated
> -        list of physical CPU numbers that virtual CPUs can be pinned
> -        to.  Each element in that list is either a single CPU number,
> +        list of physical CPU numbers that domain process and virtual CPUs
> +        can be pinned to by default. (NB: The pinning policy of domain
> +        process and virtual CPUs can be specified separately by
> +        <code>cputune</code>. If attribute <code>emulatorpin</code>
> +        of <code>cputune</code> is specified, <code>cpuset</code>
> +        specified by <code>vcpu</code> here will be ingored; Similarly,
> +        For virtual CPUs which has <code>vcpupin</code> specified,
> +        <code>cpuset</code> specified by <code>cpuset</code> here
> +        will be ignored; For virtual CPUs which doesn't have
> +        <code>vcpupin</code> specified, it will be pinned to the physical
> +        CPUs specified by <code>cpuset</code> here).
> +        Each element in that list is either a single CPU number,
>          a range of CPU numbers, or a caret followed by a CPU number to
>          be excluded from a previous range.  <span class="since">Since
>          0.8.5</span>, the optional attribute <code>current</code> can
> @@ -374,8 +384,7 @@
>          if it's specified. If both <code>cpuset</code> and <code>placement</code>
>          are not specified, or if <code>placement</code> is "static", but no
>          <code>cpuset</code> is specified, the domain process will be pinned to
> -        all the available physical CPUs. These settings are superseded
> -        by <a href="#elementsCPUTuning">CPU tuning</a>.
> +        all the available physical CPUs.
>        </dd>
>      </dl>
>  
> @@ -411,23 +420,26 @@
>        <dt><code>vcpupin</code></dt>
>        <dd>
>          The optional <code>vcpupin</code> element specifies which of host's
> -        physical CPUs the domain VCPU will be pinned to. This setting supersedes
> -        previous VCPU placement specified in <a href="#elementsCPUAllocation">CPU
> -        Allocation</a> using <code>vcpu</code> element. If this is omitted,
> -        each VCPU is pinned to all the physical CPUs by default. It contains two
> -        required attributes, the attribute <code>vcpu</code> specifies vcpu id,
> -        and the attribute <code>cpuset</code> is same as
> -        attribute <code>cpuset</code>
> -        of element <code>vcpu</code>. (NB: Only qemu driver support)
> +        physical CPUs the domain VCPU will be pinned to. If this is omitted,
> +        and attribute <code>cpuset</code> of element <code>vcpu</code> is
> +        not specified, the vCPU is pinned to all the physical CPUs by default.
> +        It contains two required attributes, the attribute <code>vcpu</code>
> +        specifies vcpu id, and the attribute <code>cpuset</code> is same as
> +        attribute <code>cpuset</code> of element <code>vcpu</code>.
> +        (NB: Only qemu driver support)
>          <span class="since">Since 0.9.0</span>
>         </dd>
>         <dt><code>emulatorpin</code></dt>
>         <dd>
>           The optional <code>emulatorpin</code> element specifies which of host
>           physical CPUs the "emulator", a subset of a domain not including vcpu,
> -         will be pinned to. If this is omitted, "emulator" is pinned to all
> -         the physical CPUs by default. It contains one required attribute
> -         <code>cpuset</code> specifying which physical CPUs to pin to.
> +         will be pinned to. If this is omitted, and attribute
> +         <code>cpuset</code> of element <code>vcpu</code> is not specified,
> +         "emulator" is pinned to all the physical CPUs by default. It contains
> +         one required attribute <code>cpuset</code> specifying which physical
> +         CPUs to pin to. NB, <code>emulatorpin</code> is not allowed if
> +         attribute <code>placement</code> of element <code>vcpu</code> is
> +         "auto".
>         </dd>
>        <dt><code>shares</code></dt>
>        <dd>
> -- 
> 1.7.7.6
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-- 
Thanks,
Hu Tao