| Message ID | 152609036246.121661.18385114679529023835.stgit@localhost.localdomain (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
| ci/Intel-compilation | success | Compilation OK |
12/05/2018 03:59, Andy Green: > /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: > In function '__rte_ring_move_prod_head': > /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:76:3: > warning: ISO C90 forbids mixed declarations and code > [-Wdeclaration-after-statement] > const uint32_t cons_tail = r->cons.tail; > ^~~~~ > /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: > In function '__rte_ring_move_cons_head': > /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:147:3: > warning: ISO C90 forbids mixed declarations and code > [-Wdeclaration-after-statement] > const uint32_t prod_tail = r->prod.tail; The fix line is: Fixes: 0dfc98c507b1 ("ring: separate out head index manipulation") But I wonder why it was done like this. Maybe there is a hidden reason. Bruce? Olivier? > Signed-off-by: Andy Green <andy@warmcat.com> [...] > --- a/lib/librte_ring/rte_ring_generic.h > +++ b/lib/librte_ring/rte_ring_generic.h > @@ -73,14 +73,13 @@ __rte_ring_move_prod_head(struct rte_ring *r, int is_sp, > */ > rte_smp_rmb(); > > - const uint32_t cons_tail = r->cons.tail; > /* > * The subtraction is done between two unsigned 32bits value > * (the result is always modulo 32 bits even if we have > * *old_head > cons_tail). So 'free_entries' is always between 0 > * and capacity (which is < size). > */ > - *free_entries = (capacity + cons_tail - *old_head); > + *free_entries = (capacity + r->cons.tail - *old_head);
On 05/14/2018 12:47 AM, Thomas Monjalon wrote: > 12/05/2018 03:59, Andy Green: >> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: >> In function '__rte_ring_move_prod_head': >> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:76:3: >> warning: ISO C90 forbids mixed declarations and code >> [-Wdeclaration-after-statement] >> const uint32_t cons_tail = r->cons.tail; >> ^~~~~ >> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: >> In function '__rte_ring_move_cons_head': >> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:147:3: >> warning: ISO C90 forbids mixed declarations and code >> [-Wdeclaration-after-statement] >> const uint32_t prod_tail = r->prod.tail; > > The fix line is: > Fixes: 0dfc98c507b1 ("ring: separate out head index manipulation") OK... people can just use git blame at the time they want to know this though. > But I wonder why it was done like this. Maybe there is a hidden reason. > Bruce? Olivier? It's not crazy to do it for clarity, or when you thought there'd be more uses of the calculation. At any rate, I don't see any openings for it to affect the code changing it. -Andy >> Signed-off-by: Andy Green <andy@warmcat.com> > [...] >> --- a/lib/librte_ring/rte_ring_generic.h >> +++ b/lib/librte_ring/rte_ring_generic.h >> @@ -73,14 +73,13 @@ __rte_ring_move_prod_head(struct rte_ring *r, int is_sp, >> */ >> rte_smp_rmb(); >> >> - const uint32_t cons_tail = r->cons.tail; >> /* >> * The subtraction is done between two unsigned 32bits value >> * (the result is always modulo 32 bits even if we have >> * *old_head > cons_tail). So 'free_entries' is always between 0 >> * and capacity (which is < size). >> */ >> - *free_entries = (capacity + cons_tail - *old_head); >> + *free_entries = (capacity + r->cons.tail - *old_head); > > >
14/05/2018 01:21, Andy Green: > > On 05/14/2018 12:47 AM, Thomas Monjalon wrote: > > 12/05/2018 03:59, Andy Green: > >> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: > >> In function '__rte_ring_move_prod_head': > >> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:76:3: > >> warning: ISO C90 forbids mixed declarations and code > >> [-Wdeclaration-after-statement] > >> const uint32_t cons_tail = r->cons.tail; > >> ^~~~~ > >> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: > >> In function '__rte_ring_move_cons_head': > >> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:147:3: > >> warning: ISO C90 forbids mixed declarations and code > >> [-Wdeclaration-after-statement] > >> const uint32_t prod_tail = r->prod.tail; > > > > The fix line is: > > Fixes: 0dfc98c507b1 ("ring: separate out head index manipulation") > > OK... people can just use git blame at the time they want to know this > though. No, we need it to automatically detect which commits should be backported, and in which branch. We also add Cc: stable@dpdk.org to confirm explicitly that is must be backported.
On 05/14/2018 07:44 AM, Thomas Monjalon wrote: > 14/05/2018 01:21, Andy Green: >> >> On 05/14/2018 12:47 AM, Thomas Monjalon wrote: >>> 12/05/2018 03:59, Andy Green: >>>> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: >>>> In function '__rte_ring_move_prod_head': >>>> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:76:3: >>>> warning: ISO C90 forbids mixed declarations and code >>>> [-Wdeclaration-after-statement] >>>> const uint32_t cons_tail = r->cons.tail; >>>> ^~~~~ >>>> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: >>>> In function '__rte_ring_move_cons_head': >>>> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:147:3: >>>> warning: ISO C90 forbids mixed declarations and code >>>> [-Wdeclaration-after-statement] >>>> const uint32_t prod_tail = r->prod.tail; >>> >>> The fix line is: >>> Fixes: 0dfc98c507b1 ("ring: separate out head index manipulation") >> >> OK... people can just use git blame at the time they want to know this >> though. > > No, we need it to automatically detect which commits should be backported, There is nothing automatic about that flow :-) It's "automatic after every committer did the work manually" you mean. > and in which branch. > > We also add Cc: stable@dpdk.org to confirm explicitly that is must be backported. In other projects this work falls on the maintainer(s). -Andy > >
14/05/2018 02:09, Andy Green: > > On 05/14/2018 07:44 AM, Thomas Monjalon wrote: > > 14/05/2018 01:21, Andy Green: > >> > >> On 05/14/2018 12:47 AM, Thomas Monjalon wrote: > >>> 12/05/2018 03:59, Andy Green: > >>>> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: > >>>> In function '__rte_ring_move_prod_head': > >>>> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:76:3: > >>>> warning: ISO C90 forbids mixed declarations and code > >>>> [-Wdeclaration-after-statement] > >>>> const uint32_t cons_tail = r->cons.tail; > >>>> ^~~~~ > >>>> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: > >>>> In function '__rte_ring_move_cons_head': > >>>> /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:147:3: > >>>> warning: ISO C90 forbids mixed declarations and code > >>>> [-Wdeclaration-after-statement] > >>>> const uint32_t prod_tail = r->prod.tail; > >>> > >>> The fix line is: > >>> Fixes: 0dfc98c507b1 ("ring: separate out head index manipulation") > >> > >> OK... people can just use git blame at the time they want to know this > >> though. > > > > No, we need it to automatically detect which commits should be backported, > > There is nothing automatic about that flow :-) > > It's "automatic after every committer did the work manually" you mean. Yes. In this case, I found it for you. You just need to keep it if doing a new revision. > > and in which branch. > > > > We also add Cc: stable@dpdk.org to confirm explicitly that is must be backported. > > In other projects this work falls on the maintainer(s). In DPDK too, the maintainers often fill this field (I did it for your 8 patches just pushed today). But we prefer when this work is done by the patch author :)
diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h index 5b110425f..c2d482bc9 100644 --- a/lib/librte_ring/rte_ring_generic.h +++ b/lib/librte_ring/rte_ring_generic.h @@ -73,14 +73,13 @@ __rte_ring_move_prod_head(struct rte_ring *r, int is_sp, */ rte_smp_rmb(); - const uint32_t cons_tail = r->cons.tail; /* * The subtraction is done between two unsigned 32bits value * (the result is always modulo 32 bits even if we have * *old_head > cons_tail). So 'free_entries' is always between 0 * and capacity (which is < size). */ - *free_entries = (capacity + cons_tail - *old_head); + *free_entries = (capacity + r->cons.tail - *old_head); /* check that we have enough room in ring */ if (unlikely(n > *free_entries)) @@ -144,13 +143,12 @@ __rte_ring_move_cons_head(struct rte_ring *r, int is_sc, */ rte_smp_rmb(); - const uint32_t prod_tail = r->prod.tail; /* The subtraction is done between two unsigned 32bits value * (the result is always modulo 32 bits even if we have * cons_head > prod_tail). So 'entries' is always between 0 * and size(ring)-1. */ - *entries = (prod_tail - *old_head); + *entries = (r->prod.tail - *old_head); /* Set the actual entries for dequeue */ if (n > *entries)
/projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: In function '__rte_ring_move_prod_head': /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:76:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] const uint32_t cons_tail = r->cons.tail; ^~~~~ /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h: In function '__rte_ring_move_cons_head': /projects/lagopus/src/dpdk/build/include/rte_ring_generic.h:147:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] const uint32_t prod_tail = r->prod.tail; Signed-off-by: Andy Green <andy@warmcat.com> --- lib/librte_ring/rte_ring_generic.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)