netdev - Re: [PATCH] net: Make synchronize_net() be expedited only when it's really need

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 23 Jan 2018 20:22:37 +0300
From:   Kirill Tkhai <ktkhai@...tuozzo.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        David Ahern <dsahern@...il.com>,
        Florian Westphal <fw@...len.de>,
        Xin Long <lucien.xin@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        mschiffer@...verse-factory.net, jakub.kicinski@...ronome.com,
        Vladislav Yasevich <vyasevich@...il.com>,
        Jiri Benc <jbenc@...hat.com>
Subject: Re: [PATCH] net: Make synchronize_net() be expedited only when it's
 really need

On 23.01.2018 20:13, Eric Dumazet wrote:
> On Tue, Jan 23, 2018 at 9:09 AM, Kirill Tkhai <ktkhai@...tuozzo.com> wrote:
>> On 23.01.2018 19:58, Eric Dumazet wrote:
>>>>
>>>> Your original patch did not provide any test results. Only the fact synchronize_rcu_expedited()
>>>> completes faster than plain synchronize_rcu(). But this is an obvious fact
>>>> just because of the design, and this is described even in the documentation.
>>>> Beleive me, I don't want to offend you this words, but it's strange you hadn't
>>>> gone the way you suggest me.
>>>
>>> Well, I guess you missed the fine changelog.
>>>
>>> Please carefully read it :
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=be3fc413da9eb17cce0991f214ab019d16c88c41
>>
>> This is just what I said.
> 
> You said that I provided no test results. But I really did. That is
> included in the changelog.
> 
> You provided no test results, but some confusing changelog that left
> the reader for whatever interpretation.

It's a result of synchronize_rcu() vs synchronize_rcu_expedited() execution.
It's an "atomic" result and it comes from RCU design. And it's obvious.
You have not provided any real workload results and how the excess interrupt
act on it. Read my message once again. This is what I wrote in it.

>>
>>> And you' ll noticed I had an Ack from Paul E. McKenney, the RCU maintainer.
>> I won't ask you either you had the ACK before you sent the patch, because
>> the answer is obvious.
>>
>> Anyway, this doesn't matter. I don't insist on the fix for this place.
>> If you're so negative on this question, we may leave everything as is,
>> and live with this ambiguous place several years more.
> 
> Your patch has potential serious issues and you refuse to address my feedback,
> I am not sure we will progress.
> 
> If you are ready to leave this for following years, why this patch was
> targeting net tree, I wonder.

Eric, I took your advice about net-next from your the first message and
agreed in my answer on it. Strange, you've repeated this already 3 times
though I have no objections.

Kirill