Re: [Qemu-devel] [PATCH V6 04/10] migration: split ufd_version_check onto receive/request features part

[Peter Xu]

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