Avoid buildenv conditional in thread_option struct
authorDaniel Gollub <daniel.gollub@gmail.com>
Thu, 1 May 2014 15:07:49 +0000 (17:07 +0200)
committerJens Axboe <axboe@fb.com>
Thu, 1 May 2014 17:21:56 +0000 (11:21 -0600)
Managed to run into issues with an external ioengine
which got build with CONFIG_LIBNUMA not defined. Fio
itself got build with CONFIG_LIBNUMA this resulted
in different struct members offsets in the two different
ELF objects. Causing crashes due to invalidate offsets
inside the thread_data structure (e.g. td->io_ops->data).

Ideally all structs which might be used by external
ioengines should be independent of buildenv conditionals
like CONFIG_LIBNUMA or others.

Removed the CONFIG_LIBNUMA in thread_options.h and replaced
the libnuma specific "struct bitmask" members with strings
which hold the option's input value. This should also make
the marshaling/demarshaling in cconv.c easier.
(Note: the NUMA bits are not handled in cconv.c at the
moment. And not part of the thread_options_packed struct)

Signed-off-by: Daniel Gollub <daniel.gollub@gmail.com>
Signed-off-by: Jens Axboe <axboe@fb.com>
backend.c
options.c
thread_options.h

index e0f8aa76f584dc8f978c1f2649fae5292f563718..9deef284e36b81be3012f837b7a3bfdd53d242c8 100644 (file)
--- a/backend.c
+++ b/backend.c
@@ -1339,6 +1339,7 @@ static void *thread_main(void *data)
 #ifdef CONFIG_LIBNUMA
        /* numa node setup */
        if (o->numa_cpumask_set || o->numa_memmask_set) {
+               struct bitmask *mask;
                int ret;
 
                if (numa_available() < 0) {
@@ -1347,7 +1348,9 @@ static void *thread_main(void *data)
                }
 
                if (o->numa_cpumask_set) {
-                       ret = numa_run_on_node_mask(o->numa_cpunodesmask);
+                       mask = numa_parse_nodestring(o->numa_cpunodes);
+                       ret = numa_run_on_node_mask(mask);
+                       numa_free_nodemask(mask);
                        if (ret == -1) {
                                td_verror(td, errno, \
                                        "numa_run_on_node_mask failed\n");
@@ -1357,12 +1360,16 @@ static void *thread_main(void *data)
 
                if (o->numa_memmask_set) {
 
+                       mask = NULL;
+                       if (o->numa_memnodes)
+                               mask = numa_parse_nodestring(o->numa_memnodes);
+
                        switch (o->numa_mem_mode) {
                        case MPOL_INTERLEAVE:
-                               numa_set_interleave_mask(o->numa_memnodesmask);
+                               numa_set_interleave_mask(mask);
                                break;
                        case MPOL_BIND:
-                               numa_set_membind(o->numa_memnodesmask);
+                               numa_set_membind(mask);
                                break;
                        case MPOL_LOCAL:
                                numa_set_localalloc();
@@ -1375,6 +1382,9 @@ static void *thread_main(void *data)
                                break;
                        }
 
+                       if (mask)
+                               numa_free_nodemask(mask);
+
                }
        }
 #endif
index 163c5fc9298595a4a26184371db6c74448244921..9dcb255d58e1e3ee2a8812c2cb58c59e176a3114 100644 (file)
--- a/options.c
+++ b/options.c
@@ -554,6 +554,7 @@ static int str_verify_cpus_allowed_cb(void *data, const char *input)
 static int str_numa_cpunodes_cb(void *data, char *input)
 {
        struct thread_data *td = data;
+       struct bitmask *verify_bitmask;
 
        if (parse_dryrun())
                return 0;
@@ -563,13 +564,15 @@ static int str_numa_cpunodes_cb(void *data, char *input)
         * numa_allocate_nodemask(), so it should be freed by
         * numa_free_nodemask().
         */
-       td->o.numa_cpunodesmask = numa_parse_nodestring(input);
-       if (td->o.numa_cpunodesmask == NULL) {
+       verify_bitmask = numa_parse_nodestring(input);
+       if (verify_bitmask == NULL) {
                log_err("fio: numa_parse_nodestring failed\n");
                td_verror(td, 1, "str_numa_cpunodes_cb");
                return 1;
        }
+       numa_free_nodemask(verify_bitmask);
 
+       td->o.numa_cpunodes = strdup(input);
        td->o.numa_cpumask_set = 1;
        return 0;
 }
@@ -581,6 +584,7 @@ static int str_numa_mpol_cb(void *data, char *input)
                { "default", "prefer", "bind", "interleave", "local", NULL };
        int i;
        char *nodelist;
+       struct bitmask *verify_bitmask;
 
        if (parse_dryrun())
                return 0;
@@ -660,12 +664,15 @@ static int str_numa_mpol_cb(void *data, char *input)
                break;
        case MPOL_INTERLEAVE:
        case MPOL_BIND:
-               td->o.numa_memnodesmask = numa_parse_nodestring(nodelist);
-               if (td->o.numa_memnodesmask == NULL) {
+               verify_bitmask = numa_parse_nodestring(nodelist);
+               if (verify_bitmask == NULL) {
                        log_err("fio: numa_parse_nodestring failed\n");
                        td_verror(td, 1, "str_numa_memnodes_cb");
                        return 1;
                }
+               td->o.numa_memnodes = strdup(nodelist);
+               numa_free_nodemask(verify_bitmask);
+                
                break;
        case MPOL_LOCAL:
        case MPOL_DEFAULT:
index 41b6e54e73726ad03fba363247706cb7f56e4772..57d84dbba3defda3df5ae44fa0880954a3c28a93 100644 (file)
@@ -158,14 +158,12 @@ struct thread_options {
        os_cpu_mask_t verify_cpumask;
        unsigned int verify_cpumask_set;
        unsigned int cpus_allowed_policy;
-#ifdef CONFIG_LIBNUMA
-       struct bitmask *numa_cpunodesmask;
+       char *numa_cpunodes;
        unsigned int numa_cpumask_set;
        unsigned short numa_mem_mode;
        unsigned int numa_mem_prefer_node;
-       struct bitmask *numa_memnodesmask;
+       char *numa_memnodes;
        unsigned int numa_memmask_set;
-#endif
        unsigned int iolog;
        unsigned int rwmixcycle;
        unsigned int rwmix[DDIR_RWDIR_CNT];