Hi Konstantin,
On 03/18/2015 04:13 PM, Ananyev, Konstantin wrote:
>
>> From: Vadim Suraev [mailto:vadim.suraev at gmail.com]
>> Sent: Wednesday, March 18, 2015 10:41 AM
>> To: Ananyev, Konstantin
>> Cc: dev at dpdk.org
>> Subject: Re: [PATCH v2] rte_mbuf: mbuf bulk alloc/free functions added +
>> unittest
>>
>> Hi, Konstantin,
>>
>> Got it. To make the same, nulling the next should be inside of the block as
>> you said.
>> One question raises here: If a segment in the chain has refcnt > 1 (so its
>> next is not assigned NULL), and the next segment has refcnt
>> == 1 (so it is freed), do you think this scenario is real/should be
>> considered? If so, the former can be safely freed only by calling
>> rte_pktmbuf_free_seg which does not iterate. So why to keep next pointing to
>> something?
>
> I think we need it, not just to keep things the same with
> rte_pktmbuf_free(), but because it is a right thing to do.
> Let say you have a packet in 2 mbufs chained together, both mbufs have
> refcnt==2.
> Then:
> rte_pktmbuf_free(firs_mbuf);
> rte_pktmbuf_free(firs_mbuf);
>
> Would work correctly and free both mbufs back to the mempool.
>
> While after:
> rte_pktmbuf_free_chain(first_mbuf);
> rte_pktmbuf_free_chain(first_mbuf);
>
> We would have first_mbuf freed back into the mempool, while second would get
> lost(memory leaking).
> Basically free() shouldn't modify any filed inside mbuf, except refcnt if
> rte_mbuf_refcnt_update(m, -1) > 0
>
> About your case, when: first_mbuf->refcnt==2 and second_mbuf->refcnt==1.
> Right now, rte_pktmbuf_free() can't handle such cases properly,
> and, as I know, such situation is not considered as valid one.
I'm not sure I understand what you are saying. To me, the case you are
describing is similar to the case below, and it should work properly:
/* allocate a packet and clone it. After that, m1 has a
* refcnt of 2 */
m1 = rte_pktmbuf_alloc();
clone1 = rte_pktmbuf_clone(m1);
/* allocate another packet */
m2 = rte_pktmbuf_alloc();
/* chain m2 after m1, updating fields like total length.
* After that, m1 has 2 segments, the first one has a refcnt
* of 1 and the second has a refcnt of 2 */
mbuf_concat(m1, m2);
/* This will decrement the refcnt on the first segment and
* free the second segment */
rte_pktmbuf_free(m1);
/* free the indirect mbuf, and as the refcnt is 1 on the
* direct mbuf (m1), also release it */
rte_pktmbuf_free(clone1);
Am I missing something?
Thanks,
Olivier
