[dpdk-dev,v6,8/8] mempool: notify memory area to pool

Message ID 20170907153042.30890-9-santosh.shukla@caviumnetworks.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Santosh Shukla Sept. 7, 2017, 3:30 p.m. UTC
  HW pool manager e.g. Octeontx SoC demands s/w to program start and end
address of pool. Currently, there is no such api in external mempool.
Introducing rte_mempool_ops_register_memory_area api which will let HW(pool
manager) to know when common layer selects hugepage:
For each hugepage - Notify its start/end address to HW pool manager.

Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
v5 --> v6:
- Renamed from rte_mempool_ops_update_range to 
  rte_mempool_ops_register_memory_area (Suggested by Olivier)
- Now renamed api retuns int (Suggestd by Olivier)
- Update API description and Error details very explicitly
  (Suggested by Olivier)
Refer [1]
[1] http://dpdk.org/dev/patchwork/patch/28419/

 lib/librte_mempool/rte_mempool.c           |  5 +++++
 lib/librte_mempool/rte_mempool.h           | 34 ++++++++++++++++++++++++++++++
 lib/librte_mempool/rte_mempool_ops.c       | 14 ++++++++++++
 lib/librte_mempool/rte_mempool_version.map |  1 +
 4 files changed, 54 insertions(+)
  

Comments

Olivier Matz Sept. 25, 2017, 11:41 a.m. UTC | #1
On Thu, Sep 07, 2017 at 09:00:42PM +0530, Santosh Shukla wrote:
> HW pool manager e.g. Octeontx SoC demands s/w to program start and end
> address of pool. Currently, there is no such api in external mempool.
> Introducing rte_mempool_ops_register_memory_area api which will let HW(pool
> manager) to know when common layer selects hugepage:
> For each hugepage - Notify its start/end address to HW pool manager.
> 
> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>
> [...]
>
> +/**
> + * @internal wrapper for mempool_ops register_memory_area callback.
> + * API to notify the mempool handler if a new memory area is added to pool.
> + *

if -> when

> + * Mempool handler usually get notified once for the case of mempool get full
> + * range of memory area. However, if several memory areas exist then mempool
> + * handler gets notified each time.

Not sure I understand this last paragraph.

> + *
> + * @param mp
> + *   Pointer to the memory pool.
> + * @param vaddr
> + *   Pointer to the buffer virtual address
> + * @param paddr
> + *   Pointer to the buffer physical address
> + * @param len
> + *   Pool size

Minor: missing dot at the end

> + * @return
> + *  - 0: Success;
> + *  - ENOTSUP: doesn't support register_memory_area ops (valid error case).

Missing minus before ENOTSUP.
The dot should be a semicolon instead.
  
Santosh Shukla Sept. 25, 2017, 10:18 p.m. UTC | #2
On Monday 25 September 2017 12:41 PM, Olivier MATZ wrote:
> On Thu, Sep 07, 2017 at 09:00:42PM +0530, Santosh Shukla wrote:
>> HW pool manager e.g. Octeontx SoC demands s/w to program start and end
>> address of pool. Currently, there is no such api in external mempool.
>> Introducing rte_mempool_ops_register_memory_area api which will let HW(pool
>> manager) to know when common layer selects hugepage:
>> For each hugepage - Notify its start/end address to HW pool manager.
>>
>> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
>> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>>
>> [...]
>>
>> +/**
>> + * @internal wrapper for mempool_ops register_memory_area callback.
>> + * API to notify the mempool handler if a new memory area is added to pool.
>> + *
> if -> when

ok.

>> + * Mempool handler usually get notified once for the case of mempool get full
>> + * range of memory area. However, if several memory areas exist then mempool
>> + * handler gets notified each time.
> Not sure I understand this last paragraph.

Refer v5 history [1] for same.

[1] http://dpdk.org/dev/patchwork/patch/28419/

there will be a case where mempool handler may have more than one memory example, no-hugepage case.
In that case _register_memory_area() ops will be called for more than once.

In v5, you suggested to mention this case explicitly in api description.

If your not clear with write up then could you propose one and also are you fine
with [8/8] patch beside above note? planning to send v7 by tomorrow, appreciate if you answer question.

>> + *
>> + * @param mp
>> + *   Pointer to the memory pool.
>> + * @param vaddr
>> + *   Pointer to the buffer virtual address
>> + * @param paddr
>> + *   Pointer to the buffer physical address
>> + * @param len
>> + *   Pool size
> Minor: missing dot at the end

ok.

>> + * @return
>> + *  - 0: Success;
>> + *  - ENOTSUP: doesn't support register_memory_area ops (valid error case).
> Missing minus before ENOTSUP.
> The dot should be a semicolon instead.
>
ok.

Thanks.
  
Santosh Shukla Sept. 29, 2017, 4:53 a.m. UTC | #3
Hi Olivier,


On Monday 25 September 2017 11:18 PM, santosh wrote:
> On Monday 25 September 2017 12:41 PM, Olivier MATZ wrote:
>> On Thu, Sep 07, 2017 at 09:00:42PM +0530, Santosh Shukla wrote:
>>> + * Mempool handler usually get notified once for the case of mempool get full
>>> + * range of memory area. However, if several memory areas exist then mempool
>>> + * handler gets notified each time.
>> Not sure I understand this last paragraph.
> Refer v5 history [1] for same.
>
> [1] http://dpdk.org/dev/patchwork/patch/28419/
>
> there will be a case where mempool handler may have more than one memory example, no-hugepage case.
> In that case _register_memory_area() ops will be called for more than once.
>
> In v5, you suggested to mention this case explicitly in api description.
>
> If your not clear with write up then could you propose one and also are you fine
> with [8/8] patch beside above note? planning to send v7 by tomorrow, appreciate if you answer question.

Ping?

IMO, remove above description keep it like:
"API to notify the mempool handler if a new memory area is added to pool." Is it ok with you? Can you pl. confirm, I need to send v7 and we want this series in -rc1, its blocking octeontx mempool and nw driver.. delayed review blocking progress.

>>> + *
>>> + * @param mp
>>> + *   Pointer to the memory pool.
>>> + * @param vaddr
>>> + *   Pointer to the buffer virtual address
>>> + * @param paddr
>>> + *   Pointer to the buffer physical address
>>> + * @param len
>>> + *   Pool size
>> Minor: missing dot at the end
> ok.
>
>>> + * @return
>>> + *  - 0: Success;
>>> + *  - ENOTSUP: doesn't support register_memory_area ops (valid error case).
>> Missing minus before ENOTSUP.
>> The dot should be a semicolon instead.
>>
> ok.
>
> Thanks.
>
  
Olivier Matz Sept. 29, 2017, 8:20 a.m. UTC | #4
On Fri, Sep 29, 2017 at 05:53:43AM +0100, santosh wrote:
> Hi Olivier,
> 
> 
> On Monday 25 September 2017 11:18 PM, santosh wrote:
> > On Monday 25 September 2017 12:41 PM, Olivier MATZ wrote:
> >> On Thu, Sep 07, 2017 at 09:00:42PM +0530, Santosh Shukla wrote:
> >>> + * Mempool handler usually get notified once for the case of mempool get full
> >>> + * range of memory area. However, if several memory areas exist then mempool
> >>> + * handler gets notified each time.
> >> Not sure I understand this last paragraph.
> > Refer v5 history [1] for same.
> >
> > [1] http://dpdk.org/dev/patchwork/patch/28419/
> >
> > there will be a case where mempool handler may have more than one memory example, no-hugepage case.
> > In that case _register_memory_area() ops will be called for more than once.
> >
> > In v5, you suggested to mention this case explicitly in api description.
> >
> > If your not clear with write up then could you propose one and also are you fine
> > with [8/8] patch beside above note? planning to send v7 by tomorrow, appreciate if you answer question.
> 
> Ping?
> 
> IMO, remove above description keep it like:
> "API to notify the mempool handler if a new memory area is added to pool." Is it ok with you? Can you pl. confirm, I need to send v7 and we want this series in -rc1, its blocking octeontx mempool and nw driver.. delayed review blocking progress.

The proposed description is ok.
I have no other comment for the rest of the patch.
  
Santosh Shukla Sept. 29, 2017, 8:25 a.m. UTC | #5
Hi Olivier,


On Friday 29 September 2017 09:20 AM, Olivier MATZ wrote:
> On Fri, Sep 29, 2017 at 05:53:43AM +0100, santosh wrote:
>> Hi Olivier,
>>
>>
>> On Monday 25 September 2017 11:18 PM, santosh wrote:
>>> On Monday 25 September 2017 12:41 PM, Olivier MATZ wrote:
>>>> On Thu, Sep 07, 2017 at 09:00:42PM +0530, Santosh Shukla wrote:
>>>>> + * Mempool handler usually get notified once for the case of mempool get full
>>>>> + * range of memory area. However, if several memory areas exist then mempool
>>>>> + * handler gets notified each time.
>>>> Not sure I understand this last paragraph.
>>> Refer v5 history [1] for same.
>>>
>>> [1] http://dpdk.org/dev/patchwork/patch/28419/
>>>
>>> there will be a case where mempool handler may have more than one memory example, no-hugepage case.
>>> In that case _register_memory_area() ops will be called for more than once.
>>>
>>> In v5, you suggested to mention this case explicitly in api description.
>>>
>>> If your not clear with write up then could you propose one and also are you fine
>>> with [8/8] patch beside above note? planning to send v7 by tomorrow, appreciate if you answer question.
>> Ping?
>>
>> IMO, remove above description keep it like:
>> "API to notify the mempool handler if a new memory area is added to pool." Is it ok with you? Can you pl. confirm, I need to send v7 and we want this series in -rc1, its blocking octeontx mempool and nw driver.. delayed review blocking progress.
> The proposed description is ok.
> I have no other comment for the rest of the patch.
>
Ok, will send v7 with above api description.
Thanks.
  

Patch

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index decdda3a6..842382f58 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -365,6 +365,11 @@  rte_mempool_populate_phys(struct rte_mempool *mp, char *vaddr,
 	struct rte_mempool_memhdr *memhdr;
 	int ret;
 
+	/* update range info to mempool */
+	ret = rte_mempool_ops_register_memory_area(mp, vaddr, paddr, len);
+	if (ret != -ENOTSUP && ret < 0)
+		return ret;
+
 	/* create the internal ring if not already done */
 	if ((mp->flags & MEMPOOL_F_POOL_CREATED) == 0) {
 		ret = rte_mempool_ops_alloc(mp);
diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index 24195dda0..8d0171b54 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -413,6 +413,12 @@  typedef unsigned (*rte_mempool_get_count)(const struct rte_mempool *mp);
 typedef int (*rte_mempool_get_capabilities_t)(const struct rte_mempool *mp,
 		unsigned int *flags);
 
+/**
+ * Notify new memory area to mempool.
+ */
+typedef int (*rte_mempool_ops_register_memory_area_t)
+(const struct rte_mempool *mp, char *vaddr, phys_addr_t paddr, size_t len);
+
 /** Structure defining mempool operations structure */
 struct rte_mempool_ops {
 	char name[RTE_MEMPOOL_OPS_NAMESIZE]; /**< Name of mempool ops struct. */
@@ -425,6 +431,10 @@  struct rte_mempool_ops {
 	 * Get the mempool capabilities
 	 */
 	rte_mempool_get_capabilities_t get_capabilities;
+	/**
+	 * Notify new memory area to mempool
+	 */
+	rte_mempool_ops_register_memory_area_t register_memory_area;
 } __rte_cache_aligned;
 
 #define RTE_MEMPOOL_MAX_OPS_IDX 16  /**< Max registered ops structs */
@@ -552,6 +562,30 @@  rte_mempool_ops_get_count(const struct rte_mempool *mp);
 int
 rte_mempool_ops_get_capabilities(const struct rte_mempool *mp,
 					unsigned int *flags);
+/**
+ * @internal wrapper for mempool_ops register_memory_area callback.
+ * API to notify the mempool handler if a new memory area is added to pool.
+ *
+ * Mempool handler usually get notified once for the case of mempool get full
+ * range of memory area. However, if several memory areas exist then mempool
+ * handler gets notified each time.
+ *
+ * @param mp
+ *   Pointer to the memory pool.
+ * @param vaddr
+ *   Pointer to the buffer virtual address
+ * @param paddr
+ *   Pointer to the buffer physical address
+ * @param len
+ *   Pool size
+ * @return
+ *  - 0: Success;
+ *  - ENOTSUP: doesn't support register_memory_area ops (valid error case).
+ *  - Otherwise, rte_mempool_populate_phys fails thus pool create fails.
+ */
+int
+rte_mempool_ops_register_memory_area(const struct rte_mempool *mp,
+				char *vaddr, phys_addr_t paddr, size_t len);
 
 /**
  * @internal wrapper for mempool_ops free callback.
diff --git a/lib/librte_mempool/rte_mempool_ops.c b/lib/librte_mempool/rte_mempool_ops.c
index f2af5e5bb..a6b5f2002 100644
--- a/lib/librte_mempool/rte_mempool_ops.c
+++ b/lib/librte_mempool/rte_mempool_ops.c
@@ -87,6 +87,7 @@  rte_mempool_register_ops(const struct rte_mempool_ops *h)
 	ops->dequeue = h->dequeue;
 	ops->get_count = h->get_count;
 	ops->get_capabilities = h->get_capabilities;
+	ops->register_memory_area = h->register_memory_area;
 
 	rte_spinlock_unlock(&rte_mempool_ops_table.sl);
 
@@ -138,6 +139,19 @@  rte_mempool_ops_get_capabilities(const struct rte_mempool *mp,
 	return ops->get_capabilities(mp, flags);
 }
 
+/* wrapper to notify new memory area to external mempool */
+int
+rte_mempool_ops_register_memory_area(const struct rte_mempool *mp, char *vaddr,
+					phys_addr_t paddr, size_t len)
+{
+	struct rte_mempool_ops *ops;
+
+	ops = rte_mempool_get_ops(mp->ops_index);
+
+	RTE_FUNC_PTR_OR_ERR_RET(ops->register_memory_area, -ENOTSUP);
+	return ops->register_memory_area(mp, vaddr, paddr, len);
+}
+
 /* sets mempool ops previously registered by rte_mempool_register_ops. */
 int
 rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name,
diff --git a/lib/librte_mempool/rte_mempool_version.map b/lib/librte_mempool/rte_mempool_version.map
index 3c3471507..2663001c3 100644
--- a/lib/librte_mempool/rte_mempool_version.map
+++ b/lib/librte_mempool/rte_mempool_version.map
@@ -46,5 +46,6 @@  DPDK_17.11 {
 	global:
 
 	rte_mempool_ops_get_capabilities;
+	rte_mempool_ops_update_range;
 
 } DPDK_16.07;