From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 9CF015A73 for ; Thu, 22 Jan 2015 16:18:00 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP; 22 Jan 2015 07:13:08 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,449,1418112000"; d="scan'208";a="665914779" Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by fmsmga002.fm.intel.com with ESMTP; 22 Jan 2015 07:17:08 -0800 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.28]) by IRSMSX108.ger.corp.intel.com ([169.254.11.64]) with mapi id 14.03.0195.001; Thu, 22 Jan 2015 15:17:07 +0000 From: "Wodkowski, PawelX" To: "Ananyev, Konstantin" , "Richardson, Bruce" , "Liang, Cunming" Thread-Topic: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for cpu assignment Thread-Index: AQHQNj3NyIWk+RuQoUmeDhB+hrz5QZzMNHiAgAAKGvA= Date: Thu, 22 Jan 2015 15:17:07 +0000 Message-ID: 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> <20150122121921.GD4580@bricha3-MOBL3> <2601191342CEEE43887BDE71AB977258213DEF7B@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB977258213DEF7B@irsmsx105.ger.corp.intel.com> Accept-Language: pl-PL, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 15:18:01 -0000 Hi, I want to mention that similar but for me much more readable syntax have=20 Pktgen-DPDK for defining core - port mapping. Maybe we can adopt this synta= x for new '--lcores' parameter. See '-m' parameter syntax on Pktgen readme. https://github.com/pktgen/Pktgen-DPDK/blob/master/dpdk/examples/pktgen/READ= ME.md > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin > Sent: Thursday, January 22, 2015 3:34 PM > To: Richardson, Bruce; Liang, Cunming > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' f= or cpu > assignment >=20 > Hi Bruce, >=20 > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson > > Sent: Thursday, January 22, 2015 12:19 PM > > To: Liang, Cunming > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores'= for cpu > assignment > > > > 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=3D'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 bel= ow > > > 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. >=20 > Didn't get you here: do you find (0,6) confusing, right? > Because braces implicitly specifies affinity for group of en-braced lcore= s? >=20 > > Perhaps we can > > limit the allowed formats here, > > * require the lcore_id to be specified - the lack of an lcore id for th= e last part > > makes having it as lcore 0 surprising. >=20 > Again, not sure I understand you properly: lcore_id(s) are always specif= ied > explicitly. > Physical cpus part might be omitted. >=20 > > * only allow one lcore id to be given for each set of cores. >=20 > So you mean for '(3-5)@(0,2)' user would have to: '3@(0,2),4@(0,2),5@(0,2= )'? > I don't see big difference here, but imagine you'd like to create a pool = of 32 EAL- > threads running on same cpu set. > With current syntax it is just something like: '(32-63)@(0-7)'. > With what you proposing it will be a very long list. >=20 > > > > 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. >=20 > I think that is supported. > See lcore_id=3D1 in Steve's example above. > As I understand: --lcores=3D'0,2,3-5' is equal to '-l 0,2,3-5' and to '-c= 0x3d'. >=20 > Konstantin >=20 > > > > It's probably still not going to be very tidy, but I think we can impro= ve 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[] =3D { > > > {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 =3D=3D RTE_MAX_LCORE) > > > min =3D idx; > > > for (idx =3D min; idx <=3D max; idx++) { > > > - cfg->lcore_role[idx] =3D ROLE_RTE; > > > - lcore_config[idx].core_index =3D count; > > > - count++; > > > + if (cfg->lcore_role[idx] !=3D ROLE_RTE) { > > > + cfg->lcore_role[idx] =3D ROLE_RTE; > > > + lcore_config[idx].core_index =3D count; > > > + count++; > > > + } > > > } > > > min =3D 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 =3D input; > > > + char *end =3D 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 !=3D '(') || *str =3D=3D '\0') > > > + return -1; > > > + > > > + /* process single number */ > > > + if (*str !=3D '(') { > > > + errno =3D 0; > > > + idx =3D strtoul(str, &end, 10); > > > + if (errno || end =3D=3D NULL || idx >=3D num) > > > + return -1; > > > + else { > > > + while (isblank(*end)) > > > + end++; > > > + > > > + if (*end !=3D ',' && *end !=3D '\0' && > > > + *end !=3D '@') > > > + return -1; > > > + > > > + set[idx] =3D 1; > > > + return end - input; > > > + } > > > + } > > > + > > > + /* process set within bracket */ > > > + str++; > > > + while (isblank(*str)) > > > + str++; > > > + if (*str =3D=3D '\0') > > > + return -1; > > > + > > > + min =3D RTE_MAX_LCORE; > > > + do { > > > + > > > + /* go ahead to the first digit */ > > > + while (isblank(*str)) > > > + str++; > > > + if (!isdigit(*str)) > > > + return -1; > > > + > > > + /* get the digit value */ > > > + errno =3D 0; > > > + idx =3D strtoul(str, &end, 10); > > > + if (errno || end =3D=3D NULL || idx >=3D num) > > > + return -1; > > > + > > > + /* go ahead to separator '-',',' and ')' */ > > > + while (isblank(*end)) > > > + end++; > > > + if (*end =3D=3D '-') { > > > + if (min =3D=3D RTE_MAX_LCORE) > > > + min =3D idx; > > > + else /* avoid continuous '-' */ > > > + return -1; > > > + } else if ((*end =3D=3D ',') || (*end =3D=3D ')')) { > > > + max =3D idx; > > > + if (min =3D=3D RTE_MAX_LCORE) > > > + min =3D idx; > > > + for (idx =3D RTE_MIN(min, max); > > > + idx <=3D RTE_MAX(min, max); idx++) > > > + set[idx] =3D 1; > > > + > > > + min =3D RTE_MAX_LCORE; > > > + } else > > > + return -1; > > > + > > > + str =3D end + 1; > > > + } while (*end !=3D '\0' && *end !=3D ')'); > > > + > > > + 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 =3D 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=3D'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 be= low > > > + * 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 =3D rte_eal_get_configuration(); > > > + static uint16_t set[RTE_MAX_LCORE]; > > > + unsigned idx =3D 0; > > > + int i; > > > + unsigned count =3D 0; > > > + const char *lcore_start =3D NULL; > > > + const char *end =3D NULL; > > > + int offset; > > > + rte_cpuset_t cpuset; > > > + int ret =3D -1; > > > + > > > + if (lcores =3D=3D NULL) > > > + return -1; > > > + > > > + /* Remove all blank characters ahead and after */ > > > + while (isblank(*lcores)) > > > + lcores++; > > > + i =3D strnlen(lcores, sysconf(_SC_ARG_MAX)); > > > + while ((i > 0) && isblank(lcores[i - 1])) > > > + i--; > > > + > > > + CPU_ZERO(&cpuset); > > > + > > > + /* Reset lcore config */ > > > + for (idx =3D 0; idx < RTE_MAX_LCORE; idx++) { > > > + cfg->lcore_role[idx] =3D ROLE_OFF; > > > + lcore_config[idx].core_index =3D -1; > > > + CPU_ZERO(&lcore_config[idx].cpuset); > > > + } > > > + > > > + /* Get list of cores */ > > > + do { > > > + while (isblank(*lcores)) > > > + lcores++; > > > + if (*lcores =3D=3D '\0') > > > + goto err; > > > + > > > + /* record lcore_set start point */ > > > + lcore_start =3D lcores; > > > + > > > + /* go across a complete bracket */ > > > + if (*lcore_start =3D=3D '(') { > > > + lcores +=3D strcspn(lcores, ")"); > > > + if (*lcores++ =3D=3D '\0') > > > + goto err; > > > + } > > > + > > > + /* scan the separator '@', ','(next) or '\0'(finish) */ > > > + lcores +=3D strcspn(lcores, "@,"); > > > + > > > + if (*lcores =3D=3D '@') { > > > + /* explict assign cpu_set */ > > > + offset =3D 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 =3D lcores + 1 + offset; > > > + } else /* ',' or '\0' */ > > > + /* haven't given cpu_set, current loop done */ > > > + end =3D lcores; > > > + > > > + if (*end !=3D ',' && *end !=3D '\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 !=3D '@' && > > > + 0 > convert_to_cpuset(&cpuset, set, RTE_DIM(set))) > > > + goto err; > > > + > > > + /* start to update lcore_set */ > > > + for (idx =3D 0; idx < RTE_MAX_LCORE; idx++) { > > > + if (!set[idx]) > > > + continue; > > > + > > > + if (cfg->lcore_role[idx] !=3D ROLE_RTE) { > > > + lcore_config[idx].core_index =3D count; > > > + cfg->lcore_role[idx] =3D ROLE_RTE; > > > + count++; > > > + } > > > + rte_memcpy(&lcore_config[idx].cpuset, &cpuset, > > > + sizeof(rte_cpuset_t)); > > > + } > > > + > > > + lcores =3D end + 1; > > > + } while (*end !=3D '\0'); > > > + > > > + if (count =3D=3D 0) > > > + goto err; > > > + > > > + cfg->lcore_count =3D count; > > > + lcores_parsed =3D 1; > > > + ret =3D 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 =3D 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] !=3D 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 '(' an= d ')'\n" > > > + " Within the group, '-' is used for range se= parator,\n" > > > + " ',' is used for single number separator.\n= " > > > + " '( )' can be omitted for single element gr= oup, '@' \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 :=3D -D_GNU_SOURCE > > > CFLAGS_eal_pci.o :=3D -D_GNU_SOURCE > > > CFLAGS_eal_pci_vfio.o :=3D -D_GNU_SOURCE > > > CFLAGS_eal_common_whitelist.o :=3D -D_GNU_SOURCE > > > +CFLAGS_eal_common_options.o :=3D -D_GNU_SOURCE > > > > > > # workaround for a gcc bug with noreturn attribute > > > # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D12603 > > > -- > > > 1.8.1.4 > > >