diff mbox

[1/2] isolation : initial odp isolation example app

Message ID 1408347660-22842-1-git-send-email-santosh.shukla@linaro.org
State New
Headers show

Commit Message

Santosh Shukla Aug. 18, 2014, 7:41 a.m. UTC
Test application to run odp dummy dp thread
on no_hz_full kernel mode for few seconds.

- Create 2 threads, running in loop for few seconds.
- Mask timer thread.

Signed-off-by: Santosh Shukla <santosh.shukla@linaro.org>
---
Note :
- Patch set based on Maxim's v2 patch under discussion
    "implement-odp_init_global-init-mask.patch"
- Open for review feedback however wont merge due
  above dependancy.
- More isolation specific changes seen at
https://git.linaro.org/people/santosh.shukla/odp.git

 configure.ac                      |    1 +
 example/Makefile.am               |    2 +-
 example/isolation/Makefile.am     |    5 +
 example/isolation/odp_isolation.c |  232 +++++++++++++++++++++++++++++++++++++
 4 files changed, 239 insertions(+), 1 deletion(-)
 create mode 100644 example/isolation/Makefile.am
 create mode 100644 example/isolation/odp_isolation.c

Comments

Stuart Haslam Aug. 18, 2014, 11 a.m. UTC | #1
On Mon, Aug 18, 2014 at 08:41:00AM +0100, Santosh Shukla wrote:
> Test application to run odp dummy dp thread
> on no_hz_full kernel mode for few seconds.
> 
> - Create 2 threads, running in loop for few seconds.
> - Mask timer thread.
> 
> Signed-off-by: Santosh Shukla <santosh.shukla@linaro.org>
> ---
> Note :
> - Patch set based on Maxim's v2 patch under discussion
>     "implement-odp_init_global-init-mask.patch"
> - Open for review feedback however wont merge due
>   above dependancy.
> - More isolation specific changes seen at
> https://git.linaro.org/people/santosh.shukla/odp.git
> 
>  configure.ac                      |    1 +
>  example/Makefile.am               |    2 +-
>  example/isolation/Makefile.am     |    5 +
>  example/isolation/odp_isolation.c |  232 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 239 insertions(+), 1 deletion(-)
>  create mode 100644 example/isolation/Makefile.am
>  create mode 100644 example/isolation/odp_isolation.c
> 
> diff --git a/configure.ac b/configure.ac
> index 74713a7..2b6758b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -142,6 +142,7 @@ AC_CONFIG_FILES([Makefile
>  		 example/packet/Makefile
>  		 example/packet_netmap/Makefile
>  		 example/timer/Makefile
> +		 example/isolation/Makefile
>  		 test/Makefile
>  		 test/api_test/Makefile
>  		 pkgconfig/libodp.pc])
> diff --git a/example/Makefile.am b/example/Makefile.am
> index 01a3305..4cb4a23 100644
> --- a/example/Makefile.am
> +++ b/example/Makefile.am
> @@ -1 +1 @@
> -SUBDIRS = generator l2fwd odp_example packet packet_netmap timer
> +SUBDIRS = generator l2fwd odp_example packet packet_netmap timer isolation
> diff --git a/example/isolation/Makefile.am b/example/isolation/Makefile.am
> new file mode 100644
> index 0000000..7e39773
> --- /dev/null
> +++ b/example/isolation/Makefile.am
> @@ -0,0 +1,5 @@
> +include $(top_srcdir)/example/Makefile.inc
> +
> +bin_PROGRAMS = odp_isolation
> +
> +dist_odp_isolation_SOURCES = odp_isolation.c
> diff --git a/example/isolation/odp_isolation.c b/example/isolation/odp_isolation.c
> new file mode 100644
> index 0000000..8d8fffc
> --- /dev/null
> +++ b/example/isolation/odp_isolation.c
> @@ -0,0 +1,232 @@
> +/* Copyright (c) 2013, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +
> +/**
> + * @file
> + *
> + * @example  odp_isol_test.c ODP isolation example application
> + */
> +
> +#include <string.h>
> +#include <stdlib.h>
> +
> +/* ODP main header */
> +#include <odp.h>
> +
> +/* ODP helper for Linux apps */
> +#include <helper/odp_linux.h>
> +
> +/* GNU lib C */
> +#include <getopt.h>
> +
> +#define MAX_WORKERS	32	/**< Max worker threads */
> +
> +/**
> + * Parsed command line application arguments
> + */
> +typedef struct {
> +	int core_count; /**< Core count*/
> +	char **core_name;	/**< Array of pointers to core name */
> +} appl_args_t;
> +
> +/**
> + * @internal Print help
> + */
> +static void print_usage(void)
> +{
> +	printf("\n\nUsage: ./odp_isolation [options]\n");
> +	printf("Options:\n");
> +	printf("  -l, --cpulist <cpu number in comma separated fashion>\n");
> +	printf("  -h, --help              this help\n");
> +	printf("\n\n");
> +}
> +
> +/**
> + * @internal Worker thread
> + *
> + * @param arg  Arguments
> + *
> + * @return NULL on failure
> + */
> +static void *run_thread(void *arg)
> +{
> +	int thr;
> +	unsigned long long tick = 0;
> +	int counter = 0;
> +
> +	thr = odp_thread_id();
> +
> +	printf("Thread %i starts on core %i\n", thr, odp_thread_core());
> +
> +	/* loop for some duration then exit */
> +	do {
> +		tick++;
> +
> +		if (tick > 10000000) {
> +
> +			tick = 0;
> +			counter++;
> +		}
> +
> +		if (counter == 1000)
> +			break;
> +	} while (1);
> +
> +	return arg;
> +}

I'm not sure it's worth adding an example application that does nothing.
Isolation is not be something the application needs explicit knowledge
of so having an example app demonstrating it is a bit odd. All of the
existing examples / tests should be capable of being run isolated.

For this test you could start with odp_example as it has no dependencies
and will exit after some time - if you need it to run for a particular
amount of time you could add an argument for that.

> +
> +/**
> + * @internal Parse arguments
> + *
> + * @param argc  Argument count
> + * @param argv  Argument vector
> + * @param args  Test arguments
> + */
> +static void parse_args(int argc, char *argv[], appl_args_t *args)
> +{
> +	int opt;
> +	int long_index;
> +	char *names, *str, *token, *save;
> +	size_t len;
> +	int i;
> +
> +	static struct option longopts[] = {
> +		{"cpulists", required_argument, NULL, 'l'}, /* return 'l' */
> +		{"help", no_argument, NULL, 'h'}, /* return 'h' */
> +		{NULL, 0, NULL, 0}
> +	};
> +
> +	while (1) {
> +		opt = getopt_long(argc, argv, "+l:h", longopts, &long_index);
> +
> +		if (opt == -1)
> +			break;	/* No more options */
> +
> +		switch (opt) {
> +		case 'l':
> +			len = strlen(optarg);
> +			if (len == 0) {
> +				print_usage();
> +				exit(EXIT_FAILURE);
> +			}
> +			len += 1;	/* add room for '\0' */
> +
> +			names = malloc(len);
> +			if (names == NULL) {
> +				print_usage();
> +				exit(EXIT_FAILURE);
> +			}
> +
> +			/* count the number of tokens separated by ',' */
> +			strcpy(names, optarg);
> +			for (str = names, i = 0;; str = NULL, i++) {
> +				token = strtok_r(str, ",", &save);
> +				if (token == NULL)
> +					break;
> +			}
> +			args->core_count = i;
> +
> +			if (args->core_count == 0) {
> +				print_usage();
> +				exit(EXIT_FAILURE);
> +			}
> +			/* allocate storage for the if names */
> +			args->core_name =
> +				calloc(args->core_count, sizeof(char *));
> +
> +			/* store the if names (reset names string) */
> +			strcpy(names, optarg);
> +			for (str = names, i = 0;; str = NULL, i++) {
> +				token = strtok_r(str, ",", &save);
> +				if (token == NULL)
> +					break;
> +				args->core_name[i] = token;
> +			}
> +			break;
> +

It would be better to change the behaviour of the existing -c
argument used in most of the examples from being a core count to
something similar to what you've done with -l here. Other related
applications (taskset [1], DPDK [2]) use -c to provide either a core
mask or a list.

[1] http://linux.die.net/man/1/taskset
[2] http://www.dpdk.info/ml/archives/dev/2014-April/002173.html

> +		case 'h':
> +			print_usage();
> +			exit(EXIT_SUCCESS);
> +			break;
> +
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
> +/**
> + * Isolation test application
> + */
> +int main(int argc, char *argv[])
> +{
> +	odp_linux_pthread_t thread_tbl[MAX_WORKERS];
> +	appl_args_t args;
> +	int thr_id;
> +	int num_workers;
> +	int first_core;
> +	int i;
> +
> +	printf("\nODP isolation test application\n");
> +
> +	memset(&args, 0, sizeof(args));
> +	parse_args(argc, argv, &args);
> +
> +	memset(thread_tbl, 0, sizeof(thread_tbl));
> +
> +	/*
> +	 * Don't need timer init for isolation, so mask them.
> +	 */
> +	if (odp_init_global(ODP_INIT_F_ALL & ~ODP_INIT_F_TIMER)) {
> +		printf("ODP global init failed.\n");
> +		return -1;
> +	}
> +
> +	printf("\n");
> +	printf("ODP system info\n");
> +	printf("---------------\n");
> +	printf("ODP API version: %s\n",        odp_version_api_str());
> +	printf("CPU model:       %s\n",        odp_sys_cpu_model_str());
> +	printf("CPU freq (hz):   %"PRIu64"\n", odp_sys_cpu_hz());
> +	printf("Cache line size: %i\n",        odp_sys_cache_line_size());
> +	printf("Max core count:  %i\n",        odp_sys_core_count());
> +
> +	printf("\n");
> +
> +	/* A worker thread per core */
> +	num_workers = odp_sys_core_count();
> +
> +	if (args.core_count)
> +		num_workers = args.core_count;
> +
> +	/* force to max core count */
> +	if (num_workers > MAX_WORKERS)
> +		num_workers = MAX_WORKERS;
> +
> +	printf("num worker threads: %i\n", num_workers);
> +
> +	/*
> +	 * Init this thread.
> +	 * */
> +	thr_id = odp_thread_create(0);
> +	odp_init_local(thr_id);
> +
> +	for (i=0; i<num_workers; i++) {
> +
> +		first_core = atoi(args.core_name[i]);
> +		/* Create and launch worker threads */
> +		odp_linux_pthread_create(&thread_tbl[i], 1, first_core,
> +					 run_thread, NULL);
> +	}
> +

I wonder if changing odp_linux_pthread_create to take an array of cores
rather than just the first core is a better way to go. A quick grep
through the existing examples / tests shows almost all callers provide 1
for the number of cores, so it's not currently being used as expected.
Santosh Shukla Aug. 18, 2014, 11:19 a.m. UTC | #2
On 18 August 2014 16:30, Stuart Haslam <stuart.haslam@arm.com> wrote:
> On Mon, Aug 18, 2014 at 08:41:00AM +0100, Santosh Shukla wrote:
>> Test application to run odp dummy dp thread
>> on no_hz_full kernel mode for few seconds.
>>
>> - Create 2 threads, running in loop for few seconds.
>> - Mask timer thread.
>>
>> Signed-off-by: Santosh Shukla <santosh.shukla@linaro.org>
>> ---
>> Note :
>> - Patch set based on Maxim's v2 patch under discussion
>>     "implement-odp_init_global-init-mask.patch"
>> - Open for review feedback however wont merge due
>>   above dependancy.
>> - More isolation specific changes seen at
>> https://git.linaro.org/people/santosh.shukla/odp.git
>>
>>  configure.ac                      |    1 +
>>  example/Makefile.am               |    2 +-
>>  example/isolation/Makefile.am     |    5 +
>>  example/isolation/odp_isolation.c |  232 +++++++++++++++++++++++++++++++++++++
>>  4 files changed, 239 insertions(+), 1 deletion(-)
>>  create mode 100644 example/isolation/Makefile.am
>>  create mode 100644 example/isolation/odp_isolation.c
>>
>> diff --git a/configure.ac b/configure.ac
>> index 74713a7..2b6758b 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -142,6 +142,7 @@ AC_CONFIG_FILES([Makefile
>>                example/packet/Makefile
>>                example/packet_netmap/Makefile
>>                example/timer/Makefile
>> +              example/isolation/Makefile
>>                test/Makefile
>>                test/api_test/Makefile
>>                pkgconfig/libodp.pc])
>> diff --git a/example/Makefile.am b/example/Makefile.am
>> index 01a3305..4cb4a23 100644
>> --- a/example/Makefile.am
>> +++ b/example/Makefile.am
>> @@ -1 +1 @@
>> -SUBDIRS = generator l2fwd odp_example packet packet_netmap timer
>> +SUBDIRS = generator l2fwd odp_example packet packet_netmap timer isolation
>> diff --git a/example/isolation/Makefile.am b/example/isolation/Makefile.am
>> new file mode 100644
>> index 0000000..7e39773
>> --- /dev/null
>> +++ b/example/isolation/Makefile.am
>> @@ -0,0 +1,5 @@
>> +include $(top_srcdir)/example/Makefile.inc
>> +
>> +bin_PROGRAMS = odp_isolation
>> +
>> +dist_odp_isolation_SOURCES = odp_isolation.c
>> diff --git a/example/isolation/odp_isolation.c b/example/isolation/odp_isolation.c
>> new file mode 100644
>> index 0000000..8d8fffc
>> --- /dev/null
>> +++ b/example/isolation/odp_isolation.c
>> @@ -0,0 +1,232 @@
>> +/* Copyright (c) 2013, Linaro Limited
>> + * All rights reserved.
>> + *
>> + * SPDX-License-Identifier:     BSD-3-Clause
>> + */
>> +
>> +/**
>> + * @file
>> + *
>> + * @example  odp_isol_test.c ODP isolation example application
>> + */
>> +
>> +#include <string.h>
>> +#include <stdlib.h>
>> +
>> +/* ODP main header */
>> +#include <odp.h>
>> +
>> +/* ODP helper for Linux apps */
>> +#include <helper/odp_linux.h>
>> +
>> +/* GNU lib C */
>> +#include <getopt.h>
>> +
>> +#define MAX_WORKERS  32      /**< Max worker threads */
>> +
>> +/**
>> + * Parsed command line application arguments
>> + */
>> +typedef struct {
>> +     int core_count; /**< Core count*/
>> +     char **core_name;       /**< Array of pointers to core name */
>> +} appl_args_t;
>> +
>> +/**
>> + * @internal Print help
>> + */
>> +static void print_usage(void)
>> +{
>> +     printf("\n\nUsage: ./odp_isolation [options]\n");
>> +     printf("Options:\n");
>> +     printf("  -l, --cpulist <cpu number in comma separated fashion>\n");
>> +     printf("  -h, --help              this help\n");
>> +     printf("\n\n");
>> +}
>> +
>> +/**
>> + * @internal Worker thread
>> + *
>> + * @param arg  Arguments
>> + *
>> + * @return NULL on failure
>> + */
>> +static void *run_thread(void *arg)
>> +{
>> +     int thr;
>> +     unsigned long long tick = 0;
>> +     int counter = 0;
>> +
>> +     thr = odp_thread_id();
>> +
>> +     printf("Thread %i starts on core %i\n", thr, odp_thread_core());
>> +
>> +     /* loop for some duration then exit */
>> +     do {
>> +             tick++;
>> +
>> +             if (tick > 10000000) {
>> +
>> +                     tick = 0;
>> +                     counter++;
>> +             }
>> +
>> +             if (counter == 1000)
>> +                     break;
>> +     } while (1);
>> +
>> +     return arg;
>> +}
>
> I'm not sure it's worth adding an example application that does nothing.
> Isolation is not be something the application needs explicit knowledge
> of so having an example app demonstrating it is a bit odd. All of the
> existing examples / tests should be capable of being run isolated.
>
> For this test you could start with odp_example as it has no dependencies

Not entirely true and I did thought about using odp_example.
odp_example does use timer api (clock_gettime) which I don't want to
call from isolated thread. Therefore choose to keep dummy (explicitly
mentioned in writeup) thread looping for x time.

More appropriate test application is l2fwd, where I am doing isolation.

There is a readme (in next patch) which very much informs NOT to use
odp method of cpuset for thread pinning instead use script. I could
choose to add job enqueue/dequeue in while loop But in next patch.

> and will exit after some time - if you need it to run for a particular
> amount of time you could add an argument for that.
>
>> +
>> +/**
>> + * @internal Parse arguments
>> + *
>> + * @param argc  Argument count
>> + * @param argv  Argument vector
>> + * @param args  Test arguments
>> + */
>> +static void parse_args(int argc, char *argv[], appl_args_t *args)
>> +{
>> +     int opt;
>> +     int long_index;
>> +     char *names, *str, *token, *save;
>> +     size_t len;
>> +     int i;
>> +
>> +     static struct option longopts[] = {
>> +             {"cpulists", required_argument, NULL, 'l'}, /* return 'l' */
>> +             {"help", no_argument, NULL, 'h'}, /* return 'h' */
>> +             {NULL, 0, NULL, 0}
>> +     };
>> +
>> +     while (1) {
>> +             opt = getopt_long(argc, argv, "+l:h", longopts, &long_index);
>> +
>> +             if (opt == -1)
>> +                     break;  /* No more options */
>> +
>> +             switch (opt) {
>> +             case 'l':
>> +                     len = strlen(optarg);
>> +                     if (len == 0) {
>> +                             print_usage();
>> +                             exit(EXIT_FAILURE);
>> +                     }
>> +                     len += 1;       /* add room for '\0' */
>> +
>> +                     names = malloc(len);
>> +                     if (names == NULL) {
>> +                             print_usage();
>> +                             exit(EXIT_FAILURE);
>> +                     }
>> +
>> +                     /* count the number of tokens separated by ',' */
>> +                     strcpy(names, optarg);
>> +                     for (str = names, i = 0;; str = NULL, i++) {
>> +                             token = strtok_r(str, ",", &save);
>> +                             if (token == NULL)
>> +                                     break;
>> +                     }
>> +                     args->core_count = i;
>> +
>> +                     if (args->core_count == 0) {
>> +                             print_usage();
>> +                             exit(EXIT_FAILURE);
>> +                     }
>> +                     /* allocate storage for the if names */
>> +                     args->core_name =
>> +                             calloc(args->core_count, sizeof(char *));
>> +
>> +                     /* store the if names (reset names string) */
>> +                     strcpy(names, optarg);
>> +                     for (str = names, i = 0;; str = NULL, i++) {
>> +                             token = strtok_r(str, ",", &save);
>> +                             if (token == NULL)
>> +                                     break;
>> +                             args->core_name[i] = token;
>> +                     }
>> +                     break;
>> +
>
> It would be better to change the behaviour of the existing -c
> argument used in most of the examples from being a core count to
> something similar to what you've done with -l here. Other related
> applications (taskset [1], DPDK [2]) use -c to provide either a core
> mask or a list.

Right, I though so. I'll do.
>
> [1] http://linux.die.net/man/1/taskset
> [2] http://www.dpdk.info/ml/archives/dev/2014-April/002173.html
>
>> +             case 'h':
>> +                     print_usage();
>> +                     exit(EXIT_SUCCESS);
>> +                     break;
>> +
>> +             default:
>> +                     break;
>> +             }
>> +     }
>> +}
>> +
>> +/**
>> + * Isolation test application
>> + */
>> +int main(int argc, char *argv[])
>> +{
>> +     odp_linux_pthread_t thread_tbl[MAX_WORKERS];
>> +     appl_args_t args;
>> +     int thr_id;
>> +     int num_workers;
>> +     int first_core;
>> +     int i;
>> +
>> +     printf("\nODP isolation test application\n");
>> +
>> +     memset(&args, 0, sizeof(args));
>> +     parse_args(argc, argv, &args);
>> +
>> +     memset(thread_tbl, 0, sizeof(thread_tbl));
>> +
>> +     /*
>> +      * Don't need timer init for isolation, so mask them.
>> +      */
>> +     if (odp_init_global(ODP_INIT_F_ALL & ~ODP_INIT_F_TIMER)) {
>> +             printf("ODP global init failed.\n");
>> +             return -1;
>> +     }
>> +
>> +     printf("\n");
>> +     printf("ODP system info\n");
>> +     printf("---------------\n");
>> +     printf("ODP API version: %s\n",        odp_version_api_str());
>> +     printf("CPU model:       %s\n",        odp_sys_cpu_model_str());
>> +     printf("CPU freq (hz):   %"PRIu64"\n", odp_sys_cpu_hz());
>> +     printf("Cache line size: %i\n",        odp_sys_cache_line_size());
>> +     printf("Max core count:  %i\n",        odp_sys_core_count());
>> +
>> +     printf("\n");
>> +
>> +     /* A worker thread per core */
>> +     num_workers = odp_sys_core_count();
>> +
>> +     if (args.core_count)
>> +             num_workers = args.core_count;
>> +
>> +     /* force to max core count */
>> +     if (num_workers > MAX_WORKERS)
>> +             num_workers = MAX_WORKERS;
>> +
>> +     printf("num worker threads: %i\n", num_workers);
>> +
>> +     /*
>> +      * Init this thread.
>> +      * */
>> +     thr_id = odp_thread_create(0);
>> +     odp_init_local(thr_id);
>> +
>> +     for (i=0; i<num_workers; i++) {
>> +
>> +             first_core = atoi(args.core_name[i]);
>> +             /* Create and launch worker threads */
>> +             odp_linux_pthread_create(&thread_tbl[i], 1, first_core,
>> +                                      run_thread, NULL);
>> +     }
>> +
>
> I wonder if changing odp_linux_pthread_create to take an array of cores
> rather than just the first core is a better way to go. A quick grep
> through the existing examples / tests shows almost all callers provide 1
> for the number of cores, so it's not currently being used as expected.
>

Nope. Not in this case, I want to create thread on any of cores based
on arg_list, That may be look like this -
-l 2,3,5,6 OR -l 3,5,6,7

Also changing the order helps me to test my isolation script.

> --
> Stuart.
>
Stuart Haslam Aug. 18, 2014, 1:57 p.m. UTC | #3
On Mon, Aug 18, 2014 at 12:19:49PM +0100, Santosh Shukla wrote:
> On 18 August 2014 16:30, Stuart Haslam <stuart.haslam@arm.com> wrote:
> > On Mon, Aug 18, 2014 at 08:41:00AM +0100, Santosh Shukla wrote:

> >
> > I'm not sure it's worth adding an example application that does nothing.
> > Isolation is not be something the application needs explicit knowledge
> > of so having an example app demonstrating it is a bit odd. All of the
> > existing examples / tests should be capable of being run isolated.
> >
> > For this test you could start with odp_example as it has no dependencies
> 
> Not entirely true and I did thought about using odp_example.
> odp_example does use timer api (clock_gettime) which I don't want to
> call from isolated thread. Therefore choose to keep dummy (explicitly
> mentioned in writeup) thread looping for x time.
> 

OK fair enough, so the odp_example would currently fail this test
because there's a problem with the linu-generic implementation of the
timers, this is a known issue and should be fixed at some point.

> More appropriate test application is l2fwd, where I am doing isolation.
> 

OK, can you send patches for that instead? :)

> There is a readme (in next patch) which very much informs NOT to use
> odp method of cpuset for thread pinning instead use script.

Yeah I read that - it doesn't say why not though.

> I could
> choose to add job enqueue/dequeue in while loop But in next patch.

I don't think that's the right direction to head. Given that any of the
tests or examples should be capable of running isolated, if we were to
have a specific isolation test then it would just end up replicating all
of the other tests. It would be better to just run the existing tests
on isolated cores. As you say that's not possible at the minute, but it
should be a goal - and the easiest way to start is by picking an
existing test that does work on isolated cores.

> >
> > I wonder if changing odp_linux_pthread_create to take an array of cores
> > rather than just the first core is a better way to go. A quick grep
> > through the existing examples / tests shows almost all callers provide 1
> > for the number of cores, so it's not currently being used as expected.
> >
> 
> Nope. Not in this case, I want to create thread on any of cores based
> on arg_list, That may be look like this -
> -l 2,3,5,6 OR -l 3,5,6,7
> 
> Also changing the order helps me to test my isolation script.
>

Yes, what I was suggesting would make that easier. Instead of looping
over num_workers and setting first_core and num for each iteration you
just pass in an array of core numbers that you want threads started on,
e.g.;

odp_linux_pthread_create(thread_tbl, num_workers, args.core_num,
                         run_thread, NULL);

Where args.core_num is just what you currently have as args.core_name
but stored as an integer.
Santosh Shukla Aug. 18, 2014, 3:10 p.m. UTC | #4
On 18 August 2014 19:27, Stuart Haslam <stuart.haslam@arm.com> wrote:
> On Mon, Aug 18, 2014 at 12:19:49PM +0100, Santosh Shukla wrote:
>> On 18 August 2014 16:30, Stuart Haslam <stuart.haslam@arm.com> wrote:
>> > On Mon, Aug 18, 2014 at 08:41:00AM +0100, Santosh Shukla wrote:
>
>> >
>> > I'm not sure it's worth adding an example application that does nothing.
>> > Isolation is not be something the application needs explicit knowledge
>> > of so having an example app demonstrating it is a bit odd. All of the
>> > existing examples / tests should be capable of being run isolated.
>> >
>> > For this test you could start with odp_example as it has no dependencies
>>
>> Not entirely true and I did thought about using odp_example.
>> odp_example does use timer api (clock_gettime) which I don't want to
>> call from isolated thread. Therefore choose to keep dummy (explicitly
>> mentioned in writeup) thread looping for x time.
>>
>
> OK fair enough, so the odp_example would currently fail this test
> because there's a problem with the linu-generic implementation of the
> timers, this is a known issue and should be fixed at some point.

Keeping known limitation of this approach in mind. I agree :)

>
>> More appropriate test application is l2fwd, where I am doing isolation.
>>
>
> OK, can you send patches for that instead? :)

Yup.. I just finished uploading those patch set to my odp.git
repository link [1]. Primarily that git repo made for demo purpose
however most of em should go for review/feedback.

[1] https://git.linaro.org/people/santosh.shukla/odp.git

[P.S: pl. ignore cruel hack done at top most patch, It was for demo.
also due design limitation in existing global_init_dpdk() function..
Its one of use-case for "global_init" generic argument passing
discussion happened last week, Assuming that Mikey working on it so
didn't paid much attention.]

>
>> There is a readme (in next patch) which very much informs NOT to use
>> odp method of cpuset for thread pinning instead use script.
>
> Yeah I read that - it doesn't say why not though.

Hmm.. I 'll make it more clear in readme -:)
>
>> I could
>> choose to add job enqueue/dequeue in while loop But in next patch.
>
> I don't think that's the right direction to head. Given that any of the
> tests or examples should be capable of running isolated, if we were to
> have a specific isolation test then it would just end up replicating all
> of the other tests. It would be better to just run the existing tests
> on isolated cores. As you say that's not possible at the minute, but it
> should be a goal - and the easiest way to start is by picking an
> existing test that does work on isolated cores.
>

Fair enough. dpdk-s-port-for-odp-l2fwd works!! My scope of work to
make one *existing* application demoable for no_hz_full isolation. As
you said, Yes we should target and introduce isolation specific hints
in framework to make other application work seamlessly.

>> >
>> > I wonder if changing odp_linux_pthread_create to take an array of cores
>> > rather than just the first core is a better way to go. A quick grep
>> > through the existing examples / tests shows almost all callers provide 1
>> > for the number of cores, so it's not currently being used as expected.
>> >
>>
>> Nope. Not in this case, I want to create thread on any of cores based
>> on arg_list, That may be look like this -
>> -l 2,3,5,6 OR -l 3,5,6,7
>>
>> Also changing the order helps me to test my isolation script.
>>
>
> Yes, what I was suggesting would make that easier. Instead of looping
> over num_workers and setting first_core and num for each iteration you
> just pass in an array of core numbers that you want threads started on,
> e.g.;
>
> odp_linux_pthread_create(thread_tbl, num_workers, args.core_num,
>                          run_thread, NULL);
>
> Where args.core_num is just what you currently have as args.core_name
> but stored as an integer.
>
even better. Sorry for confusion.

In summary, Does it make sense to keep this dummy app alive in odp? As
I iterated that Its very handy app for me to test randomize order for
"any core number pinning to any thread" and In future, there might be
possibility [Which Ola mentioned in RT vs Isolation discussion last
week] of one core two threads or many threads. In such case, It make
sense to me to keep this app alive and keep maturing it will
combination of use-cases like 1 cpu :: 2 Threads etc.. Again a test
example for isolation but much handy to test.. no setup dependency
like l2fwd etc..

> --
> Stuart.
>
diff mbox

Patch

diff --git a/configure.ac b/configure.ac
index 74713a7..2b6758b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -142,6 +142,7 @@  AC_CONFIG_FILES([Makefile
 		 example/packet/Makefile
 		 example/packet_netmap/Makefile
 		 example/timer/Makefile
+		 example/isolation/Makefile
 		 test/Makefile
 		 test/api_test/Makefile
 		 pkgconfig/libodp.pc])
diff --git a/example/Makefile.am b/example/Makefile.am
index 01a3305..4cb4a23 100644
--- a/example/Makefile.am
+++ b/example/Makefile.am
@@ -1 +1 @@ 
-SUBDIRS = generator l2fwd odp_example packet packet_netmap timer
+SUBDIRS = generator l2fwd odp_example packet packet_netmap timer isolation
diff --git a/example/isolation/Makefile.am b/example/isolation/Makefile.am
new file mode 100644
index 0000000..7e39773
--- /dev/null
+++ b/example/isolation/Makefile.am
@@ -0,0 +1,5 @@ 
+include $(top_srcdir)/example/Makefile.inc
+
+bin_PROGRAMS = odp_isolation
+
+dist_odp_isolation_SOURCES = odp_isolation.c
diff --git a/example/isolation/odp_isolation.c b/example/isolation/odp_isolation.c
new file mode 100644
index 0000000..8d8fffc
--- /dev/null
+++ b/example/isolation/odp_isolation.c
@@ -0,0 +1,232 @@ 
+/* Copyright (c) 2013, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:     BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * @example  odp_isol_test.c ODP isolation example application
+ */
+
+#include <string.h>
+#include <stdlib.h>
+
+/* ODP main header */
+#include <odp.h>
+
+/* ODP helper for Linux apps */
+#include <helper/odp_linux.h>
+
+/* GNU lib C */
+#include <getopt.h>
+
+#define MAX_WORKERS	32	/**< Max worker threads */
+
+/**
+ * Parsed command line application arguments
+ */
+typedef struct {
+	int core_count; /**< Core count*/
+	char **core_name;	/**< Array of pointers to core name */
+} appl_args_t;
+
+/**
+ * @internal Print help
+ */
+static void print_usage(void)
+{
+	printf("\n\nUsage: ./odp_isolation [options]\n");
+	printf("Options:\n");
+	printf("  -l, --cpulist <cpu number in comma separated fashion>\n");
+	printf("  -h, --help              this help\n");
+	printf("\n\n");
+}
+
+/**
+ * @internal Worker thread
+ *
+ * @param arg  Arguments
+ *
+ * @return NULL on failure
+ */
+static void *run_thread(void *arg)
+{
+	int thr;
+	unsigned long long tick = 0;
+	int counter = 0;
+
+	thr = odp_thread_id();
+
+	printf("Thread %i starts on core %i\n", thr, odp_thread_core());
+
+	/* loop for some duration then exit */
+	do {
+		tick++;
+
+		if (tick > 10000000) {
+
+			tick = 0;
+			counter++;
+		}
+
+		if (counter == 1000)
+			break;
+	} while (1);
+
+	return arg;
+}
+
+/**
+ * @internal Parse arguments
+ *
+ * @param argc  Argument count
+ * @param argv  Argument vector
+ * @param args  Test arguments
+ */
+static void parse_args(int argc, char *argv[], appl_args_t *args)
+{
+	int opt;
+	int long_index;
+	char *names, *str, *token, *save;
+	size_t len;
+	int i;
+
+	static struct option longopts[] = {
+		{"cpulists", required_argument, NULL, 'l'}, /* return 'l' */
+		{"help", no_argument, NULL, 'h'}, /* return 'h' */
+		{NULL, 0, NULL, 0}
+	};
+
+	while (1) {
+		opt = getopt_long(argc, argv, "+l:h", longopts, &long_index);
+
+		if (opt == -1)
+			break;	/* No more options */
+
+		switch (opt) {
+		case 'l':
+			len = strlen(optarg);
+			if (len == 0) {
+				print_usage();
+				exit(EXIT_FAILURE);
+			}
+			len += 1;	/* add room for '\0' */
+
+			names = malloc(len);
+			if (names == NULL) {
+				print_usage();
+				exit(EXIT_FAILURE);
+			}
+
+			/* count the number of tokens separated by ',' */
+			strcpy(names, optarg);
+			for (str = names, i = 0;; str = NULL, i++) {
+				token = strtok_r(str, ",", &save);
+				if (token == NULL)
+					break;
+			}
+			args->core_count = i;
+
+			if (args->core_count == 0) {
+				print_usage();
+				exit(EXIT_FAILURE);
+			}
+			/* allocate storage for the if names */
+			args->core_name =
+				calloc(args->core_count, sizeof(char *));
+
+			/* store the if names (reset names string) */
+			strcpy(names, optarg);
+			for (str = names, i = 0;; str = NULL, i++) {
+				token = strtok_r(str, ",", &save);
+				if (token == NULL)
+					break;
+				args->core_name[i] = token;
+			}
+			break;
+
+		case 'h':
+			print_usage();
+			exit(EXIT_SUCCESS);
+			break;
+
+		default:
+			break;
+		}
+	}
+}
+
+/**
+ * Isolation test application
+ */
+int main(int argc, char *argv[])
+{
+	odp_linux_pthread_t thread_tbl[MAX_WORKERS];
+	appl_args_t args;
+	int thr_id;
+	int num_workers;
+	int first_core;
+	int i;
+
+	printf("\nODP isolation test application\n");
+
+	memset(&args, 0, sizeof(args));
+	parse_args(argc, argv, &args);
+
+	memset(thread_tbl, 0, sizeof(thread_tbl));
+
+	/*
+	 * Don't need timer init for isolation, so mask them.
+	 */
+	if (odp_init_global(ODP_INIT_F_ALL & ~ODP_INIT_F_TIMER)) {
+		printf("ODP global init failed.\n");
+		return -1;
+	}
+
+	printf("\n");
+	printf("ODP system info\n");
+	printf("---------------\n");
+	printf("ODP API version: %s\n",        odp_version_api_str());
+	printf("CPU model:       %s\n",        odp_sys_cpu_model_str());
+	printf("CPU freq (hz):   %"PRIu64"\n", odp_sys_cpu_hz());
+	printf("Cache line size: %i\n",        odp_sys_cache_line_size());
+	printf("Max core count:  %i\n",        odp_sys_core_count());
+
+	printf("\n");
+
+	/* A worker thread per core */
+	num_workers = odp_sys_core_count();
+
+	if (args.core_count)
+		num_workers = args.core_count;
+
+	/* force to max core count */
+	if (num_workers > MAX_WORKERS)
+		num_workers = MAX_WORKERS;
+
+	printf("num worker threads: %i\n", num_workers);
+
+	/*
+	 * Init this thread.
+	 * */
+	thr_id = odp_thread_create(0);
+	odp_init_local(thr_id);
+
+	for (i=0; i<num_workers; i++) {
+
+		first_core = atoi(args.core_name[i]);
+		/* Create and launch worker threads */
+		odp_linux_pthread_create(&thread_tbl[i], 1, first_core,
+					 run_thread, NULL);
+	}
+
+	/* Wait for worker threads to exit */
+	odp_linux_pthread_join(thread_tbl, num_workers);
+
+	printf("ODP isolation test complete\n\n");
+
+	return 0;
+}
+