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 18:08:50 +0000 Message-ID: References: <0E4E13D3-8842-444E-BEF0-0D76B2547A57@mellanox.com> 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: "Burakov, Anatoly" Return-path: Received: from EUR02-VE1-obe.outbound.protection.outlook.com (mail-eopbgr20061.outbound.protection.outlook.com [40.107.2.61]) by dpdk.org (Postfix) with ESMTP id A4112AABA for ; Tue, 17 Apr 2018 20:08:53 +0200 (CEST) In-Reply-To: Content-Language: en-US Content-ID: 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 17, 2018, at 2:03 AM, Burakov, Anatoly = wrote: >=20 > On 17-Apr-18 3:48 AM, Yongseok Koh wrote: >>> 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_m= empool *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= ' 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_memp= ool *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= for memory" >> Same for mlx4. Please fix both mlx5 and mlx4 so that we can verify the n= ew design. >> However, this code block will be removed eventually. I've done a patchse= t to >> accommodate your memory hotplug design and I'll send it out soon. >=20 > Hi, >=20 > Thanks for raising this. I'll submit a patch shortly. I didn't notice that your patchset has been merged. I thought you were to s= end out a new version. Never mind. I'll send out a fix. Thanks, Yongseok >=20 >> 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 >=20 >=20 > --=20 > Thanks, > Anatoly