| Message ID | 1438331500-32555-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
I like the idea of gathering common code, and I think this is a step in the right direction, but have a reservation. Currently linux-generic is so many things: 1. The public API release 2. The reference platform intended to be simple and easily understood 3. It has ARCH optimised version of some capabilities that complicate debian packaging 4. It is reused with code sharing for at least three other platforms 5. It is the validation test suite. 6. It is the API documentation If we plan to make code reuse of linux-generic a more central role do we need to rethink this heavy overloading of linux-generic to make the roles of the code more obvious at a glance? I think that rather than proliferate a per file structure and end up with the verbose case of odp_x_internal.h again (14 files), this time we don’t follow that pattern with odp_x_common.c we create one common dir to contain all such files and drop the suffix. I think platfomr/linux-generic/common would work and then all such "common" files will gather there. On 31 July 2015 at 04:31, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Move functions which iterates with bit mask and more likely will not > be accelerated by hw to odp_cpumask_common.c. And odp_cpumask.c > will have only platform specific functions. That patch should simplify > portability to other platfroms when odp_cpumask_common.c will be > inherit from linux generic. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > platform/linux-generic/Makefile.am | 1 + > platform/linux-generic/odp_cpumask.c | 189 > ------------------------- > platform/linux-generic/odp_cpumask_common.c | 207 > ++++++++++++++++++++++++++++ > 3 files changed, 208 insertions(+), 189 deletions(-) > create mode 100644 platform/linux-generic/odp_cpumask_common.c > > diff --git a/platform/linux-generic/Makefile.am > b/platform/linux-generic/Makefile.am > index 34ceb20..4aa5504 100644 > --- a/platform/linux-generic/Makefile.am > +++ b/platform/linux-generic/Makefile.am > @@ -134,6 +134,7 @@ __LIB__libodp_la_SOURCES = \ > odp_buffer.c \ > odp_classification.c \ > odp_cpumask.c \ > + odp_cpumask_common.c \ > odp_crypto.c \ > odp_errno.c \ > odp_event.c \ > diff --git a/platform/linux-generic/odp_cpumask.c > b/platform/linux-generic/odp_cpumask.c > index c28153b..c84bb56 100644 > --- a/platform/linux-generic/odp_cpumask.c > +++ b/platform/linux-generic/odp_cpumask.c > @@ -16,195 +16,6 @@ > #include <stdlib.h> > #include <string.h> > > -void odp_cpumask_from_str(odp_cpumask_t *mask, const char *str_in) > -{ > - cpu_set_t cpuset; > - const char *str = str_in; > - const char *p; > - int cpu = 0; > - int len = strlen(str); > - > - CPU_ZERO(&cpuset); > - odp_cpumask_zero(mask); > - > - /* Strip leading "0x"/"0X" if present and verify length */ > - if ((len >= 2) && ((str[1] == 'x') || (str[1] == 'X'))) { > - str += 2; > - len -= 2; > - } > - if (!len) > - return; > - > - /* Walk string from LSB setting cpu bits */ > - for (p = str + len - 1; (len > 0) && (cpu < CPU_SETSIZE); p--, > len--) { > - char c = *p; > - int value; > - int idx; > - > - /* Convert hex nibble, abort when invalid value found */ > - if ((c >= '0') && (c <= '9')) > - value = c - '0'; > - else if ((c >= 'A') && (c <= 'F')) > - value = c - 'A' + 10; > - else if ((c >= 'a') && (c <= 'f')) > - value = c - 'a' + 10; > - else > - return; > - > - /* Walk converted nibble and set bits in mask */ > - for (idx = 0; idx < 4; idx++, cpu++) > - if (value & (1 << idx)) > - CPU_SET(cpu, &cpuset); > - } > - > - /* Copy the computed mask */ > - memcpy(&mask->set, &cpuset, sizeof(cpuset)); > -} > - > -int32_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, int32_t > len) > -{ > - char *p = str; > - int cpu = odp_cpumask_last(mask); > - int nibbles; > - int value; > - > - /* Handle bad string length, need at least 4 chars for "0x0" and > - * terminating null char */ > - if (len < 4) > - return -1; /* Failure */ > - > - /* Handle no CPU found */ > - if (cpu < 0) { > - strcpy(str, "0x0"); > - return strlen(str) + 1; /* Success */ > - } > - /* CPU was found and cpu >= 0 */ > - > - /* Compute number of nibbles in cpumask that have bits set */ > - nibbles = (cpu / 4) + 1; > - > - /* Verify minimum space (account for "0x" and termination) */ > - if (len < (3 + nibbles)) > - return -1; /* Failure */ > - > - /* Prefix */ > - *p++ = '0'; > - *p++ = 'x'; > - > - /* > - * Now we can scan the cpus down to zero and > - * build the string one nibble at a time > - */ > - value = 0; > - do { > - /* Set bit to go into the current nibble */ > - if (CPU_ISSET(cpu, &mask->set)) > - value |= 1 << (cpu % 4); > - > - /* If we are on a nibble boundary flush value to string */ > - if (0 == (cpu % 4)) { > - if (value < 0xA) > - *p++ = '0' + value; > - else > - *p++ = 'A' + value - 0xA; > - value = 0; > - } > - } while (cpu--); > - > - /* Terminate the string */ > - *p++ = 0; > - return p - str; /* Success */ > -} > - > -void odp_cpumask_zero(odp_cpumask_t *mask) > -{ > - CPU_ZERO(&mask->set); > -} > - > -void odp_cpumask_set(odp_cpumask_t *mask, int cpu) > -{ > - CPU_SET(cpu, &mask->set); > -} > - > -void odp_cpumask_setall(odp_cpumask_t *mask) > -{ > - int cpu; > - > - for (cpu = 0; cpu < CPU_SETSIZE; cpu++) > - CPU_SET(cpu, &mask->set); > -} > - > -void odp_cpumask_clr(odp_cpumask_t *mask, int cpu) > -{ > - CPU_CLR(cpu, &mask->set); > -} > - > -int odp_cpumask_isset(const odp_cpumask_t *mask, int cpu) > -{ > - return CPU_ISSET(cpu, &mask->set); > -} > - > -int odp_cpumask_count(const odp_cpumask_t *mask) > -{ > - return CPU_COUNT(&mask->set); > -} > - > -void odp_cpumask_and(odp_cpumask_t *dest, const odp_cpumask_t *src1, > - const odp_cpumask_t *src2) > -{ > - CPU_AND(&dest->set, &src1->set, &src2->set); > -} > - > -void odp_cpumask_or(odp_cpumask_t *dest, const odp_cpumask_t *src1, > - const odp_cpumask_t *src2) > -{ > - CPU_OR(&dest->set, &src1->set, &src2->set); > -} > - > -void odp_cpumask_xor(odp_cpumask_t *dest, const odp_cpumask_t *src1, > - const odp_cpumask_t *src2) > -{ > - CPU_XOR(&dest->set, &src1->set, &src2->set); > -} > - > -int odp_cpumask_equal(const odp_cpumask_t *mask1, > - const odp_cpumask_t *mask2) > -{ > - return CPU_EQUAL(&mask1->set, &mask2->set); > -} > - > -void odp_cpumask_copy(odp_cpumask_t *dest, const odp_cpumask_t *src) > -{ > - memcpy(&dest->set, &src->set, sizeof(src->set)); > -} > - > -int odp_cpumask_first(const odp_cpumask_t *mask) > -{ > - int cpu; > - > - for (cpu = 0; cpu < CPU_SETSIZE; cpu++) > - if (odp_cpumask_isset(mask, cpu)) > - return cpu; > - return -1; > -} > - > -int odp_cpumask_last(const odp_cpumask_t *mask) > -{ > - int cpu; > - > - for (cpu = CPU_SETSIZE - 1; cpu >= 0; cpu--) > - if (odp_cpumask_isset(mask, cpu)) > - return cpu; > - return -1; > -} > - > -int odp_cpumask_next(const odp_cpumask_t *mask, int cpu) > -{ > - for (cpu += 1; cpu < CPU_SETSIZE; cpu++) > - if (odp_cpumask_isset(mask, cpu)) > - return cpu; > - return -1; > -} > > int odp_cpumask_def_worker(odp_cpumask_t *mask, int num) > { > diff --git a/platform/linux-generic/odp_cpumask_common.c > b/platform/linux-generic/odp_cpumask_common.c > new file mode 100644 > index 0000000..b31e1ca > --- /dev/null > +++ b/platform/linux-generic/odp_cpumask_common.c > @@ -0,0 +1,207 @@ > +/* Copyright (c) 2013, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +#ifndef _GNU_SOURCE > +#define _GNU_SOURCE > +#endif > +#include <sched.h> > +#include <pthread.h> > + > +#include <odp/cpumask.h> > +#include <odp_debug_internal.h> > + > +#include <stdlib.h> > +#include <string.h> > + > +void odp_cpumask_from_str(odp_cpumask_t *mask, const char *str_in) > +{ > + cpu_set_t cpuset; > + const char *str = str_in; > + const char *p; > + int cpu = 0; > + int len = strlen(str); > + > + CPU_ZERO(&cpuset); > + odp_cpumask_zero(mask); > + > + /* Strip leading "0x"/"0X" if present and verify length */ > + if ((len >= 2) && ((str[1] == 'x') || (str[1] == 'X'))) { > + str += 2; > + len -= 2; > + } > + if (!len) > + return; > + > + /* Walk string from LSB setting cpu bits */ > + for (p = str + len - 1; (len > 0) && (cpu < CPU_SETSIZE); p--, > len--) { > + char c = *p; > + int value; > + int idx; > + > + /* Convert hex nibble, abort when invalid value found */ > + if ((c >= '0') && (c <= '9')) > + value = c - '0'; > + else if ((c >= 'A') && (c <= 'F')) > + value = c - 'A' + 10; > + else if ((c >= 'a') && (c <= 'f')) > + value = c - 'a' + 10; > + else > + return; > + > + /* Walk converted nibble and set bits in mask */ > + for (idx = 0; idx < 4; idx++, cpu++) > + if (value & (1 << idx)) > + CPU_SET(cpu, &cpuset); > + } > + > + /* Copy the computed mask */ > + memcpy(&mask->set, &cpuset, sizeof(cpuset)); > +} > + > +int32_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, int32_t > len) > +{ > + char *p = str; > + int cpu = odp_cpumask_last(mask); > + int nibbles; > + int value; > + > + /* Handle bad string length, need at least 4 chars for "0x0" and > + * terminating null char */ > + if (len < 4) > + return -1; /* Failure */ > + > + /* Handle no CPU found */ > + if (cpu < 0) { > + strcpy(str, "0x0"); > + return strlen(str) + 1; /* Success */ > + } > + /* CPU was found and cpu >= 0 */ > + > + /* Compute number of nibbles in cpumask that have bits set */ > + nibbles = (cpu / 4) + 1; > + > + /* Verify minimum space (account for "0x" and termination) */ > + if (len < (3 + nibbles)) > + return -1; /* Failure */ > + > + /* Prefix */ > + *p++ = '0'; > + *p++ = 'x'; > + > + /* > + * Now we can scan the cpus down to zero and > + * build the string one nibble at a time > + */ > + value = 0; > + do { > + /* Set bit to go into the current nibble */ > + if (CPU_ISSET(cpu, &mask->set)) > + value |= 1 << (cpu % 4); > + > + /* If we are on a nibble boundary flush value to string */ > + if (0 == (cpu % 4)) { > + if (value < 0xA) > + *p++ = '0' + value; > + else > + *p++ = 'A' + value - 0xA; > + value = 0; > + } > + } while (cpu--); > + > + /* Terminate the string */ > + *p++ = 0; > + return p - str; /* Success */ > +} > + > +void odp_cpumask_zero(odp_cpumask_t *mask) > +{ > + CPU_ZERO(&mask->set); > +} > + > +void odp_cpumask_set(odp_cpumask_t *mask, int cpu) > +{ > + CPU_SET(cpu, &mask->set); > +} > + > +void odp_cpumask_setall(odp_cpumask_t *mask) > +{ > + int cpu; > + > + for (cpu = 0; cpu < CPU_SETSIZE; cpu++) > + CPU_SET(cpu, &mask->set); > +} > + > +void odp_cpumask_clr(odp_cpumask_t *mask, int cpu) > +{ > + CPU_CLR(cpu, &mask->set); > +} > + > +int odp_cpumask_isset(const odp_cpumask_t *mask, int cpu) > +{ > + return CPU_ISSET(cpu, &mask->set); > +} > + > +int odp_cpumask_count(const odp_cpumask_t *mask) > +{ > + return CPU_COUNT(&mask->set); > +} > + > +void odp_cpumask_and(odp_cpumask_t *dest, const odp_cpumask_t *src1, > + const odp_cpumask_t *src2) > +{ > + CPU_AND(&dest->set, &src1->set, &src2->set); > +} > + > +void odp_cpumask_or(odp_cpumask_t *dest, const odp_cpumask_t *src1, > + const odp_cpumask_t *src2) > +{ > + CPU_OR(&dest->set, &src1->set, &src2->set); > +} > + > +void odp_cpumask_xor(odp_cpumask_t *dest, const odp_cpumask_t *src1, > + const odp_cpumask_t *src2) > +{ > + CPU_XOR(&dest->set, &src1->set, &src2->set); > +} > + > +int odp_cpumask_equal(const odp_cpumask_t *mask1, > + const odp_cpumask_t *mask2) > +{ > + return CPU_EQUAL(&mask1->set, &mask2->set); > +} > + > +void odp_cpumask_copy(odp_cpumask_t *dest, const odp_cpumask_t *src) > +{ > + memcpy(&dest->set, &src->set, sizeof(src->set)); > +} > + > +int odp_cpumask_first(const odp_cpumask_t *mask) > +{ > + int cpu; > + > + for (cpu = 0; cpu < CPU_SETSIZE; cpu++) > + if (odp_cpumask_isset(mask, cpu)) > + return cpu; > + return -1; > +} > + > +int odp_cpumask_last(const odp_cpumask_t *mask) > +{ > + int cpu; > + > + for (cpu = CPU_SETSIZE - 1; cpu >= 0; cpu--) > + if (odp_cpumask_isset(mask, cpu)) > + return cpu; > + return -1; > +} > + > +int odp_cpumask_next(const odp_cpumask_t *mask, int cpu) > +{ > + for (cpu += 1; cpu < CPU_SETSIZE; cpu++) > + if (odp_cpumask_isset(mask, cpu)) > + return cpu; > + return -1; > +} > -- > 1.9.1 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 07/31/15 15:41, Mike Holmes wrote: > I like the idea of gathering common code, and I think this is a step > in the right direction, but have a reservation. > > Currently linux-generic is so many things: > > 1. The public API release > 2. The reference platform intended to be simple and easily understood > 3. It has ARCH optimised version of some capabilities that complicate > debian packaging for debian I do not agree. The only thing we need is just to place all source to debian package. And not try to select some of them. > 4. It is reused with code sharing for at least three other platforms > 5. It is the validation test suite. > 6. It is the API documentation > > If we plan to make code reuse of linux-generic a more central role do > we need to rethink this heavy overloading of linux-generic to make the > roles of the code more obvious at a glance? > The overloading is good idea from the project start. But it might be not easy to make everyone happy. > I think that rather than proliferate a per file structure and end up > with the verbose case of odp_x_internal.h again (14 files), this time > we don’t follow that pattern with odp_x_common.c we create one common > dir to contain all such files and drop the suffix. I did not select name _internal.c due to inside _internal we have some internal to platform functions. But that is ODP API implementation functions. And I think that this function will not change from platform to platform. And might be right place is platform/linux-common. So I added _common for now as prefix that this part will directly linked in dpdk and others. And this file should be very close to his second part in file system. > > I think platfomr/linux-generic/common would work and then all such > "common" files will gather there. > That will complicate finding. For now we have structure like: platform/linux-generic/<layer>.c or platform/linux-generic/<lgoup>/<layer>*.c I like that code reused from linux-generic because that code tested in many places and if somebody will find a bug he will contribute to linux-generic also. Best regards, Maxim. > > > > On 31 July 2015 at 04:31, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > Move functions which iterates with bit mask and more likely will not > be accelerated by hw to odp_cpumask_common.c. And odp_cpumask.c > will have only platform specific functions. That patch should simplify > portability to other platfroms when odp_cpumask_common.c will be > inherit from linux generic. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> > --- > platform/linux-generic/Makefile.am | 1 + > platform/linux-generic/odp_cpumask.c | 189 > ------------------------- > platform/linux-generic/odp_cpumask_common.c | 207 > ++++++++++++++++++++++++++++ > 3 files changed, 208 insertions(+), 189 deletions(-) > create mode 100644 platform/linux-generic/odp_cpumask_common.c > > diff --git a/platform/linux-generic/Makefile.am > b/platform/linux-generic/Makefile.am > index 34ceb20..4aa5504 100644 > --- a/platform/linux-generic/Makefile.am > +++ b/platform/linux-generic/Makefile.am > @@ -134,6 +134,7 @@ __LIB__libodp_la_SOURCES = \ > odp_buffer.c \ > odp_classification.c \ > odp_cpumask.c \ > + odp_cpumask_common.c \ > odp_crypto.c \ > odp_errno.c \ > odp_event.c \ > diff --git a/platform/linux-generic/odp_cpumask.c > b/platform/linux-generic/odp_cpumask.c > index c28153b..c84bb56 100644 > --- a/platform/linux-generic/odp_cpumask.c > +++ b/platform/linux-generic/odp_cpumask.c > @@ -16,195 +16,6 @@ > #include <stdlib.h> > #include <string.h> > > -void odp_cpumask_from_str(odp_cpumask_t *mask, const char *str_in) > -{ > - cpu_set_t cpuset; > - const char *str = str_in; > - const char *p; > - int cpu = 0; > - int len = strlen(str); > - > - CPU_ZERO(&cpuset); > - odp_cpumask_zero(mask); > - > - /* Strip leading "0x"/"0X" if present and verify length */ > - if ((len >= 2) && ((str[1] == 'x') || (str[1] == 'X'))) { > - str += 2; > - len -= 2; > - } > - if (!len) > - return; > - > - /* Walk string from LSB setting cpu bits */ > - for (p = str + len - 1; (len > 0) && (cpu < CPU_SETSIZE); > p--, len--) { > - char c = *p; > - int value; > - int idx; > - > - /* Convert hex nibble, abort when invalid value > found */ > - if ((c >= '0') && (c <= '9')) > - value = c - '0'; > - else if ((c >= 'A') && (c <= 'F')) > - value = c - 'A' + 10; > - else if ((c >= 'a') && (c <= 'f')) > - value = c - 'a' + 10; > - else > - return; > - > - /* Walk converted nibble and set bits in mask */ > - for (idx = 0; idx < 4; idx++, cpu++) > - if (value & (1 << idx)) > - CPU_SET(cpu, &cpuset); > - } > - > - /* Copy the computed mask */ > - memcpy(&mask->set, &cpuset, sizeof(cpuset)); > -} > - > -int32_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, > int32_t len) > -{ > - char *p = str; > - int cpu = odp_cpumask_last(mask); > - int nibbles; > - int value; > - > - /* Handle bad string length, need at least 4 chars for > "0x0" and > - * terminating null char */ > - if (len < 4) > - return -1; /* Failure */ > - > - /* Handle no CPU found */ > - if (cpu < 0) { > - strcpy(str, "0x0"); > - return strlen(str) + 1; /* Success */ > - } > - /* CPU was found and cpu >= 0 */ > - > - /* Compute number of nibbles in cpumask that have bits set */ > - nibbles = (cpu / 4) + 1; > - > - /* Verify minimum space (account for "0x" and termination) */ > - if (len < (3 + nibbles)) > - return -1; /* Failure */ > - > - /* Prefix */ > - *p++ = '0'; > - *p++ = 'x'; > - > - /* > - * Now we can scan the cpus down to zero and > - * build the string one nibble at a time > - */ > - value = 0; > - do { > - /* Set bit to go into the current nibble */ > - if (CPU_ISSET(cpu, &mask->set)) > - value |= 1 << (cpu % 4); > - > - /* If we are on a nibble boundary flush value to > string */ > - if (0 == (cpu % 4)) { > - if (value < 0xA) > - *p++ = '0' + value; > - else > - *p++ = 'A' + value - 0xA; > - value = 0; > - } > - } while (cpu--); > - > - /* Terminate the string */ > - *p++ = 0; > - return p - str; /* Success */ > -} > - > -void odp_cpumask_zero(odp_cpumask_t *mask) > -{ > - CPU_ZERO(&mask->set); > -} > - > -void odp_cpumask_set(odp_cpumask_t *mask, int cpu) > -{ > - CPU_SET(cpu, &mask->set); > -} > - > -void odp_cpumask_setall(odp_cpumask_t *mask) > -{ > - int cpu; > - > - for (cpu = 0; cpu < CPU_SETSIZE; cpu++) > - CPU_SET(cpu, &mask->set); > -} > - > -void odp_cpumask_clr(odp_cpumask_t *mask, int cpu) > -{ > - CPU_CLR(cpu, &mask->set); > -} > - > -int odp_cpumask_isset(const odp_cpumask_t *mask, int cpu) > -{ > - return CPU_ISSET(cpu, &mask->set); > -} > - > -int odp_cpumask_count(const odp_cpumask_t *mask) > -{ > - return CPU_COUNT(&mask->set); > -} > - > -void odp_cpumask_and(odp_cpumask_t *dest, const odp_cpumask_t *src1, > - const odp_cpumask_t *src2) > -{ > - CPU_AND(&dest->set, &src1->set, &src2->set); > -} > - > -void odp_cpumask_or(odp_cpumask_t *dest, const odp_cpumask_t *src1, > - const odp_cpumask_t *src2) > -{ > - CPU_OR(&dest->set, &src1->set, &src2->set); > -} > - > -void odp_cpumask_xor(odp_cpumask_t *dest, const odp_cpumask_t *src1, > - const odp_cpumask_t *src2) > -{ > - CPU_XOR(&dest->set, &src1->set, &src2->set); > -} > - > -int odp_cpumask_equal(const odp_cpumask_t *mask1, > - const odp_cpumask_t *mask2) > -{ > - return CPU_EQUAL(&mask1->set, &mask2->set); > -} > - > -void odp_cpumask_copy(odp_cpumask_t *dest, const odp_cpumask_t *src) > -{ > - memcpy(&dest->set, &src->set, sizeof(src->set)); > -} > - > -int odp_cpumask_first(const odp_cpumask_t *mask) > -{ > - int cpu; > - > - for (cpu = 0; cpu < CPU_SETSIZE; cpu++) > - if (odp_cpumask_isset(mask, cpu)) > - return cpu; > - return -1; > -} > - > -int odp_cpumask_last(const odp_cpumask_t *mask) > -{ > - int cpu; > - > - for (cpu = CPU_SETSIZE - 1; cpu >= 0; cpu--) > - if (odp_cpumask_isset(mask, cpu)) > - return cpu; > - return -1; > -} > - > -int odp_cpumask_next(const odp_cpumask_t *mask, int cpu) > -{ > - for (cpu += 1; cpu < CPU_SETSIZE; cpu++) > - if (odp_cpumask_isset(mask, cpu)) > - return cpu; > - return -1; > -} > > int odp_cpumask_def_worker(odp_cpumask_t *mask, int num) > { > diff --git a/platform/linux-generic/odp_cpumask_common.c > b/platform/linux-generic/odp_cpumask_common.c > new file mode 100644 > index 0000000..b31e1ca > --- /dev/null > +++ b/platform/linux-generic/odp_cpumask_common.c > @@ -0,0 +1,207 @@ > +/* Copyright (c) 2013, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +#ifndef _GNU_SOURCE > +#define _GNU_SOURCE > +#endif > +#include <sched.h> > +#include <pthread.h> > + > +#include <odp/cpumask.h> > +#include <odp_debug_internal.h> > + > +#include <stdlib.h> > +#include <string.h> > + > +void odp_cpumask_from_str(odp_cpumask_t *mask, const char *str_in) > +{ > + cpu_set_t cpuset; > + const char *str = str_in; > + const char *p; > + int cpu = 0; > + int len = strlen(str); > + > + CPU_ZERO(&cpuset); > + odp_cpumask_zero(mask); > + > + /* Strip leading "0x"/"0X" if present and verify length */ > + if ((len >= 2) && ((str[1] == 'x') || (str[1] == 'X'))) { > + str += 2; > + len -= 2; > + } > + if (!len) > + return; > + > + /* Walk string from LSB setting cpu bits */ > + for (p = str + len - 1; (len > 0) && (cpu < CPU_SETSIZE); > p--, len--) { > + char c = *p; > + int value; > + int idx; > + > + /* Convert hex nibble, abort when invalid value > found */ > + if ((c >= '0') && (c <= '9')) > + value = c - '0'; > + else if ((c >= 'A') && (c <= 'F')) > + value = c - 'A' + 10; > + else if ((c >= 'a') && (c <= 'f')) > + value = c - 'a' + 10; > + else > + return; > + > + /* Walk converted nibble and set bits in mask */ > + for (idx = 0; idx < 4; idx++, cpu++) > + if (value & (1 << idx)) > + CPU_SET(cpu, &cpuset); > + } > + > + /* Copy the computed mask */ > + memcpy(&mask->set, &cpuset, sizeof(cpuset)); > +} > + > +int32_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, > int32_t len) > +{ > + char *p = str; > + int cpu = odp_cpumask_last(mask); > + int nibbles; > + int value; > + > + /* Handle bad string length, need at least 4 chars for > "0x0" and > + * terminating null char */ > + if (len < 4) > + return -1; /* Failure */ > + > + /* Handle no CPU found */ > + if (cpu < 0) { > + strcpy(str, "0x0"); > + return strlen(str) + 1; /* Success */ > + } > + /* CPU was found and cpu >= 0 */ > + > + /* Compute number of nibbles in cpumask that have bits set */ > + nibbles = (cpu / 4) + 1; > + > + /* Verify minimum space (account for "0x" and termination) */ > + if (len < (3 + nibbles)) > + return -1; /* Failure */ > + > + /* Prefix */ > + *p++ = '0'; > + *p++ = 'x'; > + > + /* > + * Now we can scan the cpus down to zero and > + * build the string one nibble at a time > + */ > + value = 0; > + do { > + /* Set bit to go into the current nibble */ > + if (CPU_ISSET(cpu, &mask->set)) > + value |= 1 << (cpu % 4); > + > + /* If we are on a nibble boundary flush value to > string */ > + if (0 == (cpu % 4)) { > + if (value < 0xA) > + *p++ = '0' + value; > + else > + *p++ = 'A' + value - 0xA; > + value = 0; > + } > + } while (cpu--); > + > + /* Terminate the string */ > + *p++ = 0; > + return p - str; /* Success */ > +} > + > +void odp_cpumask_zero(odp_cpumask_t *mask) > +{ > + CPU_ZERO(&mask->set); > +} > + > +void odp_cpumask_set(odp_cpumask_t *mask, int cpu) > +{ > + CPU_SET(cpu, &mask->set); > +} > + > +void odp_cpumask_setall(odp_cpumask_t *mask) > +{ > + int cpu; > + > + for (cpu = 0; cpu < CPU_SETSIZE; cpu++) > + CPU_SET(cpu, &mask->set); > +} > + > +void odp_cpumask_clr(odp_cpumask_t *mask, int cpu) > +{ > + CPU_CLR(cpu, &mask->set); > +} > + > +int odp_cpumask_isset(const odp_cpumask_t *mask, int cpu) > +{ > + return CPU_ISSET(cpu, &mask->set); > +} > + > +int odp_cpumask_count(const odp_cpumask_t *mask) > +{ > + return CPU_COUNT(&mask->set); > +} > + > +void odp_cpumask_and(odp_cpumask_t *dest, const odp_cpumask_t *src1, > + const odp_cpumask_t *src2) > +{ > + CPU_AND(&dest->set, &src1->set, &src2->set); > +} > + > +void odp_cpumask_or(odp_cpumask_t *dest, const odp_cpumask_t *src1, > + const odp_cpumask_t *src2) > +{ > + CPU_OR(&dest->set, &src1->set, &src2->set); > +} > + > +void odp_cpumask_xor(odp_cpumask_t *dest, const odp_cpumask_t *src1, > + const odp_cpumask_t *src2) > +{ > + CPU_XOR(&dest->set, &src1->set, &src2->set); > +} > + > +int odp_cpumask_equal(const odp_cpumask_t *mask1, > + const odp_cpumask_t *mask2) > +{ > + return CPU_EQUAL(&mask1->set, &mask2->set); > +} > + > +void odp_cpumask_copy(odp_cpumask_t *dest, const odp_cpumask_t *src) > +{ > + memcpy(&dest->set, &src->set, sizeof(src->set)); > +} > + > +int odp_cpumask_first(const odp_cpumask_t *mask) > +{ > + int cpu; > + > + for (cpu = 0; cpu < CPU_SETSIZE; cpu++) > + if (odp_cpumask_isset(mask, cpu)) > + return cpu; > + return -1; > +} > + > +int odp_cpumask_last(const odp_cpumask_t *mask) > +{ > + int cpu; > + > + for (cpu = CPU_SETSIZE - 1; cpu >= 0; cpu--) > + if (odp_cpumask_isset(mask, cpu)) > + return cpu; > + return -1; > +} > + > +int odp_cpumask_next(const odp_cpumask_t *mask, int cpu) > +{ > + for (cpu += 1; cpu < CPU_SETSIZE; cpu++) > + if (odp_cpumask_isset(mask, cpu)) > + return cpu; > + return -1; > +} > -- > 1.9.1 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > > > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM SoCs >
Hi,
There is another aspect of this which is a bit unfortunate:
In the validation, what is regarded as common regarding cpumask is what is
common to both cpumasks and threadmask. The file
test/validation/common/mask_common.c contains tests that are common to both
threadmask and cpumask.
In the actual linux-generic implementation, things are less gracefull and
the threadmask side simply calls functions from the cpumask side when
equivalent. Here follows an extract of platform/linux-generic/odp_thrmask.c:
void odp_thrmask_from_str(odp_thrmask_t *mask, const char *str)
{
odp_cpumask_from_str(&mask->m, str);
}
int32_t odp_thrmask_to_str(const odp_thrmask_t *mask, char *str, int32_t
size)
{
return odp_cpumask_to_str(&mask->m, str, size);
}
void odp_thrmask_zero(odp_thrmask_t *mask)
{
odp_cpumask_zero(&mask->m);
}
I am wondering whether the name odp_cpumask_common.c could be misleading as
here it would mean common to different platforms... what happens when we
try to group what is common to cpu and thread masks in a common file in the
linux ODP implementation as well? will it be called mask_common_common?
I feel a bit of the same confusion Mike expressed regarding the
linux-generic implementation... I am not very at ease about this code
reuse... I am wondering whether my view about the validation would not
apply here as well: Shouldn't we have a platform agnostic ODP
implementation (just showing the API and defining things for all platforms)
and then a set of platform implementations (linux being one of them)
picking what they wish from the platform agnostic side... Just an idea. Not
sure about all consequences...Not sure it is the best either...
Christophe.
On 31 July 2015 at 15:04, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> On 07/31/15 15:41, Mike Holmes wrote:
>
>> I like the idea of gathering common code, and I think this is a step in
>> the right direction, but have a reservation.
>>
>> Currently linux-generic is so many things:
>>
>> 1. The public API release
>> 2. The reference platform intended to be simple and easily understood
>> 3. It has ARCH optimised version of some capabilities that complicate
>> debian packaging
>>
>
> for debian I do not agree. The only thing we need is just to place all
> source to debian package. And not try to select some of them.
>
> 4. It is reused with code sharing for at least three other platforms
>> 5. It is the validation test suite.
>> 6. It is the API documentation
>>
>> If we plan to make code reuse of linux-generic a more central role do we
>> need to rethink this heavy overloading of linux-generic to make the roles
>> of the code more obvious at a glance?
>>
>> The overloading is good idea from the project start. But it might be not
> easy to make everyone happy.
>
> I think that rather than proliferate a per file structure and end up with
>> the verbose case of odp_x_internal.h again (14 files), this time we don’t
>> follow that pattern with odp_x_common.c we create one common dir to
>> contain all such files and drop the suffix.
>>
>
> I did not select name _internal.c due to inside _internal we have some
> internal to platform functions. But that is ODP API implementation
> functions. And I think that this function will not change from platform to
> platform. And might be right place is platform/linux-common.
>
> So I added _common for now as prefix that this part will directly linked
> in dpdk and others. And this file should be very close to his second part
> in file system.
>
>
>> I think platfomr/linux-generic/common would work and then all such
>> "common" files will gather there.
>>
>>
> That will complicate finding. For now we have structure like:
> platform/linux-generic/<layer>.c
> or
> platform/linux-generic/<lgoup>/<layer>*.c
>
> I like that code reused from linux-generic because that code tested in
> many places and if somebody will find a bug he will contribute to
> linux-generic also.
>
>
> Best regards,
> Maxim.
>
>>
>>
>>
>> On 31 July 2015 at 04:31, Maxim Uvarov <maxim.uvarov@linaro.org <mailto:
>> maxim.uvarov@linaro.org>> wrote:
>>
>> Move functions which iterates with bit mask and more likely will not
>> be accelerated by hw to odp_cpumask_common.c. And odp_cpumask.c
>> will have only platform specific functions. That patch should simplify
>> portability to other platfroms when odp_cpumask_common.c will be
>> inherit from linux generic.
>>
>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org
>> <mailto:maxim.uvarov@linaro.org>>
>>
>> ---
>> platform/linux-generic/Makefile.am | 1 +
>> platform/linux-generic/odp_cpumask.c | 189
>> -------------------------
>> platform/linux-generic/odp_cpumask_common.c | 207
>> ++++++++++++++++++++++++++++
>> 3 files changed, 208 insertions(+), 189 deletions(-)
>> create mode 100644 platform/linux-generic/odp_cpumask_common.c
>>
>> diff --git a/platform/linux-generic/Makefile.am
>> b/platform/linux-generic/Makefile.am
>> index 34ceb20..4aa5504 100644
>> --- a/platform/linux-generic/Makefile.am
>> +++ b/platform/linux-generic/Makefile.am
>> @@ -134,6 +134,7 @@ __LIB__libodp_la_SOURCES = \
>> odp_buffer.c \
>> odp_classification.c \
>> odp_cpumask.c \
>> + odp_cpumask_common.c \
>> odp_crypto.c \
>> odp_errno.c \
>> odp_event.c \
>> diff --git a/platform/linux-generic/odp_cpumask.c
>> b/platform/linux-generic/odp_cpumask.c
>> index c28153b..c84bb56 100644
>> --- a/platform/linux-generic/odp_cpumask.c
>> +++ b/platform/linux-generic/odp_cpumask.c
>> @@ -16,195 +16,6 @@
>> #include <stdlib.h>
>> #include <string.h>
>>
>> -void odp_cpumask_from_str(odp_cpumask_t *mask, const char *str_in)
>> -{
>> - cpu_set_t cpuset;
>> - const char *str = str_in;
>> - const char *p;
>> - int cpu = 0;
>> - int len = strlen(str);
>> -
>> - CPU_ZERO(&cpuset);
>> - odp_cpumask_zero(mask);
>> -
>> - /* Strip leading "0x"/"0X" if present and verify length */
>> - if ((len >= 2) && ((str[1] == 'x') || (str[1] == 'X'))) {
>> - str += 2;
>> - len -= 2;
>> - }
>> - if (!len)
>> - return;
>> -
>> - /* Walk string from LSB setting cpu bits */
>> - for (p = str + len - 1; (len > 0) && (cpu < CPU_SETSIZE);
>> p--, len--) {
>> - char c = *p;
>> - int value;
>> - int idx;
>> -
>> - /* Convert hex nibble, abort when invalid value
>> found */
>> - if ((c >= '0') && (c <= '9'))
>> - value = c - '0';
>> - else if ((c >= 'A') && (c <= 'F'))
>> - value = c - 'A' + 10;
>> - else if ((c >= 'a') && (c <= 'f'))
>> - value = c - 'a' + 10;
>> - else
>> - return;
>> -
>> - /* Walk converted nibble and set bits in mask */
>> - for (idx = 0; idx < 4; idx++, cpu++)
>> - if (value & (1 << idx))
>> - CPU_SET(cpu, &cpuset);
>> - }
>> -
>> - /* Copy the computed mask */
>> - memcpy(&mask->set, &cpuset, sizeof(cpuset));
>> -}
>> -
>> -int32_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *str,
>> int32_t len)
>> -{
>> - char *p = str;
>> - int cpu = odp_cpumask_last(mask);
>> - int nibbles;
>> - int value;
>> -
>> - /* Handle bad string length, need at least 4 chars for
>> "0x0" and
>> - * terminating null char */
>> - if (len < 4)
>> - return -1; /* Failure */
>> -
>> - /* Handle no CPU found */
>> - if (cpu < 0) {
>> - strcpy(str, "0x0");
>> - return strlen(str) + 1; /* Success */
>> - }
>> - /* CPU was found and cpu >= 0 */
>> -
>> - /* Compute number of nibbles in cpumask that have bits set */
>> - nibbles = (cpu / 4) + 1;
>> -
>> - /* Verify minimum space (account for "0x" and termination) */
>> - if (len < (3 + nibbles))
>> - return -1; /* Failure */
>> -
>> - /* Prefix */
>> - *p++ = '0';
>> - *p++ = 'x';
>> -
>> - /*
>> - * Now we can scan the cpus down to zero and
>> - * build the string one nibble at a time
>> - */
>> - value = 0;
>> - do {
>> - /* Set bit to go into the current nibble */
>> - if (CPU_ISSET(cpu, &mask->set))
>> - value |= 1 << (cpu % 4);
>> -
>> - /* If we are on a nibble boundary flush value to
>> string */
>> - if (0 == (cpu % 4)) {
>> - if (value < 0xA)
>> - *p++ = '0' + value;
>> - else
>> - *p++ = 'A' + value - 0xA;
>> - value = 0;
>> - }
>> - } while (cpu--);
>> -
>> - /* Terminate the string */
>> - *p++ = 0;
>> - return p - str; /* Success */
>> -}
>> -
>> -void odp_cpumask_zero(odp_cpumask_t *mask)
>> -{
>> - CPU_ZERO(&mask->set);
>> -}
>> -
>> -void odp_cpumask_set(odp_cpumask_t *mask, int cpu)
>> -{
>> - CPU_SET(cpu, &mask->set);
>> -}
>> -
>> -void odp_cpumask_setall(odp_cpumask_t *mask)
>> -{
>> - int cpu;
>> -
>> - for (cpu = 0; cpu < CPU_SETSIZE; cpu++)
>> - CPU_SET(cpu, &mask->set);
>> -}
>> -
>> -void odp_cpumask_clr(odp_cpumask_t *mask, int cpu)
>> -{
>> - CPU_CLR(cpu, &mask->set);
>> -}
>> -
>> -int odp_cpumask_isset(const odp_cpumask_t *mask, int cpu)
>> -{
>> - return CPU_ISSET(cpu, &mask->set);
>> -}
>> -
>> -int odp_cpumask_count(const odp_cpumask_t *mask)
>> -{
>> - return CPU_COUNT(&mask->set);
>> -}
>> -
>> -void odp_cpumask_and(odp_cpumask_t *dest, const odp_cpumask_t *src1,
>> - const odp_cpumask_t *src2)
>> -{
>> - CPU_AND(&dest->set, &src1->set, &src2->set);
>> -}
>> -
>> -void odp_cpumask_or(odp_cpumask_t *dest, const odp_cpumask_t *src1,
>> - const odp_cpumask_t *src2)
>> -{
>> - CPU_OR(&dest->set, &src1->set, &src2->set);
>> -}
>> -
>> -void odp_cpumask_xor(odp_cpumask_t *dest, const odp_cpumask_t *src1,
>> - const odp_cpumask_t *src2)
>> -{
>> - CPU_XOR(&dest->set, &src1->set, &src2->set);
>> -}
>> -
>> -int odp_cpumask_equal(const odp_cpumask_t *mask1,
>> - const odp_cpumask_t *mask2)
>> -{
>> - return CPU_EQUAL(&mask1->set, &mask2->set);
>> -}
>> -
>> -void odp_cpumask_copy(odp_cpumask_t *dest, const odp_cpumask_t *src)
>> -{
>> - memcpy(&dest->set, &src->set, sizeof(src->set));
>> -}
>> -
>> -int odp_cpumask_first(const odp_cpumask_t *mask)
>> -{
>> - int cpu;
>> -
>> - for (cpu = 0; cpu < CPU_SETSIZE; cpu++)
>> - if (odp_cpumask_isset(mask, cpu))
>> - return cpu;
>> - return -1;
>> -}
>> -
>> -int odp_cpumask_last(const odp_cpumask_t *mask)
>> -{
>> - int cpu;
>> -
>> - for (cpu = CPU_SETSIZE - 1; cpu >= 0; cpu--)
>> - if (odp_cpumask_isset(mask, cpu))
>> - return cpu;
>> - return -1;
>> -}
>> -
>> -int odp_cpumask_next(const odp_cpumask_t *mask, int cpu)
>> -{
>> - for (cpu += 1; cpu < CPU_SETSIZE; cpu++)
>> - if (odp_cpumask_isset(mask, cpu))
>> - return cpu;
>> - return -1;
>> -}
>>
>> int odp_cpumask_def_worker(odp_cpumask_t *mask, int num)
>> {
>> diff --git a/platform/linux-generic/odp_cpumask_common.c
>> b/platform/linux-generic/odp_cpumask_common.c
>> new file mode 100644
>> index 0000000..b31e1ca
>> --- /dev/null
>> +++ b/platform/linux-generic/odp_cpumask_common.c
>> @@ -0,0 +1,207 @@
>> +/* Copyright (c) 2013, Linaro Limited
>> + * All rights reserved.
>> + *
>> + * SPDX-License-Identifier: BSD-3-Clause
>> + */
>> +
>> +#ifndef _GNU_SOURCE
>> +#define _GNU_SOURCE
>> +#endif
>> +#include <sched.h>
>> +#include <pthread.h>
>> +
>> +#include <odp/cpumask.h>
>> +#include <odp_debug_internal.h>
>> +
>> +#include <stdlib.h>
>> +#include <string.h>
>> +
>> +void odp_cpumask_from_str(odp_cpumask_t *mask, const char *str_in)
>> +{
>> + cpu_set_t cpuset;
>> + const char *str = str_in;
>> + const char *p;
>> + int cpu = 0;
>> + int len = strlen(str);
>> +
>> + CPU_ZERO(&cpuset);
>> + odp_cpumask_zero(mask);
>> +
>> + /* Strip leading "0x"/"0X" if present and verify length */
>> + if ((len >= 2) && ((str[1] == 'x') || (str[1] == 'X'))) {
>> + str += 2;
>> + len -= 2;
>> + }
>> + if (!len)
>> + return;
>> +
>> + /* Walk string from LSB setting cpu bits */
>> + for (p = str + len - 1; (len > 0) && (cpu < CPU_SETSIZE);
>> p--, len--) {
>> + char c = *p;
>> + int value;
>> + int idx;
>> +
>> + /* Convert hex nibble, abort when invalid value
>> found */
>> + if ((c >= '0') && (c <= '9'))
>> + value = c - '0';
>> + else if ((c >= 'A') && (c <= 'F'))
>> + value = c - 'A' + 10;
>> + else if ((c >= 'a') && (c <= 'f'))
>> + value = c - 'a' + 10;
>> + else
>> + return;
>> +
>> + /* Walk converted nibble and set bits in mask */
>> + for (idx = 0; idx < 4; idx++, cpu++)
>> + if (value & (1 << idx))
>> + CPU_SET(cpu, &cpuset);
>> + }
>> +
>> + /* Copy the computed mask */
>> + memcpy(&mask->set, &cpuset, sizeof(cpuset));
>> +}
>> +
>> +int32_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *str,
>> int32_t len)
>> +{
>> + char *p = str;
>> + int cpu = odp_cpumask_last(mask);
>> + int nibbles;
>> + int value;
>> +
>> + /* Handle bad string length, need at least 4 chars for
>> "0x0" and
>> + * terminating null char */
>> + if (len < 4)
>> + return -1; /* Failure */
>> +
>> + /* Handle no CPU found */
>> + if (cpu < 0) {
>> + strcpy(str, "0x0");
>> + return strlen(str) + 1; /* Success */
>> + }
>> + /* CPU was found and cpu >= 0 */
>> +
>> + /* Compute number of nibbles in cpumask that have bits set */
>> + nibbles = (cpu / 4) + 1;
>> +
>> + /* Verify minimum space (account for "0x" and termination) */
>> + if (len < (3 + nibbles))
>> + return -1; /* Failure */
>> +
>> + /* Prefix */
>> + *p++ = '0';
>> + *p++ = 'x';
>> +
>> + /*
>> + * Now we can scan the cpus down to zero and
>> + * build the string one nibble at a time
>> + */
>> + value = 0;
>> + do {
>> + /* Set bit to go into the current nibble */
>> + if (CPU_ISSET(cpu, &mask->set))
>> + value |= 1 << (cpu % 4);
>> +
>> + /* If we are on a nibble boundary flush value to
>> string */
>> + if (0 == (cpu % 4)) {
>> + if (value < 0xA)
>> + *p++ = '0' + value;
>> + else
>> + *p++ = 'A' + value - 0xA;
>> + value = 0;
>> + }
>> + } while (cpu--);
>> +
>> + /* Terminate the string */
>> + *p++ = 0;
>> + return p - str; /* Success */
>> +}
>> +
>> +void odp_cpumask_zero(odp_cpumask_t *mask)
>> +{
>> + CPU_ZERO(&mask->set);
>> +}
>> +
>> +void odp_cpumask_set(odp_cpumask_t *mask, int cpu)
>> +{
>> + CPU_SET(cpu, &mask->set);
>> +}
>> +
>> +void odp_cpumask_setall(odp_cpumask_t *mask)
>> +{
>> + int cpu;
>> +
>> + for (cpu = 0; cpu < CPU_SETSIZE; cpu++)
>> + CPU_SET(cpu, &mask->set);
>> +}
>> +
>> +void odp_cpumask_clr(odp_cpumask_t *mask, int cpu)
>> +{
>> + CPU_CLR(cpu, &mask->set);
>> +}
>> +
>> +int odp_cpumask_isset(const odp_cpumask_t *mask, int cpu)
>> +{
>> + return CPU_ISSET(cpu, &mask->set);
>> +}
>> +
>> +int odp_cpumask_count(const odp_cpumask_t *mask)
>> +{
>> + return CPU_COUNT(&mask->set);
>> +}
>> +
>> +void odp_cpumask_and(odp_cpumask_t *dest, const odp_cpumask_t *src1,
>> + const odp_cpumask_t *src2)
>> +{
>> + CPU_AND(&dest->set, &src1->set, &src2->set);
>> +}
>> +
>> +void odp_cpumask_or(odp_cpumask_t *dest, const odp_cpumask_t *src1,
>> + const odp_cpumask_t *src2)
>> +{
>> + CPU_OR(&dest->set, &src1->set, &src2->set);
>> +}
>> +
>> +void odp_cpumask_xor(odp_cpumask_t *dest, const odp_cpumask_t *src1,
>> + const odp_cpumask_t *src2)
>> +{
>> + CPU_XOR(&dest->set, &src1->set, &src2->set);
>> +}
>> +
>> +int odp_cpumask_equal(const odp_cpumask_t *mask1,
>> + const odp_cpumask_t *mask2)
>> +{
>> + return CPU_EQUAL(&mask1->set, &mask2->set);
>> +}
>> +
>> +void odp_cpumask_copy(odp_cpumask_t *dest, const odp_cpumask_t *src)
>> +{
>> + memcpy(&dest->set, &src->set, sizeof(src->set));
>> +}
>> +
>> +int odp_cpumask_first(const odp_cpumask_t *mask)
>> +{
>> + int cpu;
>> +
>> + for (cpu = 0; cpu < CPU_SETSIZE; cpu++)
>> + if (odp_cpumask_isset(mask, cpu))
>> + return cpu;
>> + return -1;
>> +}
>> +
>> +int odp_cpumask_last(const odp_cpumask_t *mask)
>> +{
>> + int cpu;
>> +
>> + for (cpu = CPU_SETSIZE - 1; cpu >= 0; cpu--)
>> + if (odp_cpumask_isset(mask, cpu))
>> + return cpu;
>> + return -1;
>> +}
>> +
>> +int odp_cpumask_next(const odp_cpumask_t *mask, int cpu)
>> +{
>> + for (cpu += 1; cpu < CPU_SETSIZE; cpu++)
>> + if (odp_cpumask_isset(mask, cpu))
>> + return cpu;
>> + return -1;
>> +}
>> --
>> 1.9.1
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>>
>>
>>
>> --
>> Mike Holmes
>> Technical Manager - Linaro Networking Group
>> Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM
>> SoCs
>>
>>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index 34ceb20..4aa5504 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -134,6 +134,7 @@ __LIB__libodp_la_SOURCES = \ odp_buffer.c \ odp_classification.c \ odp_cpumask.c \ + odp_cpumask_common.c \ odp_crypto.c \ odp_errno.c \ odp_event.c \ diff --git a/platform/linux-generic/odp_cpumask.c b/platform/linux-generic/odp_cpumask.c index c28153b..c84bb56 100644 --- a/platform/linux-generic/odp_cpumask.c +++ b/platform/linux-generic/odp_cpumask.c @@ -16,195 +16,6 @@ #include <stdlib.h> #include <string.h> -void odp_cpumask_from_str(odp_cpumask_t *mask, const char *str_in) -{ - cpu_set_t cpuset; - const char *str = str_in; - const char *p; - int cpu = 0; - int len = strlen(str); - - CPU_ZERO(&cpuset); - odp_cpumask_zero(mask); - - /* Strip leading "0x"/"0X" if present and verify length */ - if ((len >= 2) && ((str[1] == 'x') || (str[1] == 'X'))) { - str += 2; - len -= 2; - } - if (!len) - return; - - /* Walk string from LSB setting cpu bits */ - for (p = str + len - 1; (len > 0) && (cpu < CPU_SETSIZE); p--, len--) { - char c = *p; - int value; - int idx; - - /* Convert hex nibble, abort when invalid value found */ - if ((c >= '0') && (c <= '9')) - value = c - '0'; - else if ((c >= 'A') && (c <= 'F')) - value = c - 'A' + 10; - else if ((c >= 'a') && (c <= 'f')) - value = c - 'a' + 10; - else - return; - - /* Walk converted nibble and set bits in mask */ - for (idx = 0; idx < 4; idx++, cpu++) - if (value & (1 << idx)) - CPU_SET(cpu, &cpuset); - } - - /* Copy the computed mask */ - memcpy(&mask->set, &cpuset, sizeof(cpuset)); -} - -int32_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, int32_t len) -{ - char *p = str; - int cpu = odp_cpumask_last(mask); - int nibbles; - int value; - - /* Handle bad string length, need at least 4 chars for "0x0" and - * terminating null char */ - if (len < 4) - return -1; /* Failure */ - - /* Handle no CPU found */ - if (cpu < 0) { - strcpy(str, "0x0"); - return strlen(str) + 1; /* Success */ - } - /* CPU was found and cpu >= 0 */ - - /* Compute number of nibbles in cpumask that have bits set */ - nibbles = (cpu / 4) + 1; - - /* Verify minimum space (account for "0x" and termination) */ - if (len < (3 + nibbles)) - return -1; /* Failure */ - - /* Prefix */ - *p++ = '0'; - *p++ = 'x'; - - /* - * Now we can scan the cpus down to zero and - * build the string one nibble at a time - */ - value = 0; - do { - /* Set bit to go into the current nibble */ - if (CPU_ISSET(cpu, &mask->set)) - value |= 1 << (cpu % 4); - - /* If we are on a nibble boundary flush value to string */ - if (0 == (cpu % 4)) { - if (value < 0xA) - *p++ = '0' + value; - else - *p++ = 'A' + value - 0xA; - value = 0; - } - } while (cpu--); - - /* Terminate the string */ - *p++ = 0; - return p - str; /* Success */ -} - -void odp_cpumask_zero(odp_cpumask_t *mask) -{ - CPU_ZERO(&mask->set); -} - -void odp_cpumask_set(odp_cpumask_t *mask, int cpu) -{ - CPU_SET(cpu, &mask->set); -} - -void odp_cpumask_setall(odp_cpumask_t *mask) -{ - int cpu; - - for (cpu = 0; cpu < CPU_SETSIZE; cpu++) - CPU_SET(cpu, &mask->set); -} - -void odp_cpumask_clr(odp_cpumask_t *mask, int cpu) -{ - CPU_CLR(cpu, &mask->set); -} - -int odp_cpumask_isset(const odp_cpumask_t *mask, int cpu) -{ - return CPU_ISSET(cpu, &mask->set); -} - -int odp_cpumask_count(const odp_cpumask_t *mask) -{ - return CPU_COUNT(&mask->set); -} - -void odp_cpumask_and(odp_cpumask_t *dest, const odp_cpumask_t *src1, - const odp_cpumask_t *src2) -{ - CPU_AND(&dest->set, &src1->set, &src2->set); -} - -void odp_cpumask_or(odp_cpumask_t *dest, const odp_cpumask_t *src1, - const odp_cpumask_t *src2) -{ - CPU_OR(&dest->set, &src1->set, &src2->set); -} - -void odp_cpumask_xor(odp_cpumask_t *dest, const odp_cpumask_t *src1, - const odp_cpumask_t *src2) -{ - CPU_XOR(&dest->set, &src1->set, &src2->set); -} - -int odp_cpumask_equal(const odp_cpumask_t *mask1, - const odp_cpumask_t *mask2) -{ - return CPU_EQUAL(&mask1->set, &mask2->set); -} - -void odp_cpumask_copy(odp_cpumask_t *dest, const odp_cpumask_t *src) -{ - memcpy(&dest->set, &src->set, sizeof(src->set)); -} - -int odp_cpumask_first(const odp_cpumask_t *mask) -{ - int cpu; - - for (cpu = 0; cpu < CPU_SETSIZE; cpu++) - if (odp_cpumask_isset(mask, cpu)) - return cpu; - return -1; -} - -int odp_cpumask_last(const odp_cpumask_t *mask) -{ - int cpu; - - for (cpu = CPU_SETSIZE - 1; cpu >= 0; cpu--) - if (odp_cpumask_isset(mask, cpu)) - return cpu; - return -1; -} - -int odp_cpumask_next(const odp_cpumask_t *mask, int cpu) -{ - for (cpu += 1; cpu < CPU_SETSIZE; cpu++) - if (odp_cpumask_isset(mask, cpu)) - return cpu; - return -1; -} int odp_cpumask_def_worker(odp_cpumask_t *mask, int num) { diff --git a/platform/linux-generic/odp_cpumask_common.c b/platform/linux-generic/odp_cpumask_common.c new file mode 100644 index 0000000..b31e1ca --- /dev/null +++ b/platform/linux-generic/odp_cpumask_common.c @@ -0,0 +1,207 @@ +/* Copyright (c) 2013, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif +#include <sched.h> +#include <pthread.h> + +#include <odp/cpumask.h> +#include <odp_debug_internal.h> + +#include <stdlib.h> +#include <string.h> + +void odp_cpumask_from_str(odp_cpumask_t *mask, const char *str_in) +{ + cpu_set_t cpuset; + const char *str = str_in; + const char *p; + int cpu = 0; + int len = strlen(str); + + CPU_ZERO(&cpuset); + odp_cpumask_zero(mask); + + /* Strip leading "0x"/"0X" if present and verify length */ + if ((len >= 2) && ((str[1] == 'x') || (str[1] == 'X'))) { + str += 2; + len -= 2; + } + if (!len) + return; + + /* Walk string from LSB setting cpu bits */ + for (p = str + len - 1; (len > 0) && (cpu < CPU_SETSIZE); p--, len--) { + char c = *p; + int value; + int idx; + + /* Convert hex nibble, abort when invalid value found */ + if ((c >= '0') && (c <= '9')) + value = c - '0'; + else if ((c >= 'A') && (c <= 'F')) + value = c - 'A' + 10; + else if ((c >= 'a') && (c <= 'f')) + value = c - 'a' + 10; + else + return; + + /* Walk converted nibble and set bits in mask */ + for (idx = 0; idx < 4; idx++, cpu++) + if (value & (1 << idx)) + CPU_SET(cpu, &cpuset); + } + + /* Copy the computed mask */ + memcpy(&mask->set, &cpuset, sizeof(cpuset)); +} + +int32_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, int32_t len) +{ + char *p = str; + int cpu = odp_cpumask_last(mask); + int nibbles; + int value; + + /* Handle bad string length, need at least 4 chars for "0x0" and + * terminating null char */ + if (len < 4) + return -1; /* Failure */ + + /* Handle no CPU found */ + if (cpu < 0) { + strcpy(str, "0x0"); + return strlen(str) + 1; /* Success */ + } + /* CPU was found and cpu >= 0 */ + + /* Compute number of nibbles in cpumask that have bits set */ + nibbles = (cpu / 4) + 1; + + /* Verify minimum space (account for "0x" and termination) */ + if (len < (3 + nibbles)) + return -1; /* Failure */ + + /* Prefix */ + *p++ = '0'; + *p++ = 'x'; + + /* + * Now we can scan the cpus down to zero and + * build the string one nibble at a time + */ + value = 0; + do { + /* Set bit to go into the current nibble */ + if (CPU_ISSET(cpu, &mask->set)) + value |= 1 << (cpu % 4); + + /* If we are on a nibble boundary flush value to string */ + if (0 == (cpu % 4)) { + if (value < 0xA) + *p++ = '0' + value; + else + *p++ = 'A' + value - 0xA; + value = 0; + } + } while (cpu--); + + /* Terminate the string */ + *p++ = 0; + return p - str; /* Success */ +} + +void odp_cpumask_zero(odp_cpumask_t *mask) +{ + CPU_ZERO(&mask->set); +} + +void odp_cpumask_set(odp_cpumask_t *mask, int cpu) +{ + CPU_SET(cpu, &mask->set); +} + +void odp_cpumask_setall(odp_cpumask_t *mask) +{ + int cpu; + + for (cpu = 0; cpu < CPU_SETSIZE; cpu++) + CPU_SET(cpu, &mask->set); +} + +void odp_cpumask_clr(odp_cpumask_t *mask, int cpu) +{ + CPU_CLR(cpu, &mask->set); +} + +int odp_cpumask_isset(const odp_cpumask_t *mask, int cpu) +{ + return CPU_ISSET(cpu, &mask->set); +} + +int odp_cpumask_count(const odp_cpumask_t *mask) +{ + return CPU_COUNT(&mask->set); +} + +void odp_cpumask_and(odp_cpumask_t *dest, const odp_cpumask_t *src1, + const odp_cpumask_t *src2) +{ + CPU_AND(&dest->set, &src1->set, &src2->set); +} + +void odp_cpumask_or(odp_cpumask_t *dest, const odp_cpumask_t *src1, + const odp_cpumask_t *src2) +{ + CPU_OR(&dest->set, &src1->set, &src2->set); +} + +void odp_cpumask_xor(odp_cpumask_t *dest, const odp_cpumask_t *src1, + const odp_cpumask_t *src2) +{ + CPU_XOR(&dest->set, &src1->set, &src2->set); +} + +int odp_cpumask_equal(const odp_cpumask_t *mask1, + const odp_cpumask_t *mask2) +{ + return CPU_EQUAL(&mask1->set, &mask2->set); +} + +void odp_cpumask_copy(odp_cpumask_t *dest, const odp_cpumask_t *src) +{ + memcpy(&dest->set, &src->set, sizeof(src->set)); +} + +int odp_cpumask_first(const odp_cpumask_t *mask) +{ + int cpu; + + for (cpu = 0; cpu < CPU_SETSIZE; cpu++) + if (odp_cpumask_isset(mask, cpu)) + return cpu; + return -1; +} + +int odp_cpumask_last(const odp_cpumask_t *mask) +{ + int cpu; + + for (cpu = CPU_SETSIZE - 1; cpu >= 0; cpu--) + if (odp_cpumask_isset(mask, cpu)) + return cpu; + return -1; +} + +int odp_cpumask_next(const odp_cpumask_t *mask, int cpu) +{ + for (cpu += 1; cpu < CPU_SETSIZE; cpu++) + if (odp_cpumask_isset(mask, cpu)) + return cpu; + return -1; +}
Move functions which iterates with bit mask and more likely will not be accelerated by hw to odp_cpumask_common.c. And odp_cpumask.c will have only platform specific functions. That patch should simplify portability to other platfroms when odp_cpumask_common.c will be inherit from linux generic. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- platform/linux-generic/Makefile.am | 1 + platform/linux-generic/odp_cpumask.c | 189 ------------------------- platform/linux-generic/odp_cpumask_common.c | 207 ++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 189 deletions(-) create mode 100644 platform/linux-generic/odp_cpumask_common.c