On 17-Apr-18 3:48 AM, Yongseok Koh wrote:
On Apr 11, 2018, at 5:30 AM, Anatoly Burakov <[email protected]> wrote: Reduce dependency on internal details of EAL memory subsystem, and simplify code. Signed-off-by: Anatoly Burakov <[email protected]> Tested-by: Santosh Shukla <[email protected]> Tested-by: Hemant Agrawal <[email protected]> Tested-by: Gowrishankar Muthukrishnan <[email protected]> --- drivers/net/mlx5/mlx5_mr.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c index 58afeb7..c96e134 100644 --- a/drivers/net/mlx5/mlx5_mr.c +++ b/drivers/net/mlx5/mlx5_mr.c @@ -234,10 +234,9 @@ struct mlx5_mr * mlx5_mr_new(struct rte_eth_dev *dev, struct rte_mempool *mp) { struct priv *priv = dev->data->dev_private; - const struct rte_memseg *ms = rte_eal_get_physmem_layout(); + const struct rte_memseg *ms; uintptr_t start; uintptr_t end; - unsigned int i; struct mlx5_mr *mr; mr = rte_zmalloc_socket(__func__, sizeof(*mr), 0, mp->socket_id); @@ -261,17 +260,15 @@ mlx5_mr_new(struct rte_eth_dev *dev, struct rte_mempool *mp) /* Save original addresses for exact MR lookup. */ mr->start = start; mr->end = end; + /* Round start and end to page boundary if found in memory segments. */ - for (i = 0; (i < RTE_MAX_MEMSEG) && (ms[i].addr != NULL); ++i) { - uintptr_t addr = (uintptr_t)ms[i].addr; - size_t len = ms[i].len; - unsigned int align = ms[i].hugepage_sz; + ms = rte_mem_virt2memseg((void *)start); + if (ms != NULL) + start = RTE_ALIGN_FLOOR(start, ms->hugepage_sz); + ms = rte_mem_virt2memseg((void *)end); + if (ms != NULL) + end = RTE_ALIGN_CEIL(end, ms->hugepage_sz);It is buggy. The memory region is [start, end), so if the memseg of 'end' isn't allocated yet, the returned ms will have zero entries and this will make 'end' zero. Instead, the following will be fine. diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c index fdf7b3e88..39bbe2481 100644 --- a/drivers/net/mlx5/mlx5_mr.c +++ b/drivers/net/mlx5/mlx5_mr.c @@ -265,9 +265,7 @@ mlx5_mr_new(struct rte_eth_dev *dev, struct rte_mempool *mp) ms = rte_mem_virt2memseg((void *)start, NULL); if (ms != NULL) start = RTE_ALIGN_FLOOR(start, ms->hugepage_sz); - ms = rte_mem_virt2memseg((void *)end, NULL); - if (ms != NULL) - end = RTE_ALIGN_CEIL(end, ms->hugepage_sz); + end = RTE_ALIGN_CEIL(end, ms->hugepage_sz); DRV_LOG(DEBUG, "port %u mempool %p using start=%p end=%p size=%zu for memory" Same for mlx4. Please fix both mlx5 and mlx4 so that we can verify the new design. However, this code block will be removed eventually. I've done a patchset to accommodate your memory hotplug design and I'll send it out soon.
Hi, Thanks for raising this. I'll submit a patch shortly.
Thanks in advance. Yongseok- if ((start > addr) && (start < addr + len)) - start = RTE_ALIGN_FLOOR(start, align); - if ((end > addr) && (end < addr + len)) - end = RTE_ALIGN_CEIL(end, align); - } DRV_LOG(DEBUG, "port %u mempool %p using start=%p end=%p size=%zu for memory" " region", -- 2.7.4
-- Thanks, Anatoly
