[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 |