All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cong Wang <xiyou.wangcong@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: Vlad Buslov <vladbu@mellanox.com>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>,
	Jamal Hadi Salim <jhs@mojatatu.com>,
	Jiri Pirko <jiri@resnulli.us>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Yevgeny Kliteynik <kliteyn@mellanox.com>
Subject: Re: [PATCH net-next v6 00/11] Modify action API for implementing lockless actions
Date: Thu, 12 Jul 2018 20:54:45 -0700	[thread overview]
Message-ID: <CAM_iQpUtybeq1T5hKgTRhZwaRJf1g-YBfR_+G3jfWoifi2+8CQ@mail.gmail.com> (raw)
In-Reply-To: <20180708.124325.344679298289898945.davem@davemloft.net>

On Sat, Jul 7, 2018 at 8:43 PM David Miller <davem@davemloft.net> wrote:
>
> From: Vlad Buslov <vladbu@mellanox.com>
> 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!!! :)

  reply	other threads:[~2018-07-13  4:07 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-05 14:24 [PATCH net-next v6 00/11] Modify action API for implementing lockless actions Vlad Buslov
2018-07-05 14:24 ` [PATCH net-next v6 01/11] net: sched: use rcu for action cookie update Vlad Buslov
2018-07-13  3:52   ` Cong Wang
2018-07-13 13:30     ` Vlad Buslov
2018-07-13 21:51       ` Cong Wang
2018-07-13 22:11         ` David Miller
2018-07-14  0:14           ` Cong Wang
2018-07-16  8:31         ` Vlad Buslov
2018-07-17 20:46           ` Cong Wang
2018-07-05 14:24 ` [PATCH net-next v6 02/11] net: sched: change type of reference and bind counters Vlad Buslov
2018-07-05 14:24 ` [PATCH net-next v6 03/11] net: sched: implement unlocked action init API Vlad Buslov
2018-07-05 14:24 ` [PATCH net-next v6 04/11] net: sched: always take reference to action Vlad Buslov
2018-07-05 14:24 ` [PATCH net-next v6 05/11] net: sched: implement action API that deletes action by index Vlad Buslov
2018-07-05 14:24 ` [PATCH net-next v6 06/11] net: sched: add 'delete' function to action ops Vlad Buslov
2018-08-09 19:38   ` Cong Wang
2018-08-10  9:41     ` Vlad Buslov
2018-07-05 14:24 ` [PATCH net-next v6 07/11] net: sched: implement reference counted action release Vlad Buslov
2018-07-05 14:24 ` [PATCH net-next v6 08/11] net: sched: don't release reference on action overwrite Vlad Buslov
2018-08-13 23:00   ` Cong Wang
2018-08-14 17:23     ` Vlad Buslov
2018-07-05 14:24 ` [PATCH net-next v6 09/11] net: sched: use reference counting action init Vlad Buslov
2018-07-05 14:24 ` [PATCH net-next v6 10/11] net: sched: atomically check-allocate action Vlad Buslov
2018-08-08  1:20   ` Cong Wang
2018-08-08 12:06     ` Vlad Buslov
2018-08-09 23:43       ` Cong Wang
2018-08-10 10:29         ` Vlad Buslov
2018-08-10 21:45           ` Cong Wang
2018-08-13  7:55             ` Vlad Buslov
2018-07-05 14:24 ` [PATCH net-next v6 11/11] net: sched: change action API to use array of pointers to actions Vlad Buslov
2018-08-07 23:26   ` Cong Wang
2018-08-08 11:41     ` Vlad Buslov
2018-08-08 18:29       ` Cong Wang
2018-08-09  7:03         ` Vlad Buslov
2018-07-07 11:41 ` [PATCH net-next v6 00/11] Modify action API for implementing lockless actions David Miller
2018-07-08  3:43 ` David Miller
2018-07-13  3:54   ` Cong Wang [this message]
2018-07-13 13:40     ` Vlad Buslov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAM_iQpUtybeq1T5hKgTRhZwaRJf1g-YBfR_+G3jfWoifi2+8CQ@mail.gmail.com \
    --to=xiyou.wangcong@gmail.com \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kliteyn@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=vladbu@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.