Thread (11 messages) 11 messages, 3 authors, 2016-10-04

Re: [PATCH 2/2] mempool: fix comments for no contiguous flag

From: Ferruh Yigit <hidden>
Date: 2016-09-21 16:15:19

Hi Olivier,

On 9/21/2016 4:12 PM, Olivier Matz wrote:
Hi Ferruh,

On 09/20/2016 06:17 PM, Ferruh Yigit wrote:
quoted
Fixes: ce94a51ff05c ("mempool: add flag for removing phys contiguous constraint")

Signed-off-by: Ferruh Yigit <redacted>
---
 lib/librte_mempool/rte_mempool.c | 4 +++-
 lib/librte_mempool/rte_mempool.h | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index e96d14f..d767f39 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -340,7 +340,9 @@ rte_mempool_free_memchunks(struct rte_mempool *mp)
 }
 
 /* Add objects in the pool, using a physically contiguous memory
- * zone. Return the number of objects added, or a negative value
+ * zone if MEMPOOL_F_NO_PHYS_CONTIG flag is not set.
+ *
+ * Return the number of objects added, or a negative value
  * on error.
  */
 int
diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index 6fc331a..291c168 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -798,6 +798,9 @@ rte_mempool_free(struct rte_mempool *mp);
  * Add a virtually and physically contiguous memory chunk in the pool
  * where objects can be instanciated.
  *
+ * mempool flag MEMPOOL_F_NO_PHYS_CONTIG changes behavior of the function
+ * and removes physical contiguous constraint.
+ *
  * @param mp
  *   A pointer to the mempool structure.
  * @param vaddr
Not sure I'm getting why you add these comments, as I don't see any
usage of MEMPOOL_F_NO_PHYS_CONTIG in rte_mempool_populate_phys().

Could you describe what makes you think the API comments are wrong here?
rte_mempool_populate_phys() now can get RTE_BAD_PHYS_ADDR as "paddr"
variable, and when paddr == RTE_BAD_PHYS_ADDR, function no more
quarantines that populated items are physically continuous.

Like how this is used in rte_mempool_populate_phys_tab(), if
MEMPOOL_F_NO_PHYS_CONTIG is set rte_mempool_populate_phys() called for
whole virtually continuous memory (len: pg_num * pg_size), so resulting
items may not be in physically continuous addresses, so function comment
becomes wrong.

Although MEMPOOL_F_NO_PHYS_CONTIG is not used within the
rte_mempool_populate_phys(), as far as I can see flag has a side effect
on the functionality.

And please help on wording if it is not accurate.

Thanks,
Olivier
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help