Hi
On Thu, Mar 13, 2014 at 1:11 PM, Thomas Hellstrom thellstrom@vmware.com wrote:
Hi.
Thanks for reviewing. I'll incorporate your suggestions, except this one, and resend.
On 03/13/2014 12:19 PM, David Herrmann wrote:
Hi
On Thu, Mar 13, 2014 at 11:57 AM, Thomas Hellstrom thellstrom@vmware.com wrote:
...
if (cmd & IOC_IN) {if (copy_from_user(kdata, (void __user *)arg,usize) != 0) {retcode = -EFAULT;
retcode = drm_ioctl_permit(ioctl->flags, file_priv);if (unlikely(retcode))That "unlikely" seems redundant given that all error paths in drm_ioctl_permit() already are "unlikely".
Yes, we know that's true, but I don't think compilers in general can combine branch prediction hints in that way, or even have the information necessary to do it. I mean even if each individual test resulting in an error is unlikely, how could the compiler know that all tests combined would result in an error being unlikely?
The function is static, so the compiler can see that it returns "!=0" only if one of the "unlikely" branches was hit. So I think it's safe to assume the whole thing returns "!=0" only in unlikely conditions. But it's probably inlined, anyway..
I'm no big fan of excessive likely/unlikely annotations, but I'm fine if you want to keep it.
Thanks David