diff mbox

[dpdk-dev,1/2] Revert "eal: fix default mempool ops"

Message ID 1517514427-28843-1-git-send-email-hemant.agrawal@nxp.com (mailing list archive)
State Superseded, archived
Headers

Checks

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

Commit Message

Hemant Agrawal Feb. 1, 2018, 7:47 p.m. UTC 
  This reverts commit fe06cb6c54fe5ada299ebba40a382bee37c919f2.
---
 lib/librte_eal/bsdapp/eal/eal.c   | 3 ---
 lib/librte_eal/linuxapp/eal/eal.c | 3 ---
 2 files changed, 6 deletions(-)

Comments

Hemant Agrawal Feb. 1, 2018, 7:56 p.m. UTC | #1 
Hi Pavan,
	Your patch was breaking the design of the best_mempool_ops and the whole purpose of selection was getting lost. 
I guess you were trying to fix  test_mempool.  I have sent another patch, which fixes that and start using the best mempool ops API
instead of default mempool ops API.

Regards,
Hemant

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hemant Agrawal
> Sent: Friday, February 02, 2018 1:17 AM
> To: olivier.matz@6wind.com; pbhagavatula@caviumnetworks.com
> Cc: thomas@monjalon.net; dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"
> 
> This reverts commit fe06cb6c54fe5ada299ebba40a382bee37c919f2.
> ---
>  lib/librte_eal/bsdapp/eal/eal.c   | 3 ---
>  lib/librte_eal/linuxapp/eal/eal.c | 3 ---
>  2 files changed, 6 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 1622a41..b612af0 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -86,9 +86,6 @@ int rte_cycles_vmware_tsc_map;  const char *
>  rte_eal_mbuf_default_mempool_ops(void)
>  {
> -	if (internal_config.user_mbuf_pool_ops_name == NULL)
> -		return RTE_MBUF_DEFAULT_MEMPOOL_OPS;
> -
>  	return internal_config.user_mbuf_pool_ops_name;
>  }
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 451fdaf..21024b4 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -96,9 +96,6 @@ int rte_cycles_vmware_tsc_map;  const char *
>  rte_eal_mbuf_default_mempool_ops(void)
>  {
> -	if (internal_config.user_mbuf_pool_ops_name == NULL)
> -		return RTE_MBUF_DEFAULT_MEMPOOL_OPS;
> -
>  	return internal_config.user_mbuf_pool_ops_name;
>  }
> 
> --
> 2.7.4
Pavan Nikhilesh Feb. 1, 2018, 8:40 p.m. UTC | #2 
Hi Hemanth,

	Currently, best_mempool_ops is broken because when
rte_mbuf_user_mempool_ops is invoked it is expected to returns 'NULL' through
internal_config.user_mbuf_pool_ops_name. IMO it is best to create a named
memzone ('mbuf_user_pool_ops') at the end of eal_init and copy mbuf-pool-ops
passed to eal.

`rte_eal_mbuf_default_mempool_ops` is not expected to return 'NULL' would doing
so break the ABI?.

---
/**
 * Get default pool ops name for mbuf
 *
 * @return
 *   returns default pool ops name.
 */
const char *
rte_eal_mbuf_default_mempool_ops(void);
---

IMO creating named mempool at the end of eal_init and changing
`rte_mbuf_user_mempool_ops` as below would be a better solution.

rte_mbuf_user_mempool_ops(void)
{
...
        mz = rte_memzone_lookup("mbuf_user_pool_ops");
        if (mz == NULL)
                return NULL;
	...
}

Thoughts?

Pavan.

On Thu, Feb 01, 2018 at 07:56:47PM +0000, Hemant Agrawal wrote:
> Hi Pavan,
> 	Your patch was breaking the design of the best_mempool_ops and the whole purpose of selection was getting lost.
> I guess you were trying to fix  test_mempool.  I have sent another patch, which fixes that and start using the best mempool ops API
> instead of default mempool ops API.
>
> Regards,
> Hemant
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hemant Agrawal
> > Sent: Friday, February 02, 2018 1:17 AM
> > To: olivier.matz@6wind.com; pbhagavatula@caviumnetworks.com
> > Cc: thomas@monjalon.net; dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"
> >
> > This reverts commit fe06cb6c54fe5ada299ebba40a382bee37c919f2.
> > ---
Hemant Agrawal Feb. 2, 2018, 5:43 a.m. UTC | #3 
HI Pavan,
> 	Currently, best_mempool_ops is broken because when
> rte_mbuf_user_mempool_ops is invoked it is expected to returns 'NULL' through
> internal_config.user_mbuf_pool_ops_name. IMO it is best to create a named
> memzone ('mbuf_user_pool_ops') at the end of eal_init and copy mbuf-pool-ops
> passed to eal.
> 
> `rte_eal_mbuf_default_mempool_ops` is not expected to return 'NULL' would
> doing so break the ABI?.
> 
> ---
> /**
>  * Get default pool ops name for mbuf
>  *
>  * @return
>  *   returns default pool ops name.
>  */
> const char *
> rte_eal_mbuf_default_mempool_ops(void);
> ---
> 
> IMO creating named mempool at the end of eal_init and changing
> `rte_mbuf_user_mempool_ops` as below would be a better solution.
> 
> rte_mbuf_user_mempool_ops(void)
> {
> ...
>         mz = rte_memzone_lookup("mbuf_user_pool_ops");
>         if (mz == NULL)
>                 return NULL;
> 	...
> }
> 
> Thoughts?


[Hemant]  It seems reasonable. We can also deprecate the eal default mempool ops API . I will be sending patch shortly.
 
Unfortunately all NXP platforms are broken at the moment, so we need to get it fixed fast.

Hemant

> 
> Pavan.
> 
> On Thu, Feb 01, 2018 at 07:56:47PM +0000, Hemant Agrawal wrote:
> > Hi Pavan,
> > 	Your patch was breaking the design of the best_mempool_ops and the
> whole purpose of selection was getting lost.
> > I guess you were trying to fix  test_mempool.  I have sent another
> > patch, which fixes that and start using the best mempool ops API instead of
> default mempool ops API.
> >
> > Regards,
> > Hemant
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hemant Agrawal
> > > Sent: Friday, February 02, 2018 1:17 AM
> > > To: olivier.matz@6wind.com; pbhagavatula@caviumnetworks.com
> > > Cc: thomas@monjalon.net; dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"
> > >
> > > This reverts commit fe06cb6c54fe5ada299ebba40a382bee37c919f2.
> > > ---
diff mbox

Patch

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 1622a41..b612af0 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -86,9 +86,6 @@  int rte_cycles_vmware_tsc_map;
 const char *
 rte_eal_mbuf_default_mempool_ops(void)
 {
-	if (internal_config.user_mbuf_pool_ops_name == NULL)
-		return RTE_MBUF_DEFAULT_MEMPOOL_OPS;
-
 	return internal_config.user_mbuf_pool_ops_name;
 }
 
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 451fdaf..21024b4 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -96,9 +96,6 @@  int rte_cycles_vmware_tsc_map;
 const char *
 rte_eal_mbuf_default_mempool_ops(void)
 {
-	if (internal_config.user_mbuf_pool_ops_name == NULL)
-		return RTE_MBUF_DEFAULT_MEMPOOL_OPS;
-
 	return internal_config.user_mbuf_pool_ops_name;
 }