From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vlad Buslov Subject: Re: [PATCH net-next v6 00/11] Modify action API for implementing lockless actions Date: Fri, 13 Jul 2018 16:40:47 +0300 Message-ID: References: <1530800673-12280-1-git-send-email-vladbu@mellanox.com> <20180708.124325.344679298289898945.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain Cc: David Miller , Linux Kernel Network Developers , Jamal Hadi Salim , Jiri Pirko , Alexei Starovoitov , Daniel Borkmann , Yevgeny Kliteynik To: Cong Wang Return-path: Received: from mail-eopbgr50068.outbound.protection.outlook.com ([40.107.5.68]:64864 "EHLO EUR03-VE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1729284AbeGMNzt (ORCPT ); Fri, 13 Jul 2018 09:55:49 -0400 In-reply-to: Sender: netdev-owner@vger.kernel.org List-ID: On Fri 13 Jul 2018 at 03:54, Cong Wang wrote: > On Sat, Jul 7, 2018 at 8:43 PM David Miller wrote: >> >> From: Vlad Buslov >> Date: Thu, 5 Jul 2018 17:24:22 +0300 >> >> > Currently, all netlink protocol handlers for updating rules, actions and >> > qdiscs are protected with single global rtnl lock which removes any >> > possibility for parallelism. This patch set is a first step to remove >> > rtnl lock dependency from TC rules update path. >> ... >> >> I'll apply this for now, I reviewed it a few more times and I see >> where you are going with this. > > Dear David, > > I don't understand why you even believe the claim of lockless > updaters here, it at least should raise a red flag when you see any > kinda of this claim. > > I know you don't trust me, how about thinking it in this way: > > Why does RCU still require a lock for RCU writers? (Or at least > RCU recommends a lock, if anyone really wants to point out some > lockless algorithm here.) > > or: > > If writers could really go lockless as easily as Vlad claims, how could > even Paul E. McKenney never bring it into RCU? > > Maybe Vlad is much cleverer than any of us here, and maybe he really > discovers a very brilliant algorithm to allow TC actions to be updated > locklessly, why not wait until he shows a proof (either code or a paper)? > Is there a rush? I don't see it. > > In fact, I discussed this with Vlad a little bit at netdev TC workshop. > I never see any brilliant algorithm from him from his slides, and I was > told by him he used "copy and replace" to archive parallel updaters, I > told him that is basically how RCU works and RCU writers have to be > sync'ed with a lock (or at least recommended). > > Also, to confirm my judgement, I checked this with Paul privately too. > Paul said you have to be extremely careful to go lockless, it is very hard > to be bug free for lockless, although he _never_ says it is impossible. > > My _personal_ bet is that, lockless updates for TC filters or actions > are impossible unless there are more things hiding behind "copy and > replace", for example, some brilliant lockless algorithm. If lockless is > really impossible in this circumstance, then many of your efforts in > this patchset are vain, by the way. > > I _do_ believe you can break RTNL down to per device, per filter or per > action, but no matter how small the locking scope is, there is still a lock. > With a lock, there is no need to make things friendly to lockless, like > making an integer increment inside an action to be atomic (your patch > 02/11). > > Please _do_ prove my personal judgement is wrong, by showing your > final code or a formal paper/article. I am very *happy* to be proved > to be wrong here, I am very open to change my mind here. > > Vlad, we need your proof. Please prove I am wrong, seriously!!! :) > > Thanks to anyone for proving me I am wrong just in case!!! :) Dear Cong, I never claimed to have some new brilliant algorithm that completely removed any locks from rules update path. Obviously, fine-grained locking is introduced when necessary. I'm sorry if my liberal usage of term "lockless" confused you. I guess I should be more specific. I'm fully agree with you that totally removing any and all locks from rules update path would require some engineering marvel.