From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yongseok Koh Subject: Re: [PATCH v6 44/70] net/mlx5: use virt2memseg instead of iteration Date: Tue, 17 Apr 2018 02:48:47 +0000 Message-ID: <0E4E13D3-8842-444E-BEF0-0D76B2547A57@mellanox.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , Adrien Mazarguil , =?iso-8859-1?Q?N=E9lio_Laranjeiro?= , "keith.wiles@intel.com" , "jianfeng.tan@intel.com" , "andras.kovacs@ericsson.com" , "laszlo.vadkeri@ericsson.com" , "benjamin.walker@intel.com" , "bruce.richardson@intel.com" , Thomas Monjalon , "konstantin.ananyev@intel.com" , "kuralamudhan.ramakrishnan@intel.com" , "louise.m.daly@intel.com" , "pepperjo@japf.ch" , "jerin.jacob@caviumnetworks.com" , "hemant.agrawal@nxp.com" , "olivier.matz@ To: Anatoly Burakov Return-path: Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-eopbgr10045.outbound.protection.outlook.com [40.107.1.45]) by dpdk.org (Postfix) with ESMTP id 85AEFBD2E for ; Tue, 17 Apr 2018 04:48:50 +0200 (CEST) In-Reply-To: Content-Language: en-US Content-ID: <8160B7D5C3E58B4F9885554893DFDDE9@eurprd05.prod.outlook.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > On Apr 11, 2018, at 5:30 AM, Anatoly Burakov = wrote: >=20 > Reduce dependency on internal details of EAL memory subsystem, and > simplify code. >=20 > Signed-off-by: Anatoly Burakov > Tested-by: Santosh Shukla > Tested-by: Hemant Agrawal > Tested-by: Gowrishankar Muthukrishnan > --- > drivers/net/mlx5/mlx5_mr.c | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) >=20 > 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 =3D dev->data->dev_private; > - const struct rte_memseg *ms =3D rte_eal_get_physmem_layout(); > + const struct rte_memseg *ms; > uintptr_t start; > uintptr_t end; > - unsigned int i; > struct mlx5_mr *mr; >=20 > mr =3D rte_zmalloc_socket(__func__, sizeof(*mr), 0, mp->socket_id); > @@ -261,17 +260,15 @@ mlx5_mr_new(struct rte_eth_dev *dev, struct rte_mem= pool *mp) > /* Save original addresses for exact MR lookup. */ > mr->start =3D start; > mr->end =3D end; > + > /* Round start and end to page boundary if found in memory segments. */ > - for (i =3D 0; (i < RTE_MAX_MEMSEG) && (ms[i].addr !=3D NULL); ++i) { > - uintptr_t addr =3D (uintptr_t)ms[i].addr; > - size_t len =3D ms[i].len; > - unsigned int align =3D ms[i].hugepage_sz; > + ms =3D rte_mem_virt2memseg((void *)start); > + if (ms !=3D NULL) > + start =3D RTE_ALIGN_FLOOR(start, ms->hugepage_sz); > + ms =3D rte_mem_virt2memseg((void *)end); > + if (ms !=3D NULL) > + end =3D RTE_ALIGN_CEIL(end, ms->hugepage_sz); It is buggy. The memory region is [start, end), so if the memseg of 'end' i= sn't allocated yet, the returned ms will have zero entries and this will make 'e= nd' 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 =3D rte_mem_virt2memseg((void *)start, NULL); if (ms !=3D NULL) start =3D RTE_ALIGN_FLOOR(start, ms->hugepage_sz); - ms =3D rte_mem_virt2memseg((void *)end, NULL); - if (ms !=3D NULL) - end =3D RTE_ALIGN_CEIL(end, ms->hugepage_sz); + end =3D RTE_ALIGN_CEIL(end, ms->hugepage_sz); DRV_LOG(DEBUG, "port %u mempool %p using start=3D%p end=3D%p size=3D%zu fo= r 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 t= o accommodate your memory hotplug design and I'll send it out soon. Thanks in advance. Yongseok > - if ((start > addr) && (start < addr + len)) > - start =3D RTE_ALIGN_FLOOR(start, align); > - if ((end > addr) && (end < addr + len)) > - end =3D RTE_ALIGN_CEIL(end, align); > - } > DRV_LOG(DEBUG, > "port %u mempool %p using start=3D%p end=3D%p size=3D%zu for memory" > " region", > --=20 > 2.7.4