| Message ID | 1447331786-13461-4-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
Possibly we want to modify our checkpatch or add the BIT macro ? Using patch: lng-odp_PATCHv13_3-9_helper_flag_to_not_link_ring_to_linked_list.mbox Trying to apply patch Patch applied WARNING: line over 80 characters #25: FILE: helper/include/odp/helper/ring.h:159: +#define ODPH_RING_F_SP_ENQ (1 << 0) /* The default enqueue is "single-producer".*/ CHECK: Prefer using the BIT macro #25: FILE: helper/include/odp/helper/ring.h:159: +#define ODPH_RING_F_SP_ENQ (1 << 0) /* The default enqueue is "single-producer".*/ WARNING: line over 80 characters #26: FILE: helper/include/odp/helper/ring.h:160: +#define ODPH_RING_F_SC_DEQ (1 << 1) /* The default dequeue is "single-consumer".*/ CHECK: Prefer using the BIT macro #26: FILE: helper/include/odp/helper/ring.h:160: +#define ODPH_RING_F_SC_DEQ (1 << 1) /* The default dequeue is "single-consumer".*/ CHECK: Prefer using the BIT macro #27: FILE: helper/include/odp/helper/ring.h:161: +#define ODPH_RING_SHM_PROC (1 << 2) /* If set - ring is visible from different CHECK: Prefer using the BIT macro #29: FILE: helper/include/odp/helper/ring.h:163: +#define ODPH_RING_NO_LIST (1 << 3) /* Do not link ring to linked list. */ On 12 November 2015 at 07:36, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Add flag ODPH_RING_NO_LIST to ring to not link it to linked list. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > helper/include/odp/helper/ring.h | 7 ++++--- > helper/ring.c | 3 ++- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/helper/include/odp/helper/ring.h > b/helper/include/odp/helper/ring.h > index 5e640a7..c3c2f6a 100644 > --- a/helper/include/odp/helper/ring.h > +++ b/helper/include/odp/helper/ring.h > @@ -156,10 +156,11 @@ typedef struct odph_ring { > } odph_ring_t; > > > -#define ODPH_RING_F_SP_ENQ 0x0001 /* The default enqueue is > "single-producer".*/ > -#define ODPH_RING_F_SC_DEQ 0x0002 /* The default dequeue is > "single-consumer".*/ > -#define ODPH_RING_SHM_PROC 0x0004 /* If set - ring is visible from > different > +#define ODPH_RING_F_SP_ENQ (1 << 0) /* The default enqueue is > "single-producer".*/ > +#define ODPH_RING_F_SC_DEQ (1 << 1) /* The default dequeue is > "single-consumer".*/ > +#define ODPH_RING_SHM_PROC (1 << 2) /* If set - ring is visible from > different > processes. Default is thread visible. > */ > +#define ODPH_RING_NO_LIST (1 << 3) /* Do not link ring to linked list. */ > #define ODPH_RING_QUOT_EXCEED (1 << 31) /* Quota exceed for burst ops */ > #define ODPH_RING_SZ_MASK (unsigned)(0x0fffffff) /* Ring size mask */ > > diff --git a/helper/ring.c b/helper/ring.c > index 844abf7..e113606 100644 > --- a/helper/ring.c > +++ b/helper/ring.c > @@ -199,7 +199,8 @@ odph_ring_create(const char *name, unsigned count, > unsigned flags) > r->prod.tail = 0; > r->cons.tail = 0; > > - TAILQ_INSERT_TAIL(&odp_ring_list, r, next); > + if (!(flags & ODPH_RING_NO_LIST)) > + TAILQ_INSERT_TAIL(&odp_ring_list, r, next); > } else { > ODPH_ERR("Cannot reserve memory\n"); > } > -- > 1.9.1 > > _______________________________________________ > lng-odp mailing list > 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
On 11/16/2015 19:20, Mike Holmes wrote: > Possibly we want to modify our checkpatch or add the BIT macro ? > we had the same warnings in the past. As I understand usage of BIT macro was introduced in kernel to better virtualization support. Where bits for BIT are different for host and guest. I.e. instead of redefined bits that macro can be redefined. Other code just move from one place to another. I vote to turn off BIT check in checkpatch.pl until we need to implement it. Maxim. > Using patch: > lng-odp_PATCHv13_3-9_helper_flag_to_not_link_ring_to_linked_list.mbox > Trying to apply patch > Patch applied > WARNING: line over 80 characters > #25: FILE: helper/include/odp/helper/ring.h:159: > +#define ODPH_RING_F_SP_ENQ (1 << 0) /* The default enqueue is > "single-producer".*/ > > CHECK: Prefer using the BIT macro > #25: FILE: helper/include/odp/helper/ring.h:159: > +#define ODPH_RING_F_SP_ENQ (1 << 0) /* The default enqueue is > "single-producer".*/ > > WARNING: line over 80 characters > #26: FILE: helper/include/odp/helper/ring.h:160: > +#define ODPH_RING_F_SC_DEQ (1 << 1) /* The default dequeue is > "single-consumer".*/ > > CHECK: Prefer using the BIT macro > #26: FILE: helper/include/odp/helper/ring.h:160: > +#define ODPH_RING_F_SC_DEQ (1 << 1) /* The default dequeue is > "single-consumer".*/ > > CHECK: Prefer using the BIT macro > #27: FILE: helper/include/odp/helper/ring.h:161: > +#define ODPH_RING_SHM_PROC (1 << 2) /* If set - ring is visible from > different > > CHECK: Prefer using the BIT macro > #29: FILE: helper/include/odp/helper/ring.h:163: > +#define ODPH_RING_NO_LIST (1 << 3) /* Do not link ring to linked > list. */ > > > On 12 November 2015 at 07:36, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > Add flag ODPH_RING_NO_LIST to ring to not link it to linked list. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> > --- > helper/include/odp/helper/ring.h | 7 ++++--- > helper/ring.c | 3 ++- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/helper/include/odp/helper/ring.h > b/helper/include/odp/helper/ring.h > index 5e640a7..c3c2f6a 100644 > --- a/helper/include/odp/helper/ring.h > +++ b/helper/include/odp/helper/ring.h > @@ -156,10 +156,11 @@ typedef struct odph_ring { > } odph_ring_t; > > > -#define ODPH_RING_F_SP_ENQ 0x0001 /* The default enqueue is > "single-producer".*/ > -#define ODPH_RING_F_SC_DEQ 0x0002 /* The default dequeue is > "single-consumer".*/ > -#define ODPH_RING_SHM_PROC 0x0004 /* If set - ring is visible > from different > +#define ODPH_RING_F_SP_ENQ (1 << 0) /* The default enqueue is > "single-producer".*/ > +#define ODPH_RING_F_SC_DEQ (1 << 1) /* The default dequeue is > "single-consumer".*/ > +#define ODPH_RING_SHM_PROC (1 << 2) /* If set - ring is visible > from different > processes. Default is thread > visible. */ > +#define ODPH_RING_NO_LIST (1 << 3) /* Do not link ring to linked > list. */ > #define ODPH_RING_QUOT_EXCEED (1 << 31) /* Quota exceed for > burst ops */ > #define ODPH_RING_SZ_MASK (unsigned)(0x0fffffff) /* Ring size > mask */ > > diff --git a/helper/ring.c b/helper/ring.c > index 844abf7..e113606 100644 > --- a/helper/ring.c > +++ b/helper/ring.c > @@ -199,7 +199,8 @@ odph_ring_create(const char *name, unsigned > count, unsigned flags) > r->prod.tail = 0; > r->cons.tail = 0; > > - TAILQ_INSERT_TAIL(&odp_ring_list, r, next); > + if (!(flags & ODPH_RING_NO_LIST)) > + TAILQ_INSERT_TAIL(&odp_ring_list, r, next); > } else { > ODPH_ERR("Cannot reserve memory\n"); > } > -- > 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 >
I concur. We don't want to be duplicating Linux macros in ODP as that invites confusion. While we're at it, part 2 of the v10 TM patch shows another checkpatch anomaly. It doesn't like the __asm__() routines no matter how you space things. First it complains that : needs spaces around it, but if you add them it then complains that they are prohibited. We should probably just ignore anything within an __asm__() argument string since that's inherently platform-dependent. On Mon, Nov 16, 2015 at 12:38 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 11/16/2015 19:20, Mike Holmes wrote: > >> Possibly we want to modify our checkpatch or add the BIT macro ? >> >> > we had the same warnings in the past. As I understand usage of BIT macro > was introduced in kernel to better virtualization support. Where bits for > BIT are > different for host and guest. I.e. instead of redefined bits that macro > can be redefined. > > Other code just move from one place to another. > > I vote to turn off BIT check in checkpatch.pl until we need to implement > it. > > Maxim. > > > Using patch: >> lng-odp_PATCHv13_3-9_helper_flag_to_not_link_ring_to_linked_list.mbox >> Trying to apply patch >> Patch applied >> WARNING: line over 80 characters >> #25: FILE: helper/include/odp/helper/ring.h:159: >> +#define ODPH_RING_F_SP_ENQ (1 << 0) /* The default enqueue is >> "single-producer".*/ >> >> CHECK: Prefer using the BIT macro >> #25: FILE: helper/include/odp/helper/ring.h:159: >> +#define ODPH_RING_F_SP_ENQ (1 << 0) /* The default enqueue is >> "single-producer".*/ >> >> WARNING: line over 80 characters >> #26: FILE: helper/include/odp/helper/ring.h:160: >> +#define ODPH_RING_F_SC_DEQ (1 << 1) /* The default dequeue is >> "single-consumer".*/ >> >> CHECK: Prefer using the BIT macro >> #26: FILE: helper/include/odp/helper/ring.h:160: >> +#define ODPH_RING_F_SC_DEQ (1 << 1) /* The default dequeue is >> "single-consumer".*/ >> >> CHECK: Prefer using the BIT macro >> #27: FILE: helper/include/odp/helper/ring.h:161: >> +#define ODPH_RING_SHM_PROC (1 << 2) /* If set - ring is visible from >> different >> >> CHECK: Prefer using the BIT macro >> #29: FILE: helper/include/odp/helper/ring.h:163: >> +#define ODPH_RING_NO_LIST (1 << 3) /* Do not link ring to linked list. >> */ >> >> >> On 12 November 2015 at 07:36, Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> wrote: >> >> Add flag ODPH_RING_NO_LIST to ring to not link it to linked list. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> >> >> --- >> helper/include/odp/helper/ring.h | 7 ++++--- >> helper/ring.c | 3 ++- >> 2 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/helper/include/odp/helper/ring.h >> b/helper/include/odp/helper/ring.h >> index 5e640a7..c3c2f6a 100644 >> --- a/helper/include/odp/helper/ring.h >> +++ b/helper/include/odp/helper/ring.h >> @@ -156,10 +156,11 @@ typedef struct odph_ring { >> } odph_ring_t; >> >> >> -#define ODPH_RING_F_SP_ENQ 0x0001 /* The default enqueue is >> "single-producer".*/ >> -#define ODPH_RING_F_SC_DEQ 0x0002 /* The default dequeue is >> "single-consumer".*/ >> -#define ODPH_RING_SHM_PROC 0x0004 /* If set - ring is visible >> from different >> +#define ODPH_RING_F_SP_ENQ (1 << 0) /* The default enqueue is >> "single-producer".*/ >> +#define ODPH_RING_F_SC_DEQ (1 << 1) /* The default dequeue is >> "single-consumer".*/ >> +#define ODPH_RING_SHM_PROC (1 << 2) /* If set - ring is visible >> from different >> processes. Default is thread >> visible. */ >> +#define ODPH_RING_NO_LIST (1 << 3) /* Do not link ring to linked >> list. */ >> #define ODPH_RING_QUOT_EXCEED (1 << 31) /* Quota exceed for >> burst ops */ >> #define ODPH_RING_SZ_MASK (unsigned)(0x0fffffff) /* Ring size >> mask */ >> >> diff --git a/helper/ring.c b/helper/ring.c >> index 844abf7..e113606 100644 >> --- a/helper/ring.c >> +++ b/helper/ring.c >> @@ -199,7 +199,8 @@ odph_ring_create(const char *name, unsigned >> count, unsigned flags) >> r->prod.tail = 0; >> r->cons.tail = 0; >> >> - TAILQ_INSERT_TAIL(&odp_ring_list, r, next); >> + if (!(flags & ODPH_RING_NO_LIST)) >> + TAILQ_INSERT_TAIL(&odp_ring_list, r, next); >> } else { >> ODPH_ERR("Cannot reserve memory\n"); >> } >> -- >> 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/helper/include/odp/helper/ring.h b/helper/include/odp/helper/ring.h index 5e640a7..c3c2f6a 100644 --- a/helper/include/odp/helper/ring.h +++ b/helper/include/odp/helper/ring.h @@ -156,10 +156,11 @@ typedef struct odph_ring { } odph_ring_t; -#define ODPH_RING_F_SP_ENQ 0x0001 /* The default enqueue is "single-producer".*/ -#define ODPH_RING_F_SC_DEQ 0x0002 /* The default dequeue is "single-consumer".*/ -#define ODPH_RING_SHM_PROC 0x0004 /* If set - ring is visible from different +#define ODPH_RING_F_SP_ENQ (1 << 0) /* The default enqueue is "single-producer".*/ +#define ODPH_RING_F_SC_DEQ (1 << 1) /* The default dequeue is "single-consumer".*/ +#define ODPH_RING_SHM_PROC (1 << 2) /* If set - ring is visible from different processes. Default is thread visible. */ +#define ODPH_RING_NO_LIST (1 << 3) /* Do not link ring to linked list. */ #define ODPH_RING_QUOT_EXCEED (1 << 31) /* Quota exceed for burst ops */ #define ODPH_RING_SZ_MASK (unsigned)(0x0fffffff) /* Ring size mask */ diff --git a/helper/ring.c b/helper/ring.c index 844abf7..e113606 100644 --- a/helper/ring.c +++ b/helper/ring.c @@ -199,7 +199,8 @@ odph_ring_create(const char *name, unsigned count, unsigned flags) r->prod.tail = 0; r->cons.tail = 0; - TAILQ_INSERT_TAIL(&odp_ring_list, r, next); + if (!(flags & ODPH_RING_NO_LIST)) + TAILQ_INSERT_TAIL(&odp_ring_list, r, next); } else { ODPH_ERR("Cannot reserve memory\n"); }
Add flag ODPH_RING_NO_LIST to ring to not link it to linked list. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- helper/include/odp/helper/ring.h | 7 ++++--- helper/ring.c | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-)