linux-kernel - Re: [Linux-nvdimm] [PATCH 4/6] SQUSHME: pmem: Micro cleaning

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 31 Mar 2015 18:43:40 +0300
From:	Boaz Harrosh <boaz@...xistor.com>
To:	Dan Williams <dan.j.williams@...el.com>
CC:	Christoph Hellwig <hch@....de>, Jens Axboe <axboe@...nel.dk>,
	linux-nvdimm <linux-nvdimm@...1.01.org>, X86 ML <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [Linux-nvdimm] [PATCH 4/6] SQUSHME: pmem: Micro cleaning

On 03/31/2015 06:30 PM, Dan Williams wrote:
> On Tue, Mar 31, 2015 at 8:24 AM, Boaz Harrosh <boaz@...xistor.com> wrote:
>> On 03/31/2015 06:17 PM, Dan Williams wrote:
>>> On Tue, Mar 31, 2015 at 6:27 AM, Boaz Harrosh <boaz@...xistor.com> wrote:
>>>>
>>>> Some error checks had unlikely some did not. Put unlikely
>>>> on all error handling paths.
>>>> (I like unlikely for error paths specially for readability)
>>>
>>> "unlikely()" is not a readability hint, it's specifically for branches
>>> that profiling shows adding it makes a difference.  Just delete them
>>> all until profiling show they make a difference.  They certainly don't
>>> make a difference in the slow paths.
>>>
>>
>> Why?
> 
> Because the compiler and cpu already does a decent job, and if you get
> the frequency wrong it can hurt performance [1].
> 
> It's pre-mature optimization to sprinkle them around, especially in slow paths.
> 
> [1]: https://lwn.net/Articles/420019/
> 

Sigh! It looks like a holy war. Again all that was said at above thread was about
statistical prediction yes-or-no. And I agree with all the use cases.

But not here. This is not an optimization this is the *error path*.
What I'm saying is:
	"No compiler nor CPU, even if 99% of the time this branch is taken
         I still consider it cold. Because it is the error case and
         I do not care for it"

And no I did not get it wrong. All these places are "error paths" that I do not
care for.

If any of these places are dependent on some input or code variable then yes
let the smarts do it. But never in the "error path".

That said. the patch is up for grabs. I like it ...

Thanks
Boaz

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/