[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V6 04/10] migration: split ufd_version_check ont
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH V6 04/10] migration: split ufd_version_check onto receive/request features part |
Date: |
Wed, 31 May 2017 20:12:59 +0100 |
User-agent: |
Mutt/1.8.2 (2017-04-18) |
* Alexey Perevalov (address@hidden) wrote:
> 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.
But Peter's only point was that to build the mask it's better to keep
the (__u64) cast for safety.
Dave
> >
> > 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
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH V6 08/10] migration: calculate vCPU blocktime on dst side, (continued)
- Message not available
- [Qemu-devel] [PATCH V6 08/10] migration: calculate vCPU blocktime on dst side, Alexey Perevalov, 2017/05/23
- Re: [Qemu-devel] [PATCH V6 08/10] migration: calculate vCPU blocktime on dst side, Peter Xu, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 08/10] migration: calculate vCPU blocktime on dst side, Alexey, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 08/10] migration: calculate vCPU blocktime on dst side, Peter Xu, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 08/10] migration: calculate vCPU blocktime on dst side, Alexey Perevalov, 2017/05/24
Message not available
- [Qemu-devel] [PATCH V6 04/10] migration: split ufd_version_check onto receive/request features part, Alexey Perevalov, 2017/05/23
- Re: [Qemu-devel] [PATCH V6 04/10] migration: split ufd_version_check onto receive/request features part, Peter Xu, 2017/05/23
- Re: [Qemu-devel] [PATCH V6 04/10] migration: split ufd_version_check onto receive/request features part, Alexey, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 04/10] migration: split ufd_version_check onto receive/request features part, Peter Xu, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 04/10] migration: split ufd_version_check onto receive/request features part, Alexey Perevalov, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 04/10] migration: split ufd_version_check onto receive/request features part,
Dr. David Alan Gilbert <=
Message not available
- [Qemu-devel] [PATCH V6 07/10] migration: add bitmap for copied page, Alexey Perevalov, 2017/05/23
- Re: [Qemu-devel] [PATCH V6 07/10] migration: add bitmap for copied page, Peter Xu, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 07/10] migration: add bitmap for copied page, Alexey, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 07/10] migration: add bitmap for copied page, Peter Xu, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 07/10] migration: add bitmap for copied page, Alexey Perevalov, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 07/10] migration: add bitmap for copied page, Peter Xu, 2017/05/24
- Re: [Qemu-devel] [PATCH V6 07/10] migration: add bitmap for copied page, Alexey Perevalov, 2017/05/25
- Re: [Qemu-devel] [PATCH V6 07/10] migration: add bitmap for copied page, Peter Xu, 2017/05/25
- Re: [Qemu-devel] [PATCH V6 07/10] migration: add bitmap for copied page, Dr. David Alan Gilbert, 2017/05/31
Message not available