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... 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, -- Peter Xu