On 2019/8/30 19:51, David Sterba wrote: > On Fri, Aug 30, 2019 at 10:06:25AM +0800, Chao Yu wrote: >> On 2019/8/29 23:43, Dan Carpenter wrote: >>>> p.s. There are 2947 (un)likely places in fs/ directory. >>> >>> I was complaining about you adding new pointless ones, not existing >>> ones. The likely/unlikely annotations are supposed to be functional and >>> not decorative. I explained this very clearly. >>> >>> Probably most of the annotations in fs/ are wrong but they are also >>> harmless except for the slight messiness. However there are definitely >>> some which are important so removing them all isn't a good idea. >> >> Hi Dan, >> >> Could you please pick up one positive example using likely and unlikely >> correctly? so we can follow the example, rather than removing them all blindly. > > Remove all of them and re-add with explanation if and how each is going > to make things better. If you can't reason about, prove by benchmarks or > point to inefficient asm code generated, then don't add them again. It seems the result of it is strongly related to arch and compiler, if we readd it after we just prove its gain only in one combination, I doubt we may suffer regression in other combination in between archs and comilers. It looks like we don't have any cheaper way to readd it? instead of verifying all/most combinations. > > The feedback I got from CPU and compiler people over the years is not to > bother using the annotations. CPUs are doing dynamic branch prediction > and compilers are applying tons of optimizations. > > GCC docs state about the builtin: "In general, you should prefer to use > actual profile feedback for this (-fprofile-arcs), as programmers are > notoriously bad at predicting how their programs actually perform." > (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html) Yes, I agree with this. Thanks a lot for sharing your experience. :) The removal change has done and been merged into Greg's tree, let's consider to readd it later if possible as you suggested. thanks, > . >
