Re: [RFC PATCH] net_sched: bulk free tcf_block

[Cong Wang]

On Wed, Nov 29, 2017 at 6:25 AM, Paolo Abeni <pab...@redhat.com> wrote:
> Currently deleting qdisc with a large number of children and filters
> can take a lot of time:
>
> tc qdisc add dev lo root htb
> for I in `seq 1 1000`; do
>         tc class add dev lo parent 1: classid 1:$I htb rate 100kbit
>         tc qdisc add dev lo parent 1:$I handle $((I + 1)): htb
>         for J in `seq 1 10`; do
>                 tc filter add dev lo parent $((I + 1)): u32 match ip src 
> 1.1.1.$J
>         done
> done
> time tc qdisc del dev root
>
> real    0m54.764s
> user    0m0.023s
> sys     0m0.000s
>
> This is due to the multiple rcu_barrier() calls, one for each tcf_block
> freed, invoked with the rtnl lock held. Most other network related
> tasks will block in this timespan.

Yeah, Eric pointed out this too and I already had an idea to cure
this.

As I already mentioned before, my idea is to refcnt the tcf block
so that we don't need to worry about which is the last. Something
like the attached patch below, note it is PoC _only_, not even
compiled yet. And I am not 100% sure it works either, I will look
deeper tomorrow.

Thanks.

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 65d0d25f2648..b051c519fd48 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -279,6 +279,7 @@ struct tcf_block {
        struct Qdisc *q;
        struct list_head cb_list;
        struct work_struct work;
+       unsigned int nr_chains;
 };
 
 static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index ddcf04b4ab43..da74b311f09e 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -190,6 +190,7 @@ static struct tcf_chain *tcf_chain_create(struct tcf_block 
*block,
                return NULL;
        list_add_tail(&chain->list, &block->chain_list);
        chain->block = block;
+       block->nr_chains++;
        chain->index = chain_index;
        chain->refcnt = 1;
        return chain;
@@ -218,8 +219,12 @@ static void tcf_chain_flush(struct tcf_chain *chain)
 
 static void tcf_chain_destroy(struct tcf_chain *chain)
 {
+       struct tcf_block *block = chain->block;
+
        list_del(&chain->list);
        kfree(chain);
+       if (!--block->nr_chains)
+               kfree(block);
 }
 
 static void tcf_chain_hold(struct tcf_chain *chain)
@@ -341,7 +346,6 @@ static void tcf_block_put_final(struct work_struct *work)
        list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
                tcf_chain_put(chain);
        rtnl_unlock();
-       kfree(block);
 }
 
 /* XXX: Standalone actions are not allowed to jump to any chain, and bound
@@ -365,11 +369,6 @@ void tcf_block_put_ext(struct tcf_block *block, struct 
Qdisc *q,
        tcf_block_offload_unbind(block, q, ei);
 
        INIT_WORK(&block->work, tcf_block_put_final);
-       /* Wait for existing RCU callbacks to cool down, make sure their works
-        * have been queued before this. We can not flush pending works here
-        * because we are holding the RTNL lock.
-        */
-       rcu_barrier();
        tcf_queue_work(&block->work);
 }
 EXPORT_SYMBOL(tcf_block_put_ext);