[1/2] Revert "eal: fix default mempool ops"

Message ID 1517514427-28843-1-git-send-email-hemant.agrawal@nxp.com
State New
Headers show
Series
  • [1/2] Revert "eal: fix default mempool ops"
Related show

Commit Message

Hemant Agrawal Feb. 1, 2018, 7:47 p.m.
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(-)

-- 
2.7.4

Comments

Hemant Agrawal Feb. 1, 2018, 7:56 p.m. | #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. | #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. | #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.

> > > ---

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;
 }