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. 3. Allowing an object (chain 0) exists with refcnt==0 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