linux-kernel - Re: [PATCH] target/file: don't zero iter before iov_iter

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]

Message-ID: <BYAPR04MB4965683837A93C9D863AAFAF86AB0@BYAPR04MB4965.namprd04.prod.outlook.com>
Date:   Mon, 11 Jan 2021 05:23:50 +0000
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@....com>
To:     Pavel Begunkov <asml.silence@...il.com>
CC:     "Martin K . Petersen" <martin.petersen@...cle.com>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "target-devel@...r.kernel.org" <target-devel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] target/file: don't zero iter before iov_iter_bvec

On 1/10/21 18:32, Pavel Begunkov wrote:
> On 11/01/2021 02:06, Chaitanya Kulkarni wrote:
>> On 1/9/21 13:29, Pavel Begunkov wrote:
>>> On 09/01/2021 20:52, Chaitanya Kulkarni wrote:
>>>> On 1/9/21 12:40, Pavel Begunkov wrote:
>>>>> I expect you won't find any, but such little things can pile up
>>>>> into a not-easy-to-spot overhead over time.
>>>> That is what I suspected with the resulting assembly. The commit log
>>>> needs to document that there is no direct impact on the performance
>>> It's obvious that 3-4 extra mov $0 off(%reg) won't change performance
>>> but still hasn't been formally confirmed ...
>> This is obvious for you and me since we spent time into looking into
>> resulting assembly not every reviewer is expected to do that see [1].
>>>> which can be seen with this patch, but this is nice to have
>>> ... so if you don't mind, I won't be resending just for that.
>> As per commit log guidelines [1] you have to quantify the optimization.
>>
>> Since you cannot quantify the optimization modify the commit log explaining
> And then you see "Optimizations usually aren’t free but trade-offs
> between", and the patch doesn't fall under it.
First part applies to all the optimizations with and without tradeoffs
"Quantify optimizations and trade-offs."
The later part doesn't mean optimizations without trade-offs should be
allowed without having any supportive data.
>
> Let me be frank, I see it more like as a whim. If the maintainer agrees
> with that strange requirement of yours and want to bury it under
> bureaucracy, fine by me, don't take it, I don't care, but I haven't
> ever been asked here to do that for patches as this.
I didn't write the commit log guidelines, as a reviewer I'm following them.
The patch commit log claims optimization with neither having any data nor
having the supporting fact ("possibly no observable difference but in the
long term it matters") for the completeness.
> It's not "I cannot" but rather "I haven't even tried to and expect...".
> Don't mix, there is a huge difference between.
Then provide the numbers to support your claim.