Re: Review request: draft ioctl_userfaultfd(2) manual page

[Michael Kerrisk (man-pages)]

Hi Mike,

On 04/21/2017 01:07 PM, Mike Rapoport wrote:
> Hello Michael,
> 
> On Fri, Apr 21, 2017 at 11:11:18AM +0200, Michael Kerrisk (man-pages) wrote:
>> Hello Mike,
>> Hello Andrea (we need your help!),
>>
>> On 03/22/2017 02:54 PM, Mike Rapoport wrote:
>>> Hello Michael,
>>>
>>> On Mon, Mar 20, 2017 at 09:11:07PM +0100, Michael Kerrisk (man-pages) wrote:
>>>> Hello Andrea, Mike, and all,
>>>>
>>>> Mike: here's the split out page that describes the 
>>>> userfaultfd ioctl() operations.
>>>>
>>>> I'd like to get review input, especially from you and
>>>> Andrea, but also anyone else, for the current version
>>>> of this page, which includes quite a few FIXMEs to be
>>>> sorted.
>>>>
>>>> I've shown the rendered version of the page below. 
>>>> The groff source is attached, and can also be found
>>>> at the branch here:
>>>>
>>>> https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/log/?h=draft_userfaultfd
>>>>
>>>> The new ioctl_userfaultfd(2) page follows this mail.
>>>>
>>>> Cheers,
>>>>
>>>> Michael
>>>>
>>>> NAME
>>>>        userfaultfd - create a file descriptor for handling page faults in user
>>>>        space
>>>>
>>>> SYNOPSIS
>>>>        #include <sys/ioctl.h>
>>>>
>>>>        int ioctl(int fd, int cmd, ...);
>>>>
>>>> DESCRIPTION
>>>>        Various ioctl(2) operations can be performed on  a  userfaultfd  object
>>>>        (created by a call to userfaultfd(2)) using calls of the form:
>>>>
>>>>            ioctl(fd, cmd, argp);
>>>>
>>>>        In  the  above,  fd  is  a  file  descriptor referring to a userfaultfd
>>>>        object, cmd is one of the commands listed below, and argp is a  pointer
>>>>        to a data structure that is specific to cmd.
>>>>
>>>>        The  various  ioctl(2) operations are described below.  The UFFDIO_API,
>>>>        UFFDIO_REGISTER, and UFFDIO_UNREGISTER operations are used to configure
>>>>        userfaultfd behavior.  These operations allow the caller to choose what
>>>>        features will be enabled and what kinds of events will be delivered  to
>>>>        the application.  The remaining operations are range operations.  These
>>>>        operations enable the calling application to resolve page-fault  events
>>>>        in a consistent way.
>>>>
>>>>
>>>>        âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>>        âFIXME                                                â
>>>>        âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>>        âAbove: What does "consistent" mean?                  â
>>>>        â                                                     â
>>>>        âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>
>>> Andrea, can you please help with this one?
>>
>> Let's see what Andrea has to say.
> 
> Actually, I though I've copied this text from Andrea's docs, but now I've
> found out it was my wording and I really don't remember now what was my
> intention for "consistent" :)
> My guess is that I was thinking about atomicity of UFFDIO_COPY, or the fact
> that from the faulting thread perspective the page fault handling is the
> same whether it's done in kernel or via userfaultfd...
> That said, maybe it'd be better just to drop "in a consistent way".

Okay. Dropped.

>>>>    UFFDIO_API
>>>>        (Since Linux 4.3.)  Enable operation of the userfaultfd and perform API
>>>>        handshake.  The argp argument is a pointer to a  uffdio_api  structure,
>>>>        defined as:
>>>>
>>>>            struct uffdio_api {
>>>>                __u64 api;        /* Requested API version (input) */
>>>>                __u64 features;   /* Must be zero */
>>>>                __u64 ioctls;     /* Available ioctl() operations (output) */
>>>>            };
>>>>
>>>>        The  api  field  denotes  the API version requested by the application.
>>>>        Before the call, the features field must be initialized to zero.
>>>>
>>>>
>>>>        âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>>        âFIXME                                                â
>>>>        âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>>        âAbove: Why must the 'features' field be  initialized â
>>>>        âto zero?                                             â
>>>>        âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>
>>> Until 4.11 the only supported feature is delegation of missing page fault
>>> and the UFFDIO_FEATURES bitmask is 0.
>>
>> So, the thing that was not clear, but now I think I understand:
>> 'features' is an input field where one can ask about supported features
>> (but none are supported, before Linux 4.11). Is that correct?
> 
> Yes.

Thanks.

>> I've changed the text here to read:
>>
>>        Before the call, the features field must be  initialized
>>        to  zero.  In the future, it is intended that this field can be
>>        used to ask whether particular features are supported.
>>
>> Seem okay?
> 
> Yes.
> Just the future is only a week or two from today as we are at 4.11-rc7 :)

Yes, I understand :-). So of course there's a *lot* more
new stuff to document, right?

[...]

>>>>    UFFDIO_REGISTER

[...]

>>>>        EINVAL There as an incompatible mapping in the specified address range.
>>>>
>>>>
>>>>               âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>>               âFIXME                                                â
>>>>               âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>>               âAbove: What does "incompatible" mean?                â
>>>>               â                                                     â
>>>>               âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>
>>> Up to 4.10 userfault context may be registered only for MAP_ANONYMOUS |
>>> MAP_PRIVATE mappings.
>>
>> Hmmm -- this restriction is not actually mentioned in the description
>> of UFFDIO_REGISTER. So, at the start of the description of that operation, 
>> I've made the text as follows:
>>
>> [[
>> .SS UFFDIO_REGISTER
>> (Since Linux 4.3.)
>> Register a memory address range with the userfaultfd object.
>> The pages in the range must be "compatible".
>> In the current implementation,
>> .\" According to Mike Rapoport, this will change in Linux 4.11.
>> only private anonymous ranges are compatible for registering with
>> .BR UFFDIO_REGISTER .
>> ]]
>>
>> Okay?
> 
> Yes.

Thanks for checking it.

>>>>    UFFDIO_UNREGISTER

[...]

>>>>        EINVAL There as an incompatible mapping in the specified address range.
>>>>
>>>>
>>>>               âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>>               âFIXME                                                â
>>>>               âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>>               âAbove: What does "incompatible" mean?                â
>>>>               âââââââââââââââââââââââââââââââââââââââââââââââââââââââ
>>>
>>> The same comments as for UFFDIO_REGISTER apply here as well.
>>
>> Okay. I changed the introductory text on UFFDIO_UNREGISTER to say:
>>
>> [[
>> .SS UFFDIO_UNREGISTER
>> (Since Linux 4.3.)
>> Unregister a memory address range from userfaultfd.
>> The pages in the range must be "compatible" (see the description of
>> .BR  UFFDIO_REGISTER .)
>> ]]
>>
>> Okay?
> 
> Yes.

Thanks.

[...]

The current version of the two pages has been pushed to 
https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/log/?h=draft_userfaultfd

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/