diff mbox

[PATCHv2] validation: sched: fix out of array reference

Message ID 1464378622-5072-1-git-send-email-maxim.uvarov@linaro.org
State Accepted
Commit 3e7e52c96521358b479e46e566310fefe9861c7f
Headers show

Commit Message

Maxim Uvarov May 27, 2016, 7:50 p.m. UTC 
If MAX_CPUS defined to less then current cpus, then
odp call calculates current available cpus and references
to array on thread creation. Do change to allocate array
dynamically.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 v2: calculate num_workers first, then calloc needed size.

 test/performance/odp_scheduling.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

Comments

Bill Fischofer May 27, 2016, 8:41 p.m. UTC | #1 
v2 works much better :)

On Fri, May 27, 2016 at 2:50 PM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> If MAX_CPUS defined to less then current cpus, then
> odp call calculates current available cpus and references
> to array on thread creation. Do change to allocate array
> dynamically.
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>

Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org>


> ---
>  v2: calculate num_workers first, then calloc needed size.
>
>  test/performance/odp_scheduling.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/test/performance/odp_scheduling.c
> b/test/performance/odp_scheduling.c
> index 1d3bfd1..c575b70 100644
> --- a/test/performance/odp_scheduling.c
> +++ b/test/performance/odp_scheduling.c
> @@ -27,8 +27,6 @@
>  /* GNU lib C */
>  #include <getopt.h>
>
> -
> -#define MAX_WORKERS           32            /**< Max worker threads */
>  #define MSG_POOL_SIZE         (4*1024*1024) /**< Message pool size */
>  #define MAX_ALLOCS            35            /**< Alloc burst size */
>  #define QUEUES_PER_PRIO       64            /**< Queue per priority */
> @@ -775,7 +773,7 @@ static void parse_args(int argc, char *argv[],
> test_args_t *args)
>   */
>  int main(int argc, char *argv[])
>  {
> -       odph_odpthread_t thread_tbl[MAX_WORKERS];
> +       odph_odpthread_t *thread_tbl;
>         test_args_t args;
>         int num_workers;
>         odp_cpumask_t cpumask;
> @@ -795,8 +793,6 @@ int main(int argc, char *argv[])
>         memset(&args, 0, sizeof(args));
>         parse_args(argc, argv, &args);
>
> -       memset(thread_tbl, 0, sizeof(thread_tbl));
> -
>         /* ODP global init */
>         if (odp_init_global(&instance, NULL, NULL)) {
>                 LOG_ERR("ODP global init failed.\n");
> @@ -825,19 +821,20 @@ int main(int argc, char *argv[])
>
>         printf("\n");
>
> -       /* Default to system CPU count unless user specified */
> -       num_workers = MAX_WORKERS;
> -       if (args.cpu_count)
> -               num_workers = args.cpu_count;
> -
>         /* Get default worker cpumask */
> -       num_workers = odp_cpumask_default_worker(&cpumask, num_workers);
> +       num_workers = odp_cpumask_default_worker(&cpumask, args.cpu_count);
>         (void)odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
>
>         printf("num worker threads: %i\n", num_workers);
>         printf("first CPU:          %i\n", odp_cpumask_first(&cpumask));
>         printf("cpu mask:           %s\n", cpumaskstr);
>
> +       thread_tbl = calloc(sizeof(odph_odpthread_t), num_workers);
> +       if (!thread_tbl) {
> +               LOG_ERR("no memory for thread_tbl\n");
> +               return -1;
> +       }
> +
>         /* Test cycle count frequency */
>         test_cpu_freq();
>
> @@ -947,6 +944,7 @@ int main(int argc, char *argv[])
>
>         /* Wait for worker threads to terminate */
>         odph_odpthreads_join(thread_tbl);
> +       free(thread_tbl);
>
>         printf("ODP example complete\n\n");
>
> --
> 2.7.1.250.gff4ea60
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Maxim Uvarov May 30, 2016, 6:45 a.m. UTC | #2 
Merged,
Maxim.

On 05/27/16 23:41, Bill Fischofer wrote:
> v2 works much better :)
>
> On Fri, May 27, 2016 at 2:50 PM, Maxim Uvarov <maxim.uvarov@linaro.org 
> <mailto:maxim.uvarov@linaro.org>> wrote:
>
>     If MAX_CPUS defined to less then current cpus, then
>     odp call calculates current available cpus and references
>     to array on thread creation. Do change to allocate array
>     dynamically.
>
>     Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org
>     <mailto:maxim.uvarov@linaro.org>>
>
>
> Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org 
> <mailto:bill.fischofer@linaro.org>>
>
>     ---
>      v2: calculate num_workers first, then calloc needed size.
>
>      test/performance/odp_scheduling.c | 20 +++++++++-----------
>      1 file changed, 9 insertions(+), 11 deletions(-)
>
>     diff --git a/test/performance/odp_scheduling.c
>     b/test/performance/odp_scheduling.c
>     index 1d3bfd1..c575b70 100644
>     --- a/test/performance/odp_scheduling.c
>     +++ b/test/performance/odp_scheduling.c
>     @@ -27,8 +27,6 @@
>      /* GNU lib C */
>      #include <getopt.h>
>
>     -
>     -#define MAX_WORKERS           32            /**< Max worker
>     threads */
>      #define MSG_POOL_SIZE         (4*1024*1024) /**< Message pool size */
>      #define MAX_ALLOCS            35            /**< Alloc burst size */
>      #define QUEUES_PER_PRIO       64            /**< Queue per
>     priority */
>     @@ -775,7 +773,7 @@ static void parse_args(int argc, char *argv[],
>     test_args_t *args)
>       */
>      int main(int argc, char *argv[])
>      {
>     -       odph_odpthread_t thread_tbl[MAX_WORKERS];
>     +       odph_odpthread_t *thread_tbl;
>             test_args_t args;
>             int num_workers;
>             odp_cpumask_t cpumask;
>     @@ -795,8 +793,6 @@ int main(int argc, char *argv[])
>             memset(&args, 0, sizeof(args));
>             parse_args(argc, argv, &args);
>
>     -       memset(thread_tbl, 0, sizeof(thread_tbl));
>     -
>             /* ODP global init */
>             if (odp_init_global(&instance, NULL, NULL)) {
>                     LOG_ERR("ODP global init failed.\n");
>     @@ -825,19 +821,20 @@ int main(int argc, char *argv[])
>
>             printf("\n");
>
>     -       /* Default to system CPU count unless user specified */
>     -       num_workers = MAX_WORKERS;
>     -       if (args.cpu_count)
>     -               num_workers = args.cpu_count;
>     -
>             /* Get default worker cpumask */
>     -       num_workers = odp_cpumask_default_worker(&cpumask,
>     num_workers);
>     +       num_workers = odp_cpumask_default_worker(&cpumask,
>     args.cpu_count);
>             (void)odp_cpumask_to_str(&cpumask, cpumaskstr,
>     sizeof(cpumaskstr));
>
>             printf("num worker threads: %i\n", num_workers);
>             printf("first CPU:          %i\n",
>     odp_cpumask_first(&cpumask));
>             printf("cpu mask:           %s\n", cpumaskstr);
>
>     +       thread_tbl = calloc(sizeof(odph_odpthread_t), num_workers);
>     +       if (!thread_tbl) {
>     +               LOG_ERR("no memory for thread_tbl\n");
>     +               return -1;
>     +       }
>     +
>             /* Test cycle count frequency */
>             test_cpu_freq();
>
>     @@ -947,6 +944,7 @@ int main(int argc, char *argv[])
>
>             /* Wait for worker threads to terminate */
>             odph_odpthreads_join(thread_tbl);
>     +       free(thread_tbl);
>
>             printf("ODP example complete\n\n");
>
>     --
>     2.7.1.250.gff4ea60
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
diff mbox

Patch

diff --git a/test/performance/odp_scheduling.c b/test/performance/odp_scheduling.c
index 1d3bfd1..c575b70 100644
--- a/test/performance/odp_scheduling.c
+++ b/test/performance/odp_scheduling.c
@@ -27,8 +27,6 @@ 
 /* GNU lib C */
 #include <getopt.h>
 
-
-#define MAX_WORKERS           32            /**< Max worker threads */
 #define MSG_POOL_SIZE         (4*1024*1024) /**< Message pool size */
 #define MAX_ALLOCS            35            /**< Alloc burst size */
 #define QUEUES_PER_PRIO       64            /**< Queue per priority */
@@ -775,7 +773,7 @@  static void parse_args(int argc, char *argv[], test_args_t *args)
  */
 int main(int argc, char *argv[])
 {
-	odph_odpthread_t thread_tbl[MAX_WORKERS];
+	odph_odpthread_t *thread_tbl;
 	test_args_t args;
 	int num_workers;
 	odp_cpumask_t cpumask;
@@ -795,8 +793,6 @@  int main(int argc, char *argv[])
 	memset(&args, 0, sizeof(args));
 	parse_args(argc, argv, &args);
 
-	memset(thread_tbl, 0, sizeof(thread_tbl));
-
 	/* ODP global init */
 	if (odp_init_global(&instance, NULL, NULL)) {
 		LOG_ERR("ODP global init failed.\n");
@@ -825,19 +821,20 @@  int main(int argc, char *argv[])
 
 	printf("\n");
 
-	/* Default to system CPU count unless user specified */
-	num_workers = MAX_WORKERS;
-	if (args.cpu_count)
-		num_workers = args.cpu_count;
-
 	/* Get default worker cpumask */
-	num_workers = odp_cpumask_default_worker(&cpumask, num_workers);
+	num_workers = odp_cpumask_default_worker(&cpumask, args.cpu_count);
 	(void)odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
 
 	printf("num worker threads: %i\n", num_workers);
 	printf("first CPU:          %i\n", odp_cpumask_first(&cpumask));
 	printf("cpu mask:           %s\n", cpumaskstr);
 
+	thread_tbl = calloc(sizeof(odph_odpthread_t), num_workers);
+	if (!thread_tbl) {
+		LOG_ERR("no memory for thread_tbl\n");
+		return -1;
+	}
+
 	/* Test cycle count frequency */
 	test_cpu_freq();
 
@@ -947,6 +944,7 @@  int main(int argc, char *argv[])
 
 	/* Wait for worker threads to terminate */
 	odph_odpthreads_join(thread_tbl);
+	free(thread_tbl);
 
 	printf("ODP example complete\n\n");