On Thu, Oct 26, 2017 at 9:05 PM, Eric Dumazet <eric.duma...@gmail.com> wrote:
> On Thu, 2017-10-26 at 18:24 -0700, Cong Wang wrote:
>> ...
>
>> On the other hand, this makes tcf_block_put() ugly and
>> harder to understand. Since David and Eric strongly dislike
>> adding synchronize_rcu(), this is probably the only
>> solution that could make everyone happy.
>
>
> ...
>
>> +static void tcf_block_put_deferred(struct work_struct *work)
>> +{
>> + struct tcf_block *block = container_of(work, struct tcf_block, work);
>> + struct tcf_chain *chain;
>>
>> + rtnl_lock();
>> /* Hold a refcnt for all chains, except 0, in case they are gone. */
>> list_for_each_entry(chain, &block->chain_list, list)
>> if (chain->index)
>> @@ -292,13 +308,27 @@ void tcf_block_put(struct tcf_block *block)
>> list_for_each_entry(chain, &block->chain_list, list)
>> tcf_chain_flush(chain);
>>
>> - /* Wait for RCU callbacks to release the reference count. */
>> + INIT_WORK(&block->work, tcf_block_put_final);
>> + /* Wait for RCU callbacks to release the reference count and make
>> + * sure their works have been queued before this.
>> + */
>> rcu_barrier();
>> + tcf_queue_work(&block->work);
>> + rtnl_unlock();
>> +}
>
>
> On a loaded server, rcu_barrier() typically takes 4 ms.
>
> Way better than synchronize_rcu() (about 90 ms) but still an issue when
> holding RTNL.
>
> We have thousands of filters, and management daemon restarts and rebuild
> TC hierarchy from scratch.
>
> Simply getting rid of 1000 old filters might block RTNL for a while, or
> maybe I misunderstood your patches.
>
Paul pointed out the same.
As I replied, this rcu_barrier() is NOT added by this patchset, it is already
there in current master branch.
