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]
Message-ID: <551AC0AC.6080205@plexistor.com>
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ