diff mbox series

[v2,4/5] ioctl_userfaultfd.2: fix / update UFFDIO_REGISTER error code list

Message ID 20231003194547.2237424-5-axelrasmussen@google.com
State New
Headers
Series userfaultfd man page updates |

Commit Message

Axel Rasmussen Oct. 3, 2023, 7:45 p.m. UTC 
  The list of error codes in the man page was out of date with respect to
the current state of the kernel. Some errors were partially /
incorrectly described.

Update the error code listing, so it matches the current state of the
kernel, and correctly describes all the errors.

Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
---
 man2/ioctl_userfaultfd.2 | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

Comments

Alejandro Colomar Oct. 8, 2023, 10:06 p.m. UTC | #1 
Hi Axel,

On Tue, Oct 03, 2023 at 12:45:46PM -0700, Axel Rasmussen wrote:
> The list of error codes in the man page was out of date with respect to
> the current state of the kernel. Some errors were partially /
> incorrectly described.
> 
> Update the error code listing, so it matches the current state of the
> kernel, and correctly describes all the errors.
> 
> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
> ---
>  man2/ioctl_userfaultfd.2 | 37 +++++++++++++++++++++----------------
>  1 file changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> index 2ee6a0532..95d69f773 100644
> --- a/man2/ioctl_userfaultfd.2
> +++ b/man2/ioctl_userfaultfd.2
> @@ -388,12 +388,6 @@ On error, \-1 is returned and
>  .I errno
>  is set to indicate the error.
>  Possible errors include:
> -.\" FIXME Is the following error list correct?
> -.\"
> -.TP
> -.B EBUSY
> -A mapping in the specified range is registered with another
> -userfaultfd object.
>  .TP
>  .B EFAULT
>  .I argp
> @@ -408,21 +402,32 @@ field; or the
>  field was zero.
>  .TP
>  .B EINVAL
> -There is no mapping in the specified address range.
> -.TP
> -.B EINVAL
> +The specified address range was invalid.
> +More specifically,
> +no mapping exists in the given range,
> +or the mapping that exists there is invalid
> +(e.g. unsupported type of memory),
> +or the range values (

This produces some unwanted space.  Please apply the following fix to
your patch.

diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
index 6e954e98c..795014794 100644
--- a/man2/ioctl_userfaultfd.2
+++ b/man2/ioctl_userfaultfd.2
@@ -432,11 +432,11 @@ .SS UFFDIO_REGISTER
 no mapping exists in the given range,
 or the mapping that exists there is invalid
 (e.g. unsupported type of memory),
-or the range values (
-.I range.start
+or the range values
+.IR ( range.start
 or
-.I range.len
-) are not multiples of the relevant page size,
+.IR range.len )
+are not multiples of the relevant page size,
 or
 .I range.len
 is zero.

>  .I range.start
>  or
>  .I range.len
> -is not a multiple of the system page size; or,
> +) are not multiples of the relevant page size,
> +or
>  .I range.len
> -is zero; or these fields are otherwise invalid.
> +is zero.
>  .TP
> -.B EINVAL
> -There as an incompatible mapping in the specified address range.
> -.\" Mike Rapoport:
> -.\" ENOMEM if the process is exiting and the
> -.\" mm_struct has gone by the time userfault grabs it.
> +.B ENOMEM
> +The process is exiting,
> +and its address space has already been torn down
> +when userfaultfd attempts to reference it.
> +.TP
> +.B EPERM
> +The userfaultfd would allow writing to a file backing the mapping,
> +but the calling process lacks such write permissions.
> +.TP
> +.B EBUSY
> +A mapping in the specified range is registered with another
> +userfaultfd object.

Why would you move EBUSY to the end?  Do you see any reasons to order it
that way?

Thanks,
Alex

>  .SS UFFDIO_UNREGISTER
>  (Since Linux 4.3.)
>  Unregister a memory address range from userfaultfd.
> -- 
> 2.42.0.609.gbb76f46606-goog
>
Guillem Jover Oct. 17, 2023, 9:23 p.m. UTC | #2 
Hi!

On Mon, 2023-10-09 at 00:06:48 +0200, Alejandro Colomar wrote:
> This produces some unwanted space.  Please apply the following fix to
> your patch.
> 
> diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> index 6e954e98c..795014794 100644
> --- a/man2/ioctl_userfaultfd.2
> +++ b/man2/ioctl_userfaultfd.2
> @@ -432,11 +432,11 @@ .SS UFFDIO_REGISTER
>  no mapping exists in the given range,
>  or the mapping that exists there is invalid
>  (e.g. unsupported type of memory),
> -or the range values (
> -.I range.start
> +or the range values
> +.IR ( range.start

I think you meant «.RI» here?

>  or
> -.I range.len
> -) are not multiples of the relevant page size,
> +.IR range.len )
> +are not multiples of the relevant page size,
>  or
>  .I range.len
>  is zero.
> 

Regards,
Guillem
Alejandro Colomar Oct. 17, 2023, 9:42 p.m. UTC | #3 
Hi Guillem!

On Tue, Oct 17, 2023 at 11:23:10PM +0200, Guillem Jover wrote:
> Hi!
> 
> On Mon, 2023-10-09 at 00:06:48 +0200, Alejandro Colomar wrote:
> > This produces some unwanted space.  Please apply the following fix to
> > your patch.
> > 
> > diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> > index 6e954e98c..795014794 100644
> > --- a/man2/ioctl_userfaultfd.2
> > +++ b/man2/ioctl_userfaultfd.2
> > @@ -432,11 +432,11 @@ .SS UFFDIO_REGISTER
> >  no mapping exists in the given range,
> >  or the mapping that exists there is invalid
> >  (e.g. unsupported type of memory),
> > -or the range values (
> > -.I range.start
> > +or the range values
> > +.IR ( range.start
> 
> I think you meant «.RI» here?

Yup!  Good catch.

Cheers,
Alex

> 
> >  or
> > -.I range.len
> > -) are not multiples of the relevant page size,
> > +.IR range.len )
> > +are not multiples of the relevant page size,
> >  or
> >  .I range.len
> >  is zero.
> > 
> 
> Regards,
> Guillem
Axel Rasmussen Oct. 17, 2023, 10:14 p.m. UTC | #4 
On Tue, Oct 17, 2023 at 2:42 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> Hi Guillem!
>
> On Tue, Oct 17, 2023 at 11:23:10PM +0200, Guillem Jover wrote:
> > Hi!
> >
> > On Mon, 2023-10-09 at 00:06:48 +0200, Alejandro Colomar wrote:
> > > This produces some unwanted space.  Please apply the following fix to
> > > your patch.
> > >
> > > diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> > > index 6e954e98c..795014794 100644
> > > --- a/man2/ioctl_userfaultfd.2
> > > +++ b/man2/ioctl_userfaultfd.2
> > > @@ -432,11 +432,11 @@ .SS UFFDIO_REGISTER
> > >  no mapping exists in the given range,
> > >  or the mapping that exists there is invalid
> > >  (e.g. unsupported type of memory),
> > > -or the range values (
> > > -.I range.start
> > > +or the range values
> > > +.IR ( range.start
> >
> > I think you meant «.RI» here?
>
> Yup!  Good catch.

Thanks, I'll apply this change in a v3.

>
> Cheers,
> Alex
>
> >
> > >  or
> > > -.I range.len
> > > -) are not multiples of the relevant page size,
> > > +.IR range.len )
> > > +are not multiples of the relevant page size,
> > >  or
> > >  .I range.len
> > >  is zero.
> > >
> >
> > Regards,
> > Guillem

Regarding the -EBUSY ordering, I did it this way because that's the
order in which the conditions are checked in the code. But, I agree
that isn't very obvious / useful to any reader of the man page :) and
alphabetical order is preferred. I'll correct that in v3.

>
> --
> <https://www.alejandro-colomar.es/>
diff mbox series

Patch

diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
index 2ee6a0532..95d69f773 100644
--- a/man2/ioctl_userfaultfd.2
+++ b/man2/ioctl_userfaultfd.2
@@ -388,12 +388,6 @@  On error, \-1 is returned and
 .I errno
 is set to indicate the error.
 Possible errors include:
-.\" FIXME Is the following error list correct?
-.\"
-.TP
-.B EBUSY
-A mapping in the specified range is registered with another
-userfaultfd object.
 .TP
 .B EFAULT
 .I argp
@@ -408,21 +402,32 @@  field; or the
 field was zero.
 .TP
 .B EINVAL
-There is no mapping in the specified address range.
-.TP
-.B EINVAL
+The specified address range was invalid.
+More specifically,
+no mapping exists in the given range,
+or the mapping that exists there is invalid
+(e.g. unsupported type of memory),
+or the range values (
 .I range.start
 or
 .I range.len
-is not a multiple of the system page size; or,
+) are not multiples of the relevant page size,
+or
 .I range.len
-is zero; or these fields are otherwise invalid.
+is zero.
 .TP
-.B EINVAL
-There as an incompatible mapping in the specified address range.
-.\" Mike Rapoport:
-.\" ENOMEM if the process is exiting and the
-.\" mm_struct has gone by the time userfault grabs it.
+.B ENOMEM
+The process is exiting,
+and its address space has already been torn down
+when userfaultfd attempts to reference it.
+.TP
+.B EPERM
+The userfaultfd would allow writing to a file backing the mapping,
+but the calling process lacks such write permissions.
+.TP
+.B EBUSY
+A mapping in the specified range is registered with another
+userfaultfd object.
 .SS UFFDIO_UNREGISTER
 (Since Linux 4.3.)
 Unregister a memory address range from userfaultfd.