Wed, Sep 06, 2017 at 07:40:02PM CEST, xiyou.wangc...@gmail.com wrote: >On Wed, Sep 6, 2017 at 4:14 AM, Jiri Pirko <j...@resnulli.us> wrote: >> From: Jiri Pirko <j...@mellanox.com> >> >> There's a memleak happening for chain 0. The thing is, chain 0 needs to >> be always present, not created on demand. Therefore tcf_block_get upon >> creation of block calls the tcf_chain_create function directly. The >> chain is created with refcnt == 1, which is not correct in this case and >> causes the memleak. So move the refcnt increment into tcf_chain_get >> function even for the case when chain needs to be created. >> > >Your approach could work but you just make the code even >uglier than it is now: > >1. The current code is already ugly for special-casing chain 0: > > if (--chain->refcnt == 0 && !chain->filter_chain && chain->index != 0) > tcf_chain_destroy(chain); > >2. With your patch, chain 0 has a different _initial_ refcnt with others.
No. Initial refcnt is the same. ! for every action that holds the chain. So actually, it returns it back where it should be. > >3. Allowing an object (chain 0) exists with refcnt==0 So? That is for every chain that does not have goto_chain action pointing at. Please read the code. > >Compare it with my patch: > >1. No special-case for chain 0, the above ugly part is removed > >2. Every chain is equal and created with refcnt==1 > >3. Any chain with refcnt==0 is destroyed
