Hi,

On 02/02/2015 03:02 AM, 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/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.
> 
> e.g. '1,2@(5-7),(3-5)@(0,2),(0,6),7-8' means starting 9 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)
>   lcore 7 runs on cpuset 0x80 (cpu 7)
>   lcore 8 runs on cpuset 0x100 (cpu 8)
> 
> 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 | 300 
> ++++++++++++++++++++++++++++-
>  lib/librte_eal/common/eal_options.h        |   2 +
>  lib/librte_eal/linuxapp/eal/Makefile       |   1 +
>  4 files changed, 299 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);
>       }
>  }
> -


This line should be removed from the patch.


> diff --git a/lib/librte_eal/common/eal_common_options.c 
> b/lib/librte_eal/common/eal_common_options.c
> index 67e02dc..29ebb6f 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
> @@ -292,6 +296,279 @@ eal_parse_master_lcore(const char *arg)
>       return 0;
>  }
>  
> +/*
> + * Parse elem, the elem could be single number/range 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)

It's not very clear what elem is. Maybe it could be a bit reworded.
What about naming the function "eal_parse_cpuset()" instead?


> +{
> +     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 or single range of number */
> +     if (*str != '(') {
> +             errno = 0;
> +             idx = strtoul(str, &end, 10);
> +             if (errno || end == NULL || idx >= num)
> +                     return -1;
> +             else {
> +                     while (isblank(*end))
> +                             end++;
> +
> +                     min = idx;
> +                     max = idx;
> +                     if (*end == '-') {
> +                             /* proccess single <number>-<number> */
> +                             end++;
> +                             while (isblank(*end))
> +                                     end++;
> +                             if (!isdigit(*end))
> +                                     return -1;
> +
> +                             errno = 0;
> +                             idx = strtoul(end, &end, 10);
> +                             if (errno || end == NULL || idx >= num)
> +                                     return -1;
> +                             max = idx;
> +                             while (isblank(*end))
> +                                     end++;
> +                             if (*end != ',' && *end != '\0')
> +                                     return -1;
> +                     }
> +
> +                     if (*end != ',' && *end != '\0' &&
> +                         *end != '@')
> +                             return -1;
> +
> +                     for (idx = RTE_MIN(min, max);
> +                          idx <= RTE_MAX(min, max); idx++)
> +                             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;
> +}

In the function above, there are some typos in the comments
 seperator -> separator
 qulify -> qualify
 proccess -> process

> +
> +/* convert from set array to cpuset bitmap */
> +static inline int
> +convert_to_cpuset(rte_cpuset_t *cpusetp,
> +           uint16_t *set, unsigned num)

I don't think the function should be inlined



Regards,
Olivier

Reply via email to