On 05/24/2017 02:33 PM, Peter Xu wrote:
On Wed, May 24, 2017 at 09:45:48AM +0300, Alexey wrote:
[...]
return false;
}
- ioctl_mask = (__u64)1 << _UFFDIO_REGISTER |
- (__u64)1 << _UFFDIO_UNREGISTER;
+ ioctl_mask = 1 << _UFFDIO_REGISTER |
+ 1 << _UFFDIO_UNREGISTER;
Could I ask why we explicitly removed (__u64) here? Since I see the
old one better.
maybe my change not robust, in any case thank to point me, but now I
think, here should be a constant instead of ioctl_mask, like
UFFD_API_IOCTLS, the total meaning of that check it's make sure kernel
returns to us no error and accepted features.
ok, from the beginning:
if we request unsupported feature (we check it before) or internal
state of userfault ctx inside kernel isn't UFFD_STATE_WAIT_API (for
example we are in the middle of the coping process)
ioctl should end with EINVAL error and ioctls field in
uffdio_api will be empty
Right now I think ioctls check for UFFD_API is not necessary.
We just say here, we will use _UFFDIO_REGISTER, _UFFDIO_UNREGISTER,
but kernel supports it unconditionally, by contrast with
UFFDIO_REGISTER ioctl - it also returns ioctl field in uffdio_register
structure, here can be a variations.
Sorry I didn't get the point...
I misprinted
>We just say here, we will use _UFFDIO_REGISTER
s/_UFFDIO_REGISTER/_UFFDIO_API/g
but the point, ioctl_mask is not necessary here, kernel always returns it.
But for _UFFDIO_UNREGISTER, later, not in this function, yes that check is
required.
AFAIU here (__u64) makes the constant "1" a 64bit variable. Just like
when we do bit shift we normally have "1ULL<<40". I liked it since
even if _UFFDIO_REGISTER is defined as >32 it will not overflow since
by default a constant "1" is a "int" typed (and it's 32bit width).
Thanks,
--
Best regards,
Alexey Perevalov
