On Thu, Jan 22, 2015 at 04:16:25PM +0800, Cunming Liang wrote:
> It supports one new eal long option '--lcores' for EAL thread cpuset
> assignment.
>
> The format pattern:
> --lcores='lcores[@cpus]<,lcores[@cpus]>'
> lcores, cpus could be a single digit or a group.
> '(' and ')' are necessary if it's a group.
> If not supply '@cpus', the value of cpus uses the same as lcores.
>
> e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means starting 7 EAL thread as below
> lcore 0 runs on cpuset 0x41 (cpu 0,6)
> lcore 1 runs on cpuset 0x2 (cpu 1)
> lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> lcore 6 runs on cpuset 0x41 (cpu 0,6)
>
This strikes me as very confusing, though a couple of tweaks might help with
readability. The lcore 0 at the end is especially confusing. Perhaps we can
limit the allowed formats here,
* require the lcore_id to be specified - the lack of an lcore id for the last
part
makes having it as lcore 0 surprising.
* only allow one lcore id to be given for each set of cores.
I think it may still be readable if we allow the core set to be omitted if its
to be the same as the lcore_id.
It's probably still not going to be very tidy, but I think we can improve
things.
/Bruce
> Signed-off-by: Cunming Liang <cunming.liang at intel.com>
> ---
> lib/librte_eal/common/eal_common_launch.c | 1 -
> lib/librte_eal/common/eal_common_options.c | 262
> ++++++++++++++++++++++++++++-
> lib/librte_eal/common/eal_options.h | 2 +
> lib/librte_eal/linuxapp/eal/Makefile | 1 +
> 4 files changed, 261 insertions(+), 5 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_launch.c
> b/lib/librte_eal/common/eal_common_launch.c
> index 599f83b..2d732b1 100644
> --- a/lib/librte_eal/common/eal_common_launch.c
> +++ b/lib/librte_eal/common/eal_common_launch.c
> @@ -117,4 +117,3 @@ rte_eal_mp_wait_lcore(void)
> rte_eal_wait_lcore(lcore_id);
> }
> }
> -
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> index e2810ab..fc47588 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -45,6 +45,7 @@
> #include <rte_lcore.h>
> #include <rte_version.h>
> #include <rte_devargs.h>
> +#include <rte_memcpy.h>
>
> #include "eal_internal_cfg.h"
> #include "eal_options.h"
> @@ -85,6 +86,7 @@ eal_long_options[] = {
> {OPT_XEN_DOM0, 0, 0, OPT_XEN_DOM0_NUM},
> {OPT_CREATE_UIO_DEV, 1, NULL, OPT_CREATE_UIO_DEV_NUM},
> {OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM},
> + {OPT_LCORES, 1, 0, OPT_LCORES_NUM},
> {0, 0, 0, 0}
> };
>
> @@ -255,9 +257,11 @@ eal_parse_corelist(const char *corelist)
> if (min == RTE_MAX_LCORE)
> min = idx;
> for (idx = min; idx <= max; idx++) {
> - cfg->lcore_role[idx] = ROLE_RTE;
> - lcore_config[idx].core_index = count;
> - count++;
> + if (cfg->lcore_role[idx] != ROLE_RTE) {
> + cfg->lcore_role[idx] = ROLE_RTE;
> + lcore_config[idx].core_index = count;
> + count++;
> + }
> }
> min = RTE_MAX_LCORE;
> } else
> @@ -289,6 +293,241 @@ eal_parse_master_lcore(const char *arg)
> return 0;
> }
>
> +/*
> + * Parse elem, the elem could be single number or '(' ')' group
> + * Within group elem, '-' used for a range seperator;
> + * ',' used for a single number.
> + */
> +static int
> +eal_parse_set(const char *input, uint16_t set[], unsigned num)
> +{
> + unsigned idx;
> + const char *str = input;
> + char *end = NULL;
> + unsigned min, max;
> +
> + memset(set, 0, num * sizeof(uint16_t));
> +
> + while (isblank(*str))
> + str++;
> +
> + /* only digit or left bracket is qulify for start point */
> + if ((!isdigit(*str) && *str != '(') || *str == '\0')
> + return -1;
> +
> + /* process single number */
> + if (*str != '(') {
> + errno = 0;
> + idx = strtoul(str, &end, 10);
> + if (errno || end == NULL || idx >= num)
> + return -1;
> + else {
> + while (isblank(*end))
> + end++;
> +
> + if (*end != ',' && *end != '\0' &&
> + *end != '@')
> + return -1;
> +
> + set[idx] = 1;
> + return end - input;
> + }
> + }
> +
> + /* process set within bracket */
> + str++;
> + while (isblank(*str))
> + str++;
> + if (*str == '\0')
> + return -1;
> +
> + min = RTE_MAX_LCORE;
> + do {
> +
> + /* go ahead to the first digit */
> + while (isblank(*str))
> + str++;
> + if (!isdigit(*str))
> + return -1;
> +
> + /* get the digit value */
> + errno = 0;
> + idx = strtoul(str, &end, 10);
> + if (errno || end == NULL || idx >= num)
> + return -1;
> +
> + /* go ahead to separator '-',',' and ')' */
> + while (isblank(*end))
> + end++;
> + if (*end == '-') {
> + if (min == RTE_MAX_LCORE)
> + min = idx;
> + else /* avoid continuous '-' */
> + return -1;
> + } else if ((*end == ',') || (*end == ')')) {
> + max = idx;
> + if (min == RTE_MAX_LCORE)
> + min = idx;
> + for (idx = RTE_MIN(min, max);
> + idx <= RTE_MAX(min, max); idx++)
> + set[idx] = 1;
> +
> + min = RTE_MAX_LCORE;
> + } else
> + return -1;
> +
> + str = end + 1;
> + } while (*end != '\0' && *end != ')');
> +
> + return str - input;
> +}
> +
> +/* convert from set array to cpuset bitmap */
> +static inline int
> +convert_to_cpuset(rte_cpuset_t *cpusetp,
> + uint16_t *set, unsigned num)
> +{
> + unsigned idx;
> +
> + CPU_ZERO(cpusetp);
> +
> + for (idx = 0; idx < num; idx++) {
> + if (!set[idx])
> + continue;
> +
> + if (!lcore_config[idx].detected) {
> + RTE_LOG(ERR, EAL, "core %u "
> + "unavailable\n", idx);
> + return -1;
> + }
> +
> + CPU_SET(idx, cpusetp);
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * The format pattern: --lcores='lcores[@cpus]<,lcores[@cpus]>'
> + * lcores, cpus could be a single digit or a group.
> + * '(' and ')' are necessary if it's a group.
> + * If not supply '@cpus', the value of cpus uses the same as lcores.
> + * e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means start 7 EAL thread as below
> + * lcore 0 runs on cpuset 0x41 (cpu 0,6)
> + * lcore 1 runs on cpuset 0x2 (cpu 1)
> + * lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> + * lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> + * lcore 6 runs on cpuset 0x41 (cpu 0,6)
> + */
> +static int
> +eal_parse_lcores(const char *lcores)
> +{
> + struct rte_config *cfg = rte_eal_get_configuration();
> + static uint16_t set[RTE_MAX_LCORE];
> + unsigned idx = 0;
> + int i;
> + unsigned count = 0;
> + const char *lcore_start = NULL;
> + const char *end = NULL;
> + int offset;
> + rte_cpuset_t cpuset;
> + int ret = -1;
> +
> + if (lcores == NULL)
> + return -1;
> +
> + /* Remove all blank characters ahead and after */
> + while (isblank(*lcores))
> + lcores++;
> + i = strnlen(lcores, sysconf(_SC_ARG_MAX));
> + while ((i > 0) && isblank(lcores[i - 1]))
> + i--;
> +
> + CPU_ZERO(&cpuset);
> +
> + /* Reset lcore config */
> + for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> + cfg->lcore_role[idx] = ROLE_OFF;
> + lcore_config[idx].core_index = -1;
> + CPU_ZERO(&lcore_config[idx].cpuset);
> + }
> +
> + /* Get list of cores */
> + do {
> + while (isblank(*lcores))
> + lcores++;
> + if (*lcores == '\0')
> + goto err;
> +
> + /* record lcore_set start point */
> + lcore_start = lcores;
> +
> + /* go across a complete bracket */
> + if (*lcore_start == '(') {
> + lcores += strcspn(lcores, ")");
> + if (*lcores++ == '\0')
> + goto err;
> + }
> +
> + /* scan the separator '@', ','(next) or '\0'(finish) */
> + lcores += strcspn(lcores, "@,");
> +
> + if (*lcores == '@') {
> + /* explict assign cpu_set */
> + offset = eal_parse_set(lcores + 1, set, RTE_DIM(set));
> + if (offset < 0)
> + goto err;
> +
> + /* prepare cpu_set and update the end cursor */
> + if (0 > convert_to_cpuset(&cpuset,
> + set, RTE_DIM(set)))
> + goto err;
> + end = lcores + 1 + offset;
> + } else /* ',' or '\0' */
> + /* haven't given cpu_set, current loop done */
> + end = lcores;
> +
> + if (*end != ',' && *end != '\0')
> + goto err;
> +
> + /* parse lcore_set from start point */
> + if (0 > eal_parse_set(lcore_start, set, RTE_DIM(set)))
> + goto err;
> +
> + /* without '@', by default using lcore_set as cpu_set */
> + if (*lcores != '@' &&
> + 0 > convert_to_cpuset(&cpuset, set, RTE_DIM(set)))
> + goto err;
> +
> + /* start to update lcore_set */
> + for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> + if (!set[idx])
> + continue;
> +
> + if (cfg->lcore_role[idx] != ROLE_RTE) {
> + lcore_config[idx].core_index = count;
> + cfg->lcore_role[idx] = ROLE_RTE;
> + count++;
> + }
> + rte_memcpy(&lcore_config[idx].cpuset, &cpuset,
> + sizeof(rte_cpuset_t));
> + }
> +
> + lcores = end + 1;
> + } while (*end != '\0');
> +
> + if (count == 0)
> + goto err;
> +
> + cfg->lcore_count = count;
> + lcores_parsed = 1;
> + ret = 0;
> +
> +err:
> +
> + return ret;
> +}
> +
> static int
> eal_parse_syslog(const char *facility, struct internal_config *conf)
> {
> @@ -489,6 +728,13 @@ eal_parse_common_option(int opt, const char *optarg,
> conf->log_level = log;
> break;
> }
> + case OPT_LCORES_NUM:
> + if (eal_parse_lcores(optarg) < 0) {
> + RTE_LOG(ERR, EAL, "invalid parameter for --"
> + OPT_LCORES "\n");
> + return -1;
> + }
> + break;
>
> /* don't know what to do, leave this to caller */
> default:
> @@ -527,7 +773,7 @@ eal_check_common_options(struct internal_config
> *internal_cfg)
>
> if (!lcores_parsed) {
> RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
> - "-c or -l\n");
> + "-c, -l or --lcores\n");
> return -1;
> }
> if (cfg->lcore_role[cfg->master_lcore] != ROLE_RTE) {
> @@ -583,6 +829,14 @@ eal_common_usage(void)
> " The argument format is
> <c1>[-c2][,c3[-c4],...]\n"
> " where c1, c2, etc are core indexes between 0
> and %d\n"
> " --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n"
> + " --"OPT_LCORES" MAP: maps between lcore_set to phys_cpu_set\n"
> + " The argument format is\n"
> + " 'lcores[@cpus]<,lcores[@cpus],...>'\n"
> + " lcores and cpus list are grouped by '(' and
> ')'\n"
> + " Within the group, '-' is used for range
> separator,\n"
> + " ',' is used for single number separator.\n"
> + " '( )' can be omitted for single element group,
> '@' \n"
> + " can be omitted if cpus and lcores has the same
> value\n"
> " -n NUM : Number of memory channels\n"
> " -v : Display version information on startup\n"
> " -m MB : memory to allocate (see also
> --"OPT_SOCKET_MEM")\n"
> diff --git a/lib/librte_eal/common/eal_options.h
> b/lib/librte_eal/common/eal_options.h
> index e476f8d..a1cc59f 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -77,6 +77,8 @@ enum {
> OPT_CREATE_UIO_DEV_NUM,
> #define OPT_VFIO_INTR "vfio-intr"
> OPT_VFIO_INTR_NUM,
> +#define OPT_LCORES "lcores"
> + OPT_LCORES_NUM,
> OPT_LONG_MAX_NUM
> };
>
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile
> b/lib/librte_eal/linuxapp/eal/Makefile
> index 0e9c447..025d836 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -95,6 +95,7 @@ CFLAGS_eal_hugepage_info.o := -D_GNU_SOURCE
> CFLAGS_eal_pci.o := -D_GNU_SOURCE
> CFLAGS_eal_pci_vfio.o := -D_GNU_SOURCE
> CFLAGS_eal_common_whitelist.o := -D_GNU_SOURCE
> +CFLAGS_eal_common_options.o := -D_GNU_SOURCE
>
> # workaround for a gcc bug with noreturn attribute
> # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
> --
> 1.8.1.4
>
