Problem with uipc_mbuf.c

Randall Stewart rrs at cisco.com
Wed Sep 20 04:03:37 PDT 2006


Andre Oppermann wrote:
> Maxim Konovalov wrote:
> 
>> On Tue, 29 Aug 2006, 17:15+0200, Andre Oppermann wrote:
>>
>>> John-Mark Gurney wrote:
>>>
>>>> Randall Stewart wrote this message on Mon, Aug 28, 2006 at 17:04 -0400:
>>>>
>>>>>      atomic_fetchadd_int(m->m_ext.ref_cnt, -1) == 0) {
>>>>
>>>>         ^
>>>>
>>>> This should be 1 not 0.. as apparently fetchadd_int returns the
>>>> old value (at least that's what atomic(9) says), which means that
>>>> if we ever race on this comparision, we won't free though we
>>>> should of...
>>>>
>>>> if we look at refcount.h, it does:
>>>>         return (atomic_fetchadd_int(count, -1) == 1);
>>>>
>>>> which release a reference and apparently returns true if it needs to
>>>> be free'd...
>>>>
>>>> Though the wierd part is that andre, "fixed" it to be 0 in 1.157:
>>>> Fix a logic error introduced with mandatory mbuf cluster
>>>> refcounting and freeing of mbufs+clusters back to the packet zone.
>>>
>>> Honestly I'm a bit confused myself now and have to dig up things from
>>> when I did the change.  However I'm certain there was a problem and the
>>> commit fixed it in some way (not necessarily the correct way).  Before
>>> the 'fix' there were some larger leaks going on.
>>
>>
>> So what's the conclusion?  Perhaps it's worth to add an XXX comment in
>> meantime.
> 
> 
> Please give me until Thursday to resolve this issue.
> 
Andre:

It is long past Thursday.... any status on this? I have not
seen a commit from you on this... When I commit the SCTP code
(some day soon) then we will have an mbuf leak unless we change
the 0 to a 1....

R

-- 
Randall Stewart
NSSTG - Cisco Systems Inc.
803-345-0369 <or> 815-342-5222 (cell)


More information about the freebsd-net mailing list