Re: [PATCH net-next v6 01/11] net: sched: use rcu for action cookie update

From: Cong Wang <xiyou.wangcong@gmail.com>
To: Vlad Buslov <vladbu@mellanox.com>
Cc: Linux Kernel Network Developers <netdev@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	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>,
	Jiri Pirko <jiri@mellanox.com>
Subject: Re: [PATCH net-next v6 01/11] net: sched: use rcu for action cookie update
Date: Tue, 17 Jul 2018 13:46:42 -0700	[thread overview]
Message-ID: <CAM_iQpW225OFMX=-GnT=hKrEfhfJVT3E_ytF-bJPmw-55S0DhQ@mail.gmail.com> (raw)
In-Reply-To: <vbfy3ebbn6j.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>

On Mon, Jul 16, 2018 at 1:31 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
>
> On Fri 13 Jul 2018 at 21:51, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Fri, Jul 13, 2018 at 6:30 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >>
> >>
> >> On Fri 13 Jul 2018 at 03:52, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >> > On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >> >>
> >> >> Implement functions to atomically update and free action cookie
> >> >> using rcu mechanism.
> >> >
> >> > Without stating any reason..... Is this even a changelog?
> >>
> >> Yes, it is.
> >
> > What do you expect in a changelog generally? Repeating what
> > your code does? Thanks but we don't even want to read any code
> > unless the need of this code is reasonably justified.
>
> In my cover letter:
>  - Motivation for patchset is presented in first paragraph.
>  - Problems that prevent us from removing rtnl lock dependency are
>  described, problem 3 is about cookie pointer.
>  - In implementation section, point 3 presents solution for that
>  problem.

Do you want to use cover letter as a changelog for all patches in
your patchset? Seriously? :)

Every patch is your patchset is unique, because you are not fixing
a problem can be expressed by a pattern.

Given how hard lockless is generally, probably you even can't
find out a pattern. If you really do, I am happy to learn!

>
> >
> > Can we at least agree you have no justification for this change
> > in this changelog? Or you believe this patch is as trivial as
> > a white space change which doesn't need a justification?
>
> Cong, from your last letter I understand that you want to have
> justification specifically for using atomic operation in this particular
> patch. I agree with you that I should have explained it in more details.
> I found a lot of prior art for same or similar atomic ops usage for rcu
> pointers(examples in my previous mail) and assumed it to be trivial, but
> now I understand that I was wrong in this case.

Thanks for having an agreement!

I expect to see more detailed changelog in your future patches! :)