As a preparation for removing rtnl lock dependency from rules update path,
change tcf block reference counter type to refcount_t to allow modification
by concurrent users.

In block put function perform decrement and check reference counter once to
accommodate concurrent modification by unlocked users. After this change
tcf_chain_put at the end of block put function is called with
block->refcnt==0 and will deallocate block after the last chain is
released, so there is no need to manually deallocate block in this case.
However, if block reference counter reached 0 and there are no chains to
release, block must still be deallocated manually.

Signed-off-by: Vlad Buslov <vla...@mellanox.com>
Acked-by: Jiri Pirko <j...@mellanox.com>
---
 include/net/sch_generic.h |  2 +-
 net/sched/cls_api.c       | 59 ++++++++++++++++++++++++++++-------------------
 2 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 9a295e690efe..45fee65468d0 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -345,7 +345,7 @@ struct tcf_chain {
 struct tcf_block {
        struct list_head chain_list;
        u32 index; /* block index for shared blocks */
-       unsigned int refcnt;
+       refcount_t refcnt;
        struct net *net;
        struct Qdisc *q;
        struct list_head cb_list;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index cfa4a02a6a1a..c3c7d4e2f84c 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -240,7 +240,7 @@ static void tcf_chain_destroy(struct tcf_chain *chain)
        if (!chain->index)
                block->chain0.chain = NULL;
        kfree(chain);
-       if (list_empty(&block->chain_list) && block->refcnt == 0)
+       if (list_empty(&block->chain_list) && !refcount_read(&block->refcnt))
                kfree(block);
 }
 
@@ -510,7 +510,7 @@ static struct tcf_block *tcf_block_create(struct net *net, 
struct Qdisc *q,
        INIT_LIST_HEAD(&block->owner_list);
        INIT_LIST_HEAD(&block->chain0.filter_chain_list);
 
-       block->refcnt = 1;
+       refcount_set(&block->refcnt, 1);
        block->net = net;
        block->index = block_index;
 
@@ -719,7 +719,7 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct 
Qdisc *q,
                /* block_index not 0 means the shared block is requested */
                block = tcf_block_lookup(net, ei->block_index);
                if (block)
-                       block->refcnt++;
+                       refcount_inc(&block->refcnt);
        }
 
        if (!block) {
@@ -762,7 +762,7 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct 
Qdisc *q,
 err_block_insert:
                kfree(block);
        } else {
-               block->refcnt--;
+               refcount_dec(&block->refcnt);
        }
        return err;
 }
@@ -802,34 +802,45 @@ void tcf_block_put_ext(struct tcf_block *block, struct 
Qdisc *q,
        tcf_chain0_head_change_cb_del(block, ei);
        tcf_block_owner_del(block, q, ei->binder_type);
 
-       if (block->refcnt == 1) {
-               if (tcf_block_shared(block))
-                       tcf_block_remove(block, block->net);
-
-               /* Hold a refcnt for all chains, so that they don't disappear
-                * while we are iterating.
+       if (refcount_dec_and_test(&block->refcnt)) {
+               /* Flushing/putting all chains will cause the block to be
+                * deallocated when last chain is freed. However, if chain_list
+                * is empty, block has to be manually deallocated. After block
+                * reference counter reached 0, it is no longer possible to
+                * increment it or add new chains to block.
                 */
-               list_for_each_entry(chain, &block->chain_list, list)
-                       tcf_chain_hold(chain);
+               bool free_block = list_empty(&block->chain_list);
 
-               list_for_each_entry(chain, &block->chain_list, list)
-                       tcf_chain_flush(chain);
-       }
+               if (tcf_block_shared(block))
+                       tcf_block_remove(block, block->net);
 
-       tcf_block_offload_unbind(block, q, ei);
+               if (!free_block) {
+                       /* Hold a refcnt for all chains, so that they don't
+                        * disappear while we are iterating.
+                        */
+                       list_for_each_entry(chain, &block->chain_list, list)
+                               tcf_chain_hold(chain);
 
-       if (block->refcnt == 1) {
-               /* At this point, all the chains should have refcnt >= 1. */
-               list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {
-                       tcf_chain_put_explicitly_created(chain);
-                       tcf_chain_put(chain);
+                       list_for_each_entry(chain, &block->chain_list, list)
+                               tcf_chain_flush(chain);
                }
 
-               block->refcnt--;
-               if (list_empty(&block->chain_list))
+               tcf_block_offload_unbind(block, q, ei);
+
+               if (free_block) {
                        kfree(block);
+               } else {
+                       /* At this point, all the chains should have
+                        * refcnt >= 1.
+                        */
+                       list_for_each_entry_safe(chain, tmp, &block->chain_list,
+                                                list) {
+                               tcf_chain_put_explicitly_created(chain);
+                               tcf_chain_put(chain);
+                       }
+               }
        } else {
-               block->refcnt--;
+               tcf_block_offload_unbind(block, q, ei);
        }
 }
 EXPORT_SYMBOL(tcf_block_put_ext);
-- 
2.7.5

Reply via email to