On 2025/1/10 1:03, Stephen Hemminger wrote:
> On Thu, 9 Jan 2025 15:43:12 +0800
> huangdengdui <huangdeng...@huawei.com> wrote:
>
>> On 2025/1/9 0:57, Stephen Hemminger wrote:
>>> On Wed, 8 Jan 2025 10:40:43 +0800
>>> Jie Hai <haij...@huawei.com> wrote:
>>>
>>>> On 2024/12/31 1:55, Stephen Hemminger wrote:
>>>>> On Mon, 30 Dec 2024 14:54:03 +0800
>>>>> Jie Hai <haij...@huawei.com> wrote:
>>>>>
>>>>>> From: Jie Hai <haij...@huawei.com>
>>>>>> To: <dev@dpdk.org>, <tho...@monjalon.net>, <ferruh.yi...@amd.com>,
>>>>>> <david.march...@redhat.com>, <andrew.rybche...@oktetlabs.ru>, Chengwen
>>>>>> Feng <fengcheng...@huawei.com>, "Wei Hu (Xavier)"
>>>>>> <xavier.hu...@huawei.com>, Huisong Li <lihuis...@huawei.com>
>>>>>> CC: <haij...@huawei.com>, <huangdeng...@huawei.com>
>>>>>> Subject: [PATCH 1/3] net/hns3: fix simple Tx path incorrect free the mbuf
>>>>>> Date: Mon, 30 Dec 2024 14:54:03 +0800
>>>>>> X-Mailer: git-send-email 2.22.0
>>>>>>
>>>>>> From: Dengdui Huang <huangdeng...@huawei.com>
>>>>>>
>>>>>> When RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE offload is not set,
>>>>>> use rte_pktmbuf_free_seg() to free the mbuf.
>>>>>>
>>>>>> Fixes: 7ef933908f04 ("net/hns3: add simple Tx path")
>>>>>> Cc: sta...@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Dengdui Huang <huangdeng...@huawei.com>
>>>>>> Signed-off-by: Jie Hai <haij...@huawei.com>
>>>>>
>>>>> What about the fast free case which is using rte_mempool_put_bulk when
>>>>> it should use rte_pktmbuf_free_bulk instead?
>>>>>
>>>>>
>>>> Hi, Stephen Hemminger,
>>>>
>>>> During the fast free case, the performance of using
>>>> rte_mempool_put_bulk is higher than that of using
>>>> rte_pktmbuf_free_bulk because other references
>>>> to mbuf do not need to be considered. So it's better
>>>> to not change.
>>>>
>>>> Thanks,
>>>> Jie Hai
>>>
>>> The problem is that having an open coded version of this buried in
>>> one driver is a long term potential proble>
>>> If you really think that optimizing free like this is noticeable, then
>>> why not make a new function "rte_pktmuf_fast_free_bulk" and put it in the
>>> regular mbuf library.
>>>
>>
>> Do you mean to add the following functions to the library?
>>
>> void rte_pktmbuf_fast_free_bulk(struct rte_mbuf **mbufs, unsigned int count)
>> {
>> rte_mempool_put_bulk(mbufs[0]->pool, (void **)mbufs, count);
>> }
>
> Yes something like that. Or call it rte_mbuf_raw_free_bulk to align with
> what rte_mbuf_raw_free(). And maybe add some debug assertions to make sure
> that mbuf is not cloned, indirect or has refcnt.
>
> The concern is that this optimization might put an mbuf in the pool
> that has different properties than the normal free path.
> And that all semantics of allocation/free should be in rte_mbuf code to
> allow for future optimizations.
"Morten Brørup" has submitted a patch to implement this function.[1]
Currently, multiple drivers use rte_mempool_put_bulk to release mbufs.
We can modify them later so that all drivers use rte_mbuf_fast_free_bulk to
free mbufs.
The current patch is a function problem, which has caused troubles to hns3
users. Can we merge it first?
[1]https://inbox.dpdk.org/dev/20250114163951.125667-1...@smartsharesystems.com/
>
>
>>
>> The driver uses rte_mempool_put_bulk only when the following conditions are
>> met:
>> 1. All mbufs comes from the same mempool
>> 2. All mbufs have only one reference.
>> 3. All mbufs have only one segment.
>> So the rte_pktmbuf_fast_free_bulk function is just a wrapper around the
>> rte_mempool_put_bulk function.
>
