2016-10-13 17:05, Olivier MATZ: > Hi Wei, > > On 10/13/2016 02:31 PM, Ananyev, Konstantin wrote: > > > >> > >>>>> diff --git a/lib/librte_mempool/rte_mempool.c > >>>>> b/lib/librte_mempool/rte_mempool.c > >>>>> index 71017e1..e3e254a 100644 > >>>>> --- a/lib/librte_mempool/rte_mempool.c > >>>>> +++ b/lib/librte_mempool/rte_mempool.c > >>>>> @@ -426,9 +426,12 @@ rte_mempool_populate_phys_tab(struct > >>>>> rte_mempool *mp, char *vaddr, > >>>>> > >>>>> for (i = 0; i < pg_num && mp->populated_size < mp->size; i += > >>>>> n) { > >>>>> > >>>>> + phys_addr_t paddr_next; > >>>>> + paddr_next = paddr[i] + pg_sz; > >>>>> + > >>>>> /* populate with the largest group of contiguous pages > >>>>> */ > >>>>> for (n = 1; (i + n) < pg_num && > >>>>> - paddr[i] + pg_sz == paddr[i+n]; n++) > >>>>> + paddr_next == paddr[i+n]; n++, paddr_next > >>>>> += pg_sz) > >>>>> ; > >>>> > >>>> Good catch. > >>>> Why not just paddr[i + n - 1] != paddr[i + n]? > >>> > >>> Sorry, I meant 'paddr[i + n - 1] + pg_sz == paddr[i+n]' off course. > >>> > >>>> Then you don't need extra variable (paddr_next) here. > >>>> Konstantin > >> > >> Thank you, Konstantin > >> 'paddr[i + n - 1] + pg_sz = paddr[i + n]' also can fix it and have > >> straight meaning. > >> But I assume that my revision with paddr_next += pg_sz may have a bit > >> better performance. > > > > I don't think there would be any real difference, again it is not > > performance critical code-path. > > > >> By the way, paddr[i] + n * pg_sz = paddr[i + n] can also resolve it. > > > > Yes, that's one seems even better for me - make things more clear. > > Thank you for fixing this. > > My vote would go for "addr[i + n - 1] + pg_sz == paddr[i + n]" > > If you feel "paddr[i] + n * pg_sz = paddr[i + n]" is clearer, I have no > problem with it either.
No answer from Wei Dai. Please Olivier advise what to do with this patch. Thanks