[2/2] mempool: remove check for bad IOVA when populating

Message ID d541158e2c590c0d3e1a985e2bbea4c236da1f9f.1573739893.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [1/2] mempool: use actual IOVA addresses when populating |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed

Commit Message

Burakov, Anatoly Nov. 14, 2019, 1:58 p.m. UTC
  Currently, mempool will check if IOVA is bad for a segment, and reject
the IOVA if hugepages are also enabled. This check is wrong because now
that we have external memory segments, they are allowed to have their
IOVA's to be invalid. This check also doesn't make much sense in the
first place, because the following code can handle bad IOVA's perfectly
well (and in fact, this check is not triggering a failure when
--no-huge option is enabled), so there is not much sense to check for
this in the first place.

Fixes: 950e8fb4e194 ("mem: allow registering external memory areas")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    An alternative implementation would be to add a socket ID check to see
    if the memory being allocated from belongs to an external segment.

 lib/librte_mempool/rte_mempool.c | 5 -----
 1 file changed, 5 deletions(-)
  

Comments

Olivier Matz Nov. 15, 2019, 8:48 a.m. UTC | #1
On Thu, Nov 14, 2019 at 01:58:21PM +0000, Anatoly Burakov wrote:
> Currently, mempool will check if IOVA is bad for a segment, and reject
> the IOVA if hugepages are also enabled. This check is wrong because now
> that we have external memory segments, they are allowed to have their
> IOVA's to be invalid. This check also doesn't make much sense in the
> first place, because the following code can handle bad IOVA's perfectly
> well (and in fact, this check is not triggering a failure when
> --no-huge option is enabled), so there is not much sense to check for
> this in the first place.
> 
> Fixes: 950e8fb4e194 ("mem: allow registering external memory areas")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>
  
Li, WenjieX A Nov. 19, 2019, 1:45 a.m. UTC | #2
Tested-by: Chen, BoX C <box.c.chen@intel.com>

> -----Original Message-----
> From: stable [mailto:stable-bounces@dpdk.org] On Behalf Of Anatoly Burakov
> Sent: Thursday, November 14, 2019 9:58 PM
> To: dev@dpdk.org
> Cc: Olivier Matz <olivier.matz@6wind.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>; david.marchand@redhat.com; stable@dpdk.org
> Subject: [dpdk-stable] [PATCH 2/2] mempool: remove check for bad IOVA when
> populating
> 
> Currently, mempool will check if IOVA is bad for a segment, and reject the IOVA
> if hugepages are also enabled. This check is wrong because now that we have
> external memory segments, they are allowed to have their IOVA's to be invalid.
> This check also doesn't make much sense in the first place, because the
> following code can handle bad IOVA's perfectly well (and in fact, this check is
> not triggering a failure when --no-huge option is enabled), so there is not much
> sense to check for this in the first place.
> 
> Fixes: 950e8fb4e194 ("mem: allow registering external memory areas")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 
> Notes:
>     An alternative implementation would be to add a socket ID check to see
>     if the memory being allocated from belongs to an external segment.
> 
>  lib/librte_mempool/rte_mempool.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/lib/librte_mempool/rte_mempool.c
> b/lib/librte_mempool/rte_mempool.c
> index 8da2e471c7..78d8eb941e 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -390,11 +390,6 @@ rte_mempool_populate_virt(struct rte_mempool *mp,
> char *addr,
> 
>  		iova = get_iova(addr + off);
> 
> -		if (iova == RTE_BAD_IOVA && rte_eal_has_hugepages()) {
> -			ret = -EINVAL;
> -			goto fail;
> -		}
> -
>  		/* populate with the largest group of contiguous pages */
>  		for (phys_len = RTE_MIN(
>  			(size_t)(RTE_PTR_ALIGN_CEIL(addr + off + 1, pg_sz) -
> --
> 2.17.1
  

Patch

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index 8da2e471c7..78d8eb941e 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -390,11 +390,6 @@  rte_mempool_populate_virt(struct rte_mempool *mp, char *addr,
 
 		iova = get_iova(addr + off);
 
-		if (iova == RTE_BAD_IOVA && rte_eal_has_hugepages()) {
-			ret = -EINVAL;
-			goto fail;
-		}
-
 		/* populate with the largest group of contiguous pages */
 		for (phys_len = RTE_MIN(
 			(size_t)(RTE_PTR_ALIGN_CEIL(addr + off + 1, pg_sz) -