|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v11 2/5] xl: move away from the use of cpumap for hard affinity
On Sat, 2014-06-28 at 02:35 +0200, Dario Faggioli wrote:
> and start using the vcpu_hard_affinity array instead. This comes
> with a few bonuses:
>
> - allows us to unify the parsing of the two ways VCPU affinity
> is specified in the domain config file (i.e., cpus="1,3,10-15"
> and cpus=[2, 4, 8]);
>
> - unifying the parsing makes it possible to do things like this:
>
> cpus = ["3-4", "2-6"]
>
> which it was not before. What it means is that VCPU 0 must be
> pinned to PCPU 3,4 and VCPU 1 to PCPUs 2,3,4,5,6. Before this
> change, in fact, the list variant (cpus=[xx, yy]) only supported
> only single values. (Of course, the old [2, 3] syntax continues
> to work, although, without the '"' quotes, it is not possible
> to specify ranges);
>
> - the following patches, introducing soft affinity can share the
> same code path too (after just a small refactoring).
>
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> ---
> Changes from v10:
> * changed the logic that checks whether we are dealing with a
> string or a list a bit. Basically, added a bool flag to store
> that, and this killed the need of having buf2 which on it
> turn needed to be 'spuriously' initialized on gcc >= 4.9.0.
>
> Changes from v9:
> * new patch, basically containing the xl bits of what was the
> cpumap deprecation patch in v9.
> ---
> docs/man/xl.cfg.pod.5 | 8 ++++----
> tools/libxl/xl_cmdimpl.c | 41 +++++++++++++++++++++--------------------
> 2 files changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index ff9ea77..6815bf3 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -143,11 +143,11 @@ Combining this with "all" is also possible, meaning
> "all,^nodes:1"
> results in all the vcpus of the guest running on all the cpus on the
> host, except for the cpus belonging to the host NUMA node 1.
>
> -=item ["2", "3"] (or [2, 3])
> +=item ["2", "3-8,^5"]
>
> -To ask for specific vcpu mapping. That means (in this example), vcpu #0
> -of the guest will run on cpu #2 of the host and vcpu #1 of the guest will
> -run on cpu #3 of the host.
> +To ask for specific vcpu mapping. That means (in this example), vcpu 0
> +of the guest will run on cpu 2 of the host and vcpu 1 of the guest will
> +run on cpus 3,4,6,7,8 of the host.
This syntax is no longer so trivial that it can be inferred from the
example. So I think this needs a more formal specification (for example
I have inferred that ^N means excluding N. Does ^N-M work?). If there is
already a doc somewhere then please add a references.
This can be done in a follow up series.
>
> =back
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index b32345b..5bd116b 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -798,26 +798,39 @@ static void parse_config_data(const char *config_source,
> if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
> b_info->max_vcpus = l;
>
> - if (!xlu_cfg_get_list (config, "cpus", &cpus, &num_cpus, 1)) {
> + buf = NULL; num_cpus = 0;
> + if (!xlu_cfg_get_list (config, "cpus", &cpus, &num_cpus, 1) ||
> + !xlu_cfg_get_string (config, "cpus", &buf, 0)) {
> + /*
> + * If we are here, and buf is !NULL, we're dealing with a string.
> What
> + * we do in this case is parse it, and put the result in _all_ (up to
> + * b_info->max_vcpus) the elements of the vcpu affinity array.
> + *
> + * If buf is NULL, we have a list, and what we do is putting in the
> + * i-eth element of the vcpu affinity array the result of the parsing
> + * of the i-eth entry of the list. If there are more vcpus than
> + * entries, it is fine to just not touch the last array elements.
> + */
> + bool cpus_is_string = !!buf;
I'm not sure that wedging these two cases together has actually resulted
in something better than just handling them individually. Basically
every conditional involves cpus_is_string.
Also when cpus_is_string is true you parse that string repeatedly.
The buf != NULL case is really just:
allocate
vcpu_pinparse into vcpu[0]
foreach vcpu > 0
copy vcpu[0]'s bitmap.
> int j = 0;
>
> - /* Silently ignore values corresponding to non existing vcpus */
> - if (num_cpus > b_info->max_vcpus)
> + if (num_cpus > b_info->max_vcpus || cpus_is_string)
> num_cpus = b_info->max_vcpus;
>
> b_info->vcpu_hard_affinity = xmalloc(num_cpus *
> sizeof(libxl_bitmap));
>
> - while ((buf = xlu_cfg_get_listitem(cpus, j)) != NULL && j <
> num_cpus) {
> - i = atoi(buf);
> -
> + while (j < num_cpus && (cpus_is_string ||
> + (buf = xlu_cfg_get_listitem(cpus, j)) != NULL)) {
> libxl_bitmap_init(&b_info->vcpu_hard_affinity[j]);
> +
> if (libxl_cpu_bitmap_alloc(ctx,
> &b_info->vcpu_hard_affinity[j], 0)) {
> fprintf(stderr, "Unable to allocate cpumap for vcpu %d\n",
> j);
> exit(1);
> }
> - libxl_bitmap_set_none(&b_info->vcpu_hard_affinity[j]);
> - libxl_bitmap_set(&b_info->vcpu_hard_affinity[j], i);
> +
> + if (vcpupin_parse(buf, &b_info->vcpu_hard_affinity[j]))
> + exit(1);
>
> j++;
> }
> @@ -826,18 +839,6 @@ static void parse_config_data(const char *config_source,
> /* We have a list of cpumaps, disable automatic placement */
> libxl_defbool_set(&b_info->numa_placement, false);
> }
> - else if (!xlu_cfg_get_string (config, "cpus", &buf, 0)) {
> - if (libxl_cpu_bitmap_alloc(ctx, &b_info->cpumap, 0)) {
> - fprintf(stderr, "Unable to allocate cpumap\n");
> - exit(1);
> - }
> -
> - libxl_bitmap_set_none(&b_info->cpumap);
> - if (vcpupin_parse(buf, &b_info->cpumap))
> - exit(1);
> -
> - libxl_defbool_set(&b_info->numa_placement, false);
> - }
>
> if (!xlu_cfg_get_long (config, "memory", &l, 0)) {
> b_info->max_memkb = l * 1024;
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |