Re: [PATCH] staging: exfat: add exfat filesystem code to staging

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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,

> .
> 



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux