linux-kernel - Re: sparc: bpf_jit: Move four assignments in bpf_jit

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:   Sun, 4 Sep 2016 15:59:39 +1000
From:   Julian Calaby <julian.calaby@...il.com>
To:     SF Markus Elfring <elfring@...rs.sourceforge.net>
Cc:     sparclinux <sparclinux@...r.kernel.org>,
        Adam Buchbinder <adam.buchbinder@...il.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        "David S. Miller" <davem@...emloft.net>,
        Rabin Vincent <rabin@....in>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors <kernel-janitors@...r.kernel.org>,
        Julia Lawall <julia.lawall@...6.fr>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: sparc: bpf_jit: Move four assignments in bpf_jit_compile()

Hi Markus,

On Sun, Sep 4, 2016 at 3:45 PM, SF Markus Elfring
<elfring@...rs.sourceforge.net> wrote:
>>> I hope so. - I propose to give the refactorings "Reduce scope of variable"
>>> and "Extract a function" (and the corresponding consequences) another look.
>>
>> So you _think_ it does. Come back with real proof.
>
> Which test environments would you find acceptable for further clarification?

Compiling it on GCC for Sparc, obviously.

>> I must also point out that these sorts of optimisations are things the
>> compiler does automatically when compiling this code.
>
> Do you take this detail for granted?

I trust that the GCC developers have done their work well.

>> Therefore it's highly likely that this change will make absolutely
>> no difference whatsoever.
>
> I find this outcome unlikely.

Then provide proof.

>> (And no it won't improve compile speed in any justifiable way)
>
> This was not a goal of my update suggestion for the function "bpf_jit_compile".

I'm looking for some glimmer of usefullness in this patch. I'm not seeing any.

>>> It is generally possible that a specific code generation variant will also affect
>>> the run time properties you mentioned.
>>
>> It's _possible_? Come back with benchmarks.
>
> Which code generation variant would be useful to be clarified further?
>
> Should we avoid to compare software things similar to "apples" and "oranges"
> (while these fruits can make more fun)?   ;-)

Write a benchmark that exercises this function. Measure the time it
took without this change, measure the time it took with this change,
is there a difference.

You cannot expect people to take you seriously if you're proposing
performance changes without any actual ability or interest in
producing performance related data to go along with them.

>> I must also point out that this is a "slow path" i.e. as long as it's
>> not stupidly inefficient, the speed doesn't matter that much.
>
> Can this execution path become warmer (or even "hot") for some other use cases?

That is for you to prove.

>> This change isn't going to improve the speed of this function by any amount
>> that matters.
>
> This is also possible.
>
>
>> Unless you have some pretty damn good proof that these changes improve things,
>> there is absolutely no reason to take them as-is
>
> Would you care for a better source code structure?

You're essentially saying "I think doing things this way is better"
and providing _nothing_ else. I think that things are perfectly fine
the way they are. Convince me with data or something else. Was this
suggested by some expert? Is this part of some big tree-wide effort to
do this that was started by some expert? Did someone do this to some
other driver and you're applying the same fix elsewhere?

>> - you are making the code longer
>
> Yes. - This is true for the suggested update in this function.
>
>
>> and more difficult to read for no benefit
>
> I proposed specific benefits for this software module.

What benefits? You have not proved that this change produces any
useful benefits.

>> and wasting everyone's time in the process.
>
> I assume that a few contributors can take the presented ideas for further considerations.
> Will their value evolve a bit more later?

I am subscribed to four fairly high traffic mailing lists and I read
hundreds of patches each week. You are the only person proposing
changes like these ones as you are (as far as I know) the only person
who thinks they have any value.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@...il.com
Profile: http://www.google.com/profiles/julian.calaby/