xfs
[Top] [All Lists]

Re: [PATCH] split xfs_ioc_xattr

To: Timothy Shimmin <tes@xxxxxxx>
Subject: Re: [PATCH] split xfs_ioc_xattr
From: Andi Kleen <andi@xxxxxxxxxxxxxx>
Date: Wed, 16 Apr 2008 09:29:27 +0200
Cc: David Chinner <dgc@xxxxxxx>, Niv Sardi <xaiki@xxxxxxxxxxxxx>, Christoph Hellwig <hch@xxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <4805A589.7080906@xxxxxxx> (Timothy Shimmin's message of "Wed, 16 Apr 2008 17:06:49 +1000")
References: <20080319204014.GA23644@xxxxxx> <ncciqylf7q0.fsf@xxxxxxx> <20080414032940.GA10579@xxxxxx> <ncclk3ejwam.fsf@xxxxxxx> <20080416063712.GN108924158@xxxxxxx> <4805A589.7080906@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.3 (gnu/linux)
Timothy Shimmin <tes@xxxxxxx> writes:
>
> I'm still wondering if likely() and unlikely() should ever be used or not?

It's more than just branch predictors. unlikely also moves unlikely
code out of line and keeps it out of the icache (the obvious
drawback is that it makes the asm code much harder to read
during debugging though -- that is why it used to be turned off 
on x86-64) 

Then CPUs have two types of branch predictors: dynamic ones with
history and static ones. Dynamic branch predictors tend to only work
when the code has been recently executed several times and is still
cached in their history buffers

Now the nature of the kernel is that it is a library serving much more
code running in user space. This leads to often the user space
clearing out the history buffers and caches so kernel code has to often
deal with running cache cold and also branch predictor cold.

Then there are the static branch predictors in the CPU and unlikely()
actually rearranges code to make them predict correctly.

Personally I would say the cache effects (moving code out of line) 
are more important than the branch prediction because cache misses
are more costly than branch misprediction.

That all said it might make sense in some really performance critical code,
especially if it's a in a loop and the gcc static branch predictors
(gcc has a large range of builtin heuristics that say e.g. (x < 0) or
(x == NULL) is unlikely). Most code is probably not performance critical
enough to justify the ugliness of the code annotations. And again
for many situations the builtin predictors of gcc (and the CPU) do
fine without help anyways.

Also if you add them you should at some point run with the unlikely
profiler patch from mm just to make sure that your guesses about
which paths are likely are actually correct. Humans are unfortunately
often wrong on such guesses.

Ideally (but that might ask for too much for normal code writing) you
would only add them to code where you have oprofile data for branch
mispredictions or icache misses.

-Andi
 


<Prev in Thread] Current Thread [Next in Thread>