netdev - Re: [Patch net-next] net_sched: get rid of rcu_barrier() in tcf_block_put

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 3 Dec 2017 11:32:19 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Jiri Pirko <jiri@...lanox.com>,
        Jamal Hadi Salim <jhs@...atatu.com>
Subject: Re: [Patch net-next] net_sched: get rid of rcu_barrier() in tcf_block_put_ext()

On Sat, Dec 2, 2017 at 1:21 AM, Jiri Pirko <jiri@...nulli.us> wrote:
> Sat, Dec 02, 2017 at 01:18:04AM CET, xiyou.wangcong@...il.com wrote:
>>Both Eric and Paolo noticed the rcu_barrier() we use in
>>tcf_block_put_ext() could be a performance bottleneck when
>>we have lots of filters.
>
> The problem is not a lots of filters, the problem is lots of classes and
> therefore tcf_blocks

Fixed.


[...]
>>@@ -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)
>
> You don't need this counter. You can just check
> list_empty(block->chain_list);


Makes sense! Done.

[...]
>>@@ -364,13 +355,9 @@ 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);
>>+      /* At this point, all the chains should have refcnt >= 1. */
>>+      list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
>>+              tcf_chain_put(chain);
>
> I think this is correct. Would be probably good to elaborate a bit more
> about what is happening. Perhaps a comment?

OK, I will add a comment about this refcnt.

Thanks!