On 03/13/2014 01:15 PM, David Herrmann wrote:
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 a compiler can't (or shouldn't) make that assumption. Just as an (adapted) example, imagine that each test had a 20% probability of returning an error. The probability of the function returning an error would then be 68%..
I'm no big fan of excessive likely/unlikely annotations, but I'm fine if you want to keep it.
Fair enough.
Thanks, Thomas
Thanks David _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel