Re: [dpdk-dev] [PATCH v6 44/70] net/mlx5: use virt2memseg instead of iteration
> On Apr 17, 2018, at 2:03 AM, Burakov, Anatoly
> wrote:
>
> On 17-Apr-18 3:48 AM, Yongseok Koh wrote:
>>> On Apr 11, 2018, at 5:30 AM, Anatoly Burakov
>>> wrote:
>>>
>>> Reduce dependency on internal details of EAL memory subsystem, and
>>> simplify code.
>>>
>>> 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(-)
>>>
>>> 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.
I didn't notice that your patchset has been merged. I thought you were to send
out
a new version.
Never mind. I'll send out a fix.
Thanks,
Yongseok
>
>> 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
Re: [dpdk-dev] [PATCH v6 44/70] net/mlx5: use virt2memseg instead of iteration
On 17-Apr-18 3:48 AM, Yongseok Koh wrote:
On Apr 11, 2018, at 5:30 AM, Anatoly Burakov wrote:
Reduce dependency on internal details of EAL memory subsystem, and
simplify code.
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(-)
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
Re: [dpdk-dev] [PATCH v6 44/70] net/mlx5: use virt2memseg instead of iteration
> On Apr 11, 2018, at 5:30 AM, Anatoly Burakov
> wrote:
>
> Reduce dependency on internal details of EAL memory subsystem, and
> simplify code.
>
> 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(-)
>
> 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.
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
[dpdk-dev] [PATCH v6 44/70] net/mlx5: use virt2memseg instead of iteration
Reduce dependency on internal details of EAL memory subsystem, and
simplify code.
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(-)
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);
- 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
