Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Mon, 9 Oct 2023 11:49:11 +0300
From:   Mike Rapoport <rppt@kernel.org>
To:     Alejandro Colomar <alx@kernel.org>
Cc:     Axel Rasmussen <axelrasmussen@google.com>,
        Peter Xu <peterx@redhat.com>, linux-man@vger.kernel.org,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 07/10] ioctl_userfaultfd.2: correct and update UFFDIO_API
 ioctl error codes
Message-ID: <20231009084911.GK3303@kernel.org>
References: <20230919190206.388896-1-axelrasmussen@google.com>
 <20230919190206.388896-8-axelrasmussen@google.com>
 <5ddepg57wqnidtvsio2pse44dot6pvr3rcmhwld6ml3sflwcz3@ijd3h4teqblr>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5ddepg57wqnidtvsio2pse44dot6pvr3rcmhwld6ml3sflwcz3@ijd3h4teqblr>
Precedence: bulk

On Tue, Sep 26, 2023 at 01:52:34AM +0200, Alejandro Colomar wrote:
> Hi Axel,
> 
> On Tue, Sep 19, 2023 at 12:02:03PM -0700, Axel Rasmussen wrote:
> > First, it is not correct that repeated UFFDIO_API calls result in
> > EINVAL. This is true *if both calls enable features*, but in the case
> > where we're doing a two-step feature detection handshake, the kernel
> > explicitly expects 2 calls (one with no features set). So, correct this
> > description.
> > 
> > Then, some new error cases have been added to the kernel recently, and
> > the man page wasn't updated to note these. So, add in descriptions of
> > these new error cases.
> > 
> > Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
> > ---
> >  man2/ioctl_userfaultfd.2 | 24 +++++++++++++++++++-----
> >  1 file changed, 19 insertions(+), 5 deletions(-)
> > 
> > diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> > index 53b1f473f..1aa9654be 100644
> > --- a/man2/ioctl_userfaultfd.2
> > +++ b/man2/ioctl_userfaultfd.2
> > @@ -280,17 +280,31 @@ refers to an address that is outside the calling process's
> >  accessible address space.
> >  .TP
> >  .B EINVAL
> > -The userfaultfd has already been enabled by a previous
> > -.B UFFDIO_API
> > -operation.
> > -.TP
> > -.B EINVAL
> >  The API version requested in the
> >  .I api
> >  field is not supported by this kernel, or the
> >  .I features
> >  field passed to the kernel includes feature bits that are not supported
> >  by the current kernel version.
> > +.TP
> > +.B EPERM
> 
> This EPERM should probably be at the end.  Unless you have a good reason
> to break alphabetic order.

I agree with Alex here, other than that feel free to add

Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org>

 
> Thanks,
> Alex
> 
> > +The
> > +.B UFFD_FEATURE_EVENT_FORK
> > +feature was enabled,
> > +but the calling process doesn't have the
> > +.B CAP_SYS_PTRACE
> > +capability.
> > +.TP
> > +.B EINVAL
> > +A previous
> > +.B UFFDIO_API
> > +call already enabled one or more features for this userfaultfd.
> > +Calling
> > +.B UFFDIO_API
> > +twice,
> > +the first time with no features set,
> > +is explicitly allowed
> > +as per the two-step feature detection handshake.
> >  .\" FIXME In the above error case, the returned 'uffdio_api' structure is
> >  .\" zeroed out. Why is this done? This should be explained in the manual page.
> >  .\"
> > -- 
> > 2.42.0.459.ge4e396fd5e-goog
> > 


-- 
Sincerely yours,
Mike.