From: Kyle Moffett <mrmacman_g4@mac.com>
To: Erez Zadok <ezk@cs.sunysb.edu>
Cc: "Kok, Auke" <auke-jan.h.kok@intel.com>,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org, viro@ftp.linux.org.uk,
hch@infradead.org
Subject: Re: [PATCH 10/25] Unionfs: add un/likely conditionals on copyup ops
Date: Wed, 26 Sep 2007 11:23:19 -0400 [thread overview]
Message-ID: <2DDDD55D-4445-4565-9384-4E4BE7B41D7D@mac.com> (raw)
In-Reply-To: <200709261340.l8QDeKie025351@agora.fsl.cs.sunysb.edu>
On Sep 26, 2007, at 09:40:20, Erez Zadok wrote:
> In message <46F9E0EC.3010105@intel.com>, "Kok, Auke" writes:
>> I've been told several times that adding these is almost always
>> bogus - either it messes up the CPU branch prediction or the
>> compiler/CPU just does a lot better at finding the right way
>> without these hints.
>>
>> Adding them as a blanket seems rather strange. Have you got any
>> numbers that this really improves performance?
>
> Auke, that's a good question, but I found it hard to find any info
> about it. There's no discussion on it in Documentation/, and very
> little I could find elsewhere. I did see one url explaining what
> un/likely does precisely, but no guidelines. My understanding is
> that it can improve performance, as long as it's used carefully
> (otherwise it may hurt performance).
Hmm, even still I agree with Auke, you probably use it too much.
> Recently we've done a full audit of the entire code, and added un/
> likely where we felt that the chance of succeeding is 95% or better
> (e.g., error conditions that should rarely happen, and such).
Actually due to the performance penalty on some systems I think you
only want to use it if the chance of succeeding is 99% or better, as
the benefit if predicted is a cycle or two and the harm if
mispredicted can be more than 50 cycles, depending on the CPU. You
should also remember than in filesystems many "failures" are
triggered by things like the ld.so library searches, where it
literally calls access() 20 different times on various possible paths
for library files, failing the first 19. It does this once for each
necessary library.
Typically you only want to add unlikely() or likely() for about 2
reasons:
(A) It's a hot path and the unlikely case is just going to burn a
bunch of CPU anyways
(B) It really is extremely unlikely that it fails (Think physical
hardware failure)
Anything else is just bogus.
Cheers,
Kyle Moffett
next prev parent reply other threads:[~2007-09-26 15:24 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-09-26 3:09 [GIT PULL -mm] 00/25 Unionfs updates/cleanups/fixes Erez Zadok
2007-09-26 3:09 ` [PATCH 01/25] Unionfs: Simplify unionfs_get_nlinks Erez Zadok
2007-09-26 3:09 ` [PATCH 02/25] Unionfs: Remove unused #defines Erez Zadok
2007-09-26 3:09 ` [PATCH 03/25] Unionfs: display informational messages only if debug is on Erez Zadok
2007-09-26 8:36 ` Jan Engelhardt
2007-09-26 14:01 ` Erez Zadok
2007-09-26 15:24 ` Jan Engelhardt
2007-09-26 15:28 ` Erez Zadok
2007-09-26 3:09 ` [PATCH 04/25] Unionfs: cache-coherency fixes Erez Zadok
2007-09-26 3:09 ` [PATCH 05/25] Unionfs: cast page->index loff_t before shifting Erez Zadok
2007-09-26 8:31 ` Christoph Hellwig
2007-09-26 13:44 ` Erez Zadok
2007-09-26 3:09 ` [PATCH 06/25] Unionfs: minor coding style updates Erez Zadok
2007-09-26 3:09 ` [PATCH 07/25] Unionfs: add lower nameidata debugging support Erez Zadok
2007-09-26 3:09 ` [PATCH 08/25] Unionfs: lower nameidata support for nfsv4 Erez Zadok
2007-09-26 3:09 ` [PATCH 09/25] Unionfs: add un/likely conditionals on common fileops Erez Zadok
2007-09-26 3:09 ` [PATCH 10/25] Unionfs: add un/likely conditionals on copyup ops Erez Zadok
2007-09-26 4:32 ` Kok, Auke
2007-09-26 13:40 ` Erez Zadok
2007-09-26 15:23 ` Kyle Moffett [this message]
2007-09-26 15:43 ` Erez Zadok
2007-09-26 16:47 ` Jan Engelhardt
2007-09-26 16:51 ` Erez Zadok
2007-09-26 18:34 ` Adrian Bunk
2007-09-26 3:09 ` [PATCH 11/25] Unionfs: add un/likely conditionals on debug ops Erez Zadok
2007-09-26 21:34 ` roel
2007-09-26 3:09 ` [PATCH 12/25] Unionfs: add un/likely conditionals on dentry ops Erez Zadok
2007-09-26 3:09 ` [PATCH 13/25] Unionfs: add un/likely conditionals on dir ops Erez Zadok
2007-09-26 21:40 ` roel
2007-09-27 14:28 ` Erez Zadok
2007-09-26 3:09 ` [PATCH 14/25] Unionfs: add un/likely conditionals on headers Erez Zadok
2007-09-26 3:09 ` [PATCH 15/25] Unionfs: add un/likely conditionals on fileops Erez Zadok
2007-09-26 3:09 ` [PATCH 16/25] Unionfs: add un/likely conditionals on inode ops Erez Zadok
2007-09-26 3:09 ` [PATCH 17/25] Unionfs: add un/likely conditionals on lookup ops Erez Zadok
2007-09-26 3:09 ` [PATCH 18/25] Unionfs: add un/likely conditionals on super ops Erez Zadok
2007-09-26 3:09 ` [PATCH 19/25] Unionfs: add un/likely conditionals on mmap ops Erez Zadok
2007-09-26 3:09 ` [PATCH 20/25] Unionfs: add un/likely conditionals on rename ops Erez Zadok
2007-09-26 3:10 ` [PATCH 21/25] Unionfs: add un/likely conditionals on readdir ops Erez Zadok
2007-09-26 3:10 ` [PATCH 22/25] Unionfs: add un/likely conditionals on common subr Erez Zadok
2007-09-26 3:10 ` [PATCH 23/25] Unionfs: add un/likely conditionals on unlink ops Erez Zadok
2007-09-26 3:10 ` [PATCH 24/25] Unionfs: add un/likely conditionals on xattr ops Erez Zadok
2007-09-26 3:10 ` [PATCH 25/25] Unionfs: use poison.h for safe poison pointers Erez Zadok
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2DDDD55D-4445-4565-9384-4E4BE7B41D7D@mac.com \
--to=mrmacman_g4@mac.com \
--cc=akpm@linux-foundation.org \
--cc=auke-jan.h.kok@intel.com \
--cc=ezk@cs.sunysb.edu \
--cc=hch@infradead.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=viro@ftp.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®