Re: [PATCH] power: use hugepage memory for queue list entry structure

[lihuisong (C)]

在 2025/2/24 19:12, Konstantin Ananyev 写道:

在 2025/2/20 17:41, Konstantin Ananyev 写道:

Hi

Hi all,

Kindly ping for review.


在 2024/12/19 15:53, Huisong Li 写道:

The queue_list_entry structure data is used in rx_callback of io path
when enable PMD Power Management. However its memory is currently from
normal heap memory. For better performance, use hugepage memory to
replace it.

Make sense to me.
Acked-by: Konstantin Ananyev <konstantin.anan...@huawei.com>

I  suppose it would also help if you can provide some numbers:
i.e.: how much exactly it is 'better'?
Did you see any changes in throughput/latency numbers, etc.

This patch is just from my knowledge of DPDK.

I don't what is the good way to evaluate the performance of l3fwd-power.

But I did a test for this after you said.

I found that the throughput of using malloc is better than rte_malloc in
continuous packet flow case.😮

Can you test this patch on your platform?

I did a quick test - didn't see any diff in performance (packet flooding) 
with/without the patch.

Thanks for your testing. So let's drop this patch.

Signed-off-by: Huisong Li <lihuis...@huawei.com>
---
    lib/power/rte_power_pmd_mgmt.c | 10 +++++-----
    1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/power/rte_power_pmd_mgmt.c b/lib/power/rte_power_pmd_mgmt.c
index a2fff3b765..c7bf57a910 100644
--- a/lib/power/rte_power_pmd_mgmt.c
+++ b/lib/power/rte_power_pmd_mgmt.c
@@ -97,7 +97,7 @@ queue_list_find(const struct pmd_core_cfg *cfg, const union 
queue *q)
    }

    static int
-queue_list_add(struct pmd_core_cfg *cfg, const union queue *q)
+queue_list_add(struct pmd_core_cfg *cfg, const union queue *q, unsigned int 
lcore_id)
    {
        struct queue_list_entry *qle;

@@ -105,10 +105,10 @@ queue_list_add(struct pmd_core_cfg *cfg, const union 
queue *q)
        if (queue_list_find(cfg, q) != NULL)
                return -EEXIST;

-       qle = malloc(sizeof(*qle));
+       qle = rte_zmalloc_socket(NULL, sizeof(*qle), RTE_CACHE_LINE_SIZE,
+                                rte_lcore_to_socket_id(lcore_id));
        if (qle == NULL)
                return -ENOMEM;
-       memset(qle, 0, sizeof(*qle));

        queue_copy(&qle->queue, q);
        TAILQ_INSERT_TAIL(&cfg->head, qle, next);
@@ -570,7 +570,7 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, 
uint16_t port_id,
                goto end;
        }
        /* add this queue to the list */
-       ret = queue_list_add(lcore_cfg, &qdata);
+       ret = queue_list_add(lcore_cfg, &qdata, lcore_id);
        if (ret < 0) {
                POWER_LOG(DEBUG, "Failed to add queue to list: %s",
                                strerror(-ret));
@@ -664,7 +664,7 @@ rte_power_ethdev_pmgmt_queue_disable(unsigned int lcore_id,
         * callbacks can be freed. we're intentionally casting away const-ness.
         */
        rte_free((void *)(uintptr_t)queue_cfg->cb);
-       free(queue_cfg);
+       rte_free(queue_cfg);

        return 0;
    }