Hi Bynes Thanks for your feedback. > -----Original Message----- > From: bynes adam [mailto:adambynes at outlook.com] > Sent: Friday, July 22, 2016 4:45 AM > To: Dai, Wei <wei.dai at intel.com> > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] eal: fix parsing of argument of option > --lcores > > On Thu, Jul 21, 2016 at 02:03:38PM +0800, Wei Dai wrote: > Hi Wei, > > The '-' in lcores set overrides cpu set of following lcore set in the > > argument of EAL option --lcores. > > > > Fixes: 53e54bf81700 ("eal: new option --lcores for cpu assignment") > > > > Signed-off-by: Wei Dai <wei.dai at intel.com> > > --- > > lib/librte_eal/common/eal_common_options.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/lib/librte_eal/common/eal_common_options.c > > b/lib/librte_eal/common/eal_common_options.c > > index 0a594d7..96eb1a9 100644 > > --- a/lib/librte_eal/common/eal_common_options.c > > +++ b/lib/librte_eal/common/eal_common_options.c > > @@ -563,6 +563,7 @@ convert_to_cpuset(rte_cpuset_t *cpusetp, > > * lcores, cpus could be a single digit/range or a group. > > * '(' and ')' are necessary if it's a group. > > * If not supply '@cpus', the value of cpus uses the same as lcores. > > + * The 'a-b' in lcores not within '(' and ')' means a,a+1,...,b-1,b . > this description is not very clear, a-b and (a-b) are both the same meaning. > may be need a table for comparison > a-b@(c-d) > a-b at c-d > (a-b)@c-d > (a-b)@(c-d) > all the above I believe are the same > only the following two cases: > a-b, > (a-b),
With --lcores '0-3 at 12-15', eal_parse_cores( ) will fail because eal_parse_set( ) find the next char after lcore set is '@' and is not ',' or '\0'. So the bug in eal_parse_set( ) should be fixed. A patch v3 will be provided. After fixing, I test it with --lcores '0-3 at 12-15, 4-7@(8-11), (8-11)@4-7, (12-15)@(0-3), 16-19, (20-23) ' It works well. Thanks Wei > so the key point here is the @ and (), not only @ > > * e.g. '1,2@(5-7),(3-5)@(0,2),(0,6),7-8' means start 9 EAL thread as below > > * lcore 0 runs on cpuset 0x41 (cpu 0,6) > > * lcore 1 runs on cpuset 0x2 (cpu 1) > > @@ -571,6 +572,15 @@ convert_to_cpuset(rte_cpuset_t *cpusetp, > > * lcore 6 runs on cpuset 0x41 (cpu 0,6) > > * lcore 7 runs on cpuset 0x80 (cpu 7) > > * lcore 8 runs on cpuset 0x100 (cpu 8) > > + * e.g. '0-2,(3-5)@(3,4),6@(5,6),7@(5-7)'means start 8 EAL threads as > below > > + * lcore 0 runs on cpuset 0x1 (cpu 0) > > + * lcore 1 runs on cpuset 0x2 (cpu 1) > > + * lcore 2 runs on cpuset ox4 (cpu 2) > > + * lcore 3,4,5 runs on cpuset 0x18 (cpu 3,4) > > + * lcore 6 runs on cpuset 0x60 (cpu 5,6) > > + * lcore 7 runs on cpuset 0xe0 (cpu 5,6,7) > > + * The second case is used to test bugfix for lflags not be cleared > > + after use > you can put this sentance and description into the commit log I don't think > you > should put bugfix description in comments here. > > + */ > > */ > > static int > > eal_parse_lcores(const char *lcores) > > @@ -679,6 +689,8 @@ eal_parse_lcores(const char *lcores) > > sizeof(rte_cpuset_t)); > > } > > > > + lflags = 0; > > + > > lcores = end + 1; > > } while (*end != '\0'); > > > > -- > > 2.5.5 > Adam Bynes
