From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 360E65424 for ; Thu, 22 Jan 2015 13:19:26 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 22 Jan 2015 04:19:24 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="443676075" Received: from bricha3-mobl3.ger.corp.intel.com ([10.243.20.25]) by FMSMGA003.fm.intel.com with SMTP; 22 Jan 2015 04:05:59 -0800 Received: by (sSMTP sendmail emulation); Thu, 22 Jan 2015 12:19:21 +0025 Date: Thu, 22 Jan 2015 12:19:21 +0000 From: Bruce Richardson To: Cunming Liang Message-ID: <20150122121921.GD4580@bricha3-MOBL3> References: <1417589628-43666-1-git-send-email-cunming.liang@intel.com> <1421914598-2747-1-git-send-email-cunming.liang@intel.com> <1421914598-2747-3-git-send-email-cunming.liang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1421914598-2747-3-git-send-email-cunming.liang@intel.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for cpu assignment X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Jan 2015 12:19:27 -0000 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 > --- > 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 > #include > #include > +#include > > #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 [-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 >