docs-rst: update infiniband uverbs doc

docs-rst: update infiniband uverbs doc

[Joel Nider]

A small patchset to update the verbs API documentation with some
information regarding the ioctl syscall. First patch converts the
file format to ReST, since this is the new preferred format, moves
the file to Documentation/userspace-api, and updates the index.
The 2nd patch adds the new content, documenting a bit of the internal
workings of the kernel side of the API functions. The goal is to make
it easier for developers unfamiliar with the structure to understand
what is going on when adding a new function.

[PATCH v2 1/2] docs-rst: Convert user verbs doc to rst

[Joel Nider]

Move user_verbs from infiniband to userspace while changing the
format. Replace the existing Documentation/infiniband/user_verbs.txt
with Documentation/userspace-api/user_verbs.rst. No substantial changes
to the content - just some minor reformatting to have the rendering
come out nicely.

Since this documents a userspace API, its home should be with the
other userspace API docs.

This is in preparation for updating the content in a subsequent
patch.

Signed-off-by: Joel Nider <joeln@il.ibm.com>
---
 Documentation/infiniband/user_verbs.txt    | 69 -----------------------------
 Documentation/userspace-api/index.rst      |  1 +
 Documentation/userspace-api/user_verbs.rst | 70 ++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 69 deletions(-)
 delete mode 100644 Documentation/infiniband/user_verbs.txt
 create mode 100644 Documentation/userspace-api/user_verbs.rst

diff --git a/Documentation/infiniband/user_verbs.txt b/Documentation/infiniband/user_verbs.txt
deleted file mode 100644
index df049b9..0000000
--- a/Documentation/infiniband/user_verbs.txt
+++ /dev/null
@@ -1,69 +0,0 @@
-USERSPACE VERBS ACCESS
-
-  The ib_uverbs module, built by enabling CONFIG_INFINIBAND_USER_VERBS,
-  enables direct userspace access to IB hardware via "verbs," as
-  described in chapter 11 of the InfiniBand Architecture Specification.
-
-  To use the verbs, the libibverbs library, available from
-  https://github.com/linux-rdma/rdma-core, is required. libibverbs contains a
-  device-independent API for using the ib_uverbs interface.
-  libibverbs also requires appropriate device-dependent kernel and
-  userspace driver for your InfiniBand hardware.  For example, to use
-  a Mellanox HCA, you will need the ib_mthca kernel module and the
-  libmthca userspace driver be installed.
-
-User-kernel communication
-
-  Userspace communicates with the kernel for slow path, resource
-  management operations via the /dev/infiniband/uverbsN character
-  devices.  Fast path operations are typically performed by writing
-  directly to hardware registers mmap()ed into userspace, with no
-  system call or context switch into the kernel.
-
-  Commands are sent to the kernel via write()s on these device files.
-  The ABI is defined in drivers/infiniband/include/ib_user_verbs.h.
-  The structs for commands that require a response from the kernel
-  contain a 64-bit field used to pass a pointer to an output buffer.
-  Status is returned to userspace as the return value of the write()
-  system call.
-
-Resource management
-
-  Since creation and destruction of all IB resources is done by
-  commands passed through a file descriptor, the kernel can keep track
-  of which resources are attached to a given userspace context.  The
-  ib_uverbs module maintains idr tables that are used to translate
-  between kernel pointers and opaque userspace handles, so that kernel
-  pointers are never exposed to userspace and userspace cannot trick
-  the kernel into following a bogus pointer.
-
-  This also allows the kernel to clean up when a process exits and
-  prevent one process from touching another process's resources.
-
-Memory pinning
-
-  Direct userspace I/O requires that memory regions that are potential
-  I/O targets be kept resident at the same physical address.  The
-  ib_uverbs module manages pinning and unpinning memory regions via
-  get_user_pages() and put_page() calls.  It also accounts for the
-  amount of memory pinned in the process's locked_vm, and checks that
-  unprivileged processes do not exceed their RLIMIT_MEMLOCK limit.
-
-  Pages that are pinned multiple times are counted each time they are
-  pinned, so the value of locked_vm may be an overestimate of the
-  number of pages pinned by a process.
-
-/dev files
-
-  To create the appropriate character device files automatically with
-  udev, a rule like
-
-    KERNEL=="uverbs*", NAME="infiniband/%k"
-
-  can be used.  This will create device nodes named
-
-    /dev/infiniband/uverbs0
-
-  and so on.  Since the InfiniBand userspace verbs should be safe for
-  use by non-privileged processes, it may be useful to add an
-  appropriate MODE or GROUP to the udev rule.
diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
index a3233da..c37bbbe 100644
--- a/Documentation/userspace-api/index.rst
+++ b/Documentation/userspace-api/index.rst
@@ -20,6 +20,7 @@ place where this information is gathered.
    seccomp_filter
    unshare
    spec_ctrl
+   user_verbs
 
 .. only::  subproject and html
 
diff --git a/Documentation/userspace-api/user_verbs.rst b/Documentation/userspace-api/user_verbs.rst
new file mode 100644
index 0000000..ffc4aec
--- /dev/null
+++ b/Documentation/userspace-api/user_verbs.rst
@@ -0,0 +1,70 @@
+======================
+Userspace Verbs Access
+======================
+The ib_uverbs module, built by enabling CONFIG_INFINIBAND_USER_VERBS,
+enables direct userspace access to IB hardware via "verbs," as
+described in chapter 11 of the InfiniBand Architecture Specification.
+
+To use the verbs, the libibverbs library, available from
+https://github.com/linux-rdma/rdma-core, is required. libibverbs contains a
+device-independent API for using the ib_uverbs interface.
+libibverbs also requires appropriate device-dependent kernel and
+userspace driver for your InfiniBand hardware.  For example, to use
+a Mellanox HCA, you will need the ib_mthca kernel module and the
+libmthca userspace driver be installed.
+
+User-kernel communication
+=========================
+Userspace communicates with the kernel for slow path, resource
+management operations via the /dev/infiniband/uverbsN character
+devices.  Fast path operations are typically performed by writing
+directly to hardware registers mmap()ed into userspace, with no
+system call or context switch into the kernel.
+
+Commands are sent to the kernel via write()s on these device files.
+The ABI is defined in drivers/infiniband/include/ib_user_verbs.h.
+The structs for commands that require a response from the kernel
+contain a 64-bit field used to pass a pointer to an output buffer.
+Status is returned to userspace as the return value of the write()
+system call.
+
+Resource management
+===================
+Since creation and destruction of all IB resources is done by
+commands passed through a file descriptor, the kernel can keep track
+of which resources are attached to a given userspace context.  The
+ib_uverbs module maintains idr tables that are used to translate
+between kernel pointers and opaque userspace handles, so that kernel
+pointers are never exposed to userspace and userspace cannot trick
+the kernel into following a bogus pointer.
+
+This also allows the kernel to clean up when a process exits and
+prevent one process from touching another process's resources.
+
+Memory pinning
+==============
+Direct userspace I/O requires that memory regions that are potential
+I/O targets be kept resident at the same physical address.  The
+ib_uverbs module manages pinning and unpinning memory regions via
+get_user_pages() and put_page() calls.  It also accounts for the
+amount of memory pinned in the process's locked_vm, and checks that
+unprivileged processes do not exceed their RLIMIT_MEMLOCK limit.
+
+Pages that are pinned multiple times are counted each time they are
+pinned, so the value of locked_vm may be an overestimate of the
+number of pages pinned by a process.
+
+/dev files
+==========
+To create the appropriate character device files automatically with
+udev, a rule like::
+
+   KERNEL=="uverbs*", NAME="infiniband/%k"
+
+can be used.  This will create device nodes named::
+
+    /dev/infiniband/uverbs0
+
+and so on.  Since the InfiniBand userspace verbs should be safe for
+use by non-privileged processes, it may be useful to add an
+appropriate MODE or GROUP to the udev rule.
-- 
2.7.4

Re: [PATCH v2 1/2] docs-rst: Convert user verbs doc to rst

[Jason Gunthorpe]

On Tue, Jan 15, 2019 at 12:26:30PM +0200, Joel Nider wrote:
> Move user_verbs from infiniband to userspace while changing the
> format. Replace the existing Documentation/infiniband/user_verbs.txt
> with Documentation/userspace-api/user_verbs.rst. No substantial changes
> to the content - just some minor reformatting to have the rendering
> come out nicely.
> 
> Since this documents a userspace API, its home should be with the
> other userspace API docs.
> 
> This is in preparation for updating the content in a subsequent
> patch.
> 
> Signed-off-by: Joel Nider <joeln@il.ibm.com>
> ---
>  Documentation/infiniband/user_verbs.txt    | 69 -----------------------------
>  Documentation/userspace-api/index.rst      |  1 +
>  Documentation/userspace-api/user_verbs.rst | 70 ++++++++++++++++++++++++++++++
>  3 files changed, 71 insertions(+), 69 deletions(-)
>  delete mode 100644 Documentation/infiniband/user_verbs.txt
>  create mode 100644 Documentation/userspace-api/user_verbs.rst

Need to update MAINTAINERS for the new file.. Maybe call this
rdma_user_verbs.rst as we could also have rdma_umad, rdma_cm, etc
 
Jason

Re: [PATCH v2 1/2] docs-rst: Convert user verbs doc to rst

[Jonathan Corbet]

On Tue, 15 Jan 2019 08:14:02 -0700
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> > Move user_verbs from infiniband to userspace while changing the
> > format. Replace the existing Documentation/infiniband/user_verbs.txt
> > with Documentation/userspace-api/user_verbs.rst. No substantial changes
> > to the content - just some minor reformatting to have the rendering
> > come out nicely.
> > 
> > Since this documents a userspace API, its home should be with the
> > other userspace API docs.
> > 
> > This is in preparation for updating the content in a subsequent
> > patch.
> > 
> > Signed-off-by: Joel Nider <joeln@il.ibm.com>
> > ---
> >  Documentation/infiniband/user_verbs.txt    | 69 -----------------------------
> >  Documentation/userspace-api/index.rst      |  1 +
> >  Documentation/userspace-api/user_verbs.rst | 70 ++++++++++++++++++++++++++++++
> >  3 files changed, 71 insertions(+), 69 deletions(-)
> >  delete mode 100644 Documentation/infiniband/user_verbs.txt
> >  create mode 100644 Documentation/userspace-api/user_verbs.rst  
> 
> Need to update MAINTAINERS for the new file.. Maybe call this
> rdma_user_verbs.rst as we could also have rdma_umad, rdma_cm, etc

Both of those suggestions make sense.  Joel, can you do a quick respin
along those lines?  Then I think this one is ready.

Thanks,

jon

Re: [PATCH v2 1/2] docs-rst: Convert user verbs doc to rst

[Joel Nider]

Jonathan Corbet <corbet@lwn.net> wrote on 01/15/2019 08:02:18 PM:

> From: Jonathan Corbet <corbet@lwn.net>
> To: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Joel Nider <joeln@il.ibm.com>, Leon Romanovsky <leon@kernel.org>, 
Doug 
> Ledford <dledford@redhat.com>, Mike Rapoport <rppt@linux.ibm.com>, 
linux-
> doc@vger.kernel.org, linux-kernel@vger.kernel.org
> Date: 01/15/2019 08:14 PM
> Subject: Re: [PATCH v2 1/2] docs-rst: Convert user verbs doc to rst
> 
> On Tue, 15 Jan 2019 08:14:02 -0700
> Jason Gunthorpe <jgg@ziepe.ca> wrote:
> 
> > > Move user_verbs from infiniband to userspace while changing the
> > > format. Replace the existing Documentation/infiniband/user_verbs.txt
> > > with Documentation/userspace-api/user_verbs.rst. No substantial 
changes
> > > to the content - just some minor reformatting to have the rendering
> > > come out nicely.
> > > 
> > > Since this documents a userspace API, its home should be with the
> > > other userspace API docs.
> > > 
> > > This is in preparation for updating the content in a subsequent
> > > patch.
> > > 
> > > Signed-off-by: Joel Nider <joeln@il.ibm.com>
> > > ---
> > >  Documentation/infiniband/user_verbs.txt    | 69 
-----------------------------
> > >  Documentation/userspace-api/index.rst      |  1 +
> > >  Documentation/userspace-api/user_verbs.rst | 70 
++++++++++++++++++++++++++++++
> > >  3 files changed, 71 insertions(+), 69 deletions(-)
> > >  delete mode 100644 Documentation/infiniband/user_verbs.txt
> > >  create mode 100644 Documentation/userspace-api/user_verbs.rst 
> > 
> > Need to update MAINTAINERS for the new file.. Maybe call this
> > rdma_user_verbs.rst as we could also have rdma_umad, rdma_cm, etc
> 
> Both of those suggestions make sense.  Joel, can you do a quick respin
> along those lines?  Then I think this one is ready.

Yes, if I understood your other message correctly (I'll wait to see your 
response on the other thread), I can make these two changes.

> Thanks,
> 
> jon
> 

[PATCH v2 2/2] docs-rst: userspace: update verbs API details

[Joel Nider]

It is important to understand the existing framework when implementing
a new verb. The majority of existing API functions are implemented using
the write syscall, but this has been superceded by the ioctl syscall
for new commands. This patch updates the documentation regarding how
to go about implementing a new verb, focusing on the new ioctl
interface.

The documentation is far from complete, but this is a good step in the
right direction. Future patches can add more detail according to need.
Also, the interface is still undergoing substantial changes so an
effort was made to document only the stable parts so as to avoid
incorrect information since documentation changes tend to lag behind
code changes.

Signed-off-by: Joel Nider <joeln@il.ibm.com>
---
 Documentation/userspace-api/user_verbs.rst | 69 +++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/user_verbs.rst b/Documentation/userspace-api/user_verbs.rst
index ffc4aec..f0c7cd3 100644
--- a/Documentation/userspace-api/user_verbs.rst
+++ b/Documentation/userspace-api/user_verbs.rst
@@ -21,12 +21,79 @@ devices.  Fast path operations are typically performed by writing
 directly to hardware registers mmap()ed into userspace, with no
 system call or context switch into the kernel.
 
-Commands are sent to the kernel via write()s on these device files.
+There are currently two methods for executing commands in the kernel: write() and ioctl().
+Older commands are sent to the kernel via write()s on the device files
+mentioned earlier. New commands must use the ioctl() method. For completeness,
+both mechanisms are described here.
+
+The interface between userspace and kernel is kept in sync by checking the
+version number. In the kernel, it is defined by IB_USER_VERBS_ABI_VERSION
+(in include/uapi/rdma/ib_user_verbs.h).
+
+Write system call
+-----------------
 The ABI is defined in drivers/infiniband/include/ib_user_verbs.h.
 The structs for commands that require a response from the kernel
 contain a 64-bit field used to pass a pointer to an output buffer.
 Status is returned to userspace as the return value of the write()
 system call.
+The entry point to the kernel is the ib_uverbs_write() function, which is
+invoked as a response to the 'write' system call. The requested function is
+looked up from an array called uverbs_cmd_table which contains function pointers
+to the various command handlers.
+
+Write Command Handlers
+~~~~~~~~~~~~~~~~~~~~~~
+These command handler functions are declared
+with the IB_VERBS_DECLARE_CMD macro in drivers/infiniband/core/uverbs.h. There
+are also extended commands, which are kept in a similar manner in the
+uverbs_ex_cmd_table. The extended commands use 64-bit values in the command
+header, as opposed to the 32-bit values used in the regular command table.
+
+
+Ioctl system call
+-----------------
+The entry point for the 'ioctl' system call is the ib_uverbs_ioctl() function.
+Unlike write(), ioctl() accepts a 'cmd' parameter, which must have the value
+defined by RDMA_VERBS_IOCTL. More documentation regarding the ioctl numbering
+scheme can be found in: Documentation/ioctl/ioctl-number.txt. The
+command-specific information is passed as a pointer in the 'arg' parameter,
+which is cast as a 'struct ib_uverbs_ioctl_hdr*'.
+
+The way command handler functions (methods) are looked up is more complicated
+than the array index used for write(). Here, the ib_uverbs_cmd_verbs() function
+uses a radix tree to search for the correct command handler. If the lookup
+succeeds, the method is invoked by ib_uverbs_run_method().
+
+Ioctl Command Handlers
+~~~~~~~~~~~~~~~~~~~~~~
+Command handlers (also known as 'methods') for ioctl are declared with the
+UVERBS_HANDLER macro. The handler is registered for use by the
+DECLARE_UVERBS_NAMED_METHOD macro, which binds the name of the handler with its
+attributes. By convention, the methods are implemented in files named with the
+prefix 'uverbs_std_types_'.
+
+Each method can accept a set of parameters called attributes. There are 6
+types of attributes: idr, fd, pointer, enum, const and flags. The idr attribute
+declares an indirect (translated) handle for the method, and
+specifies the object that the method will act upon. The first attribute should
+be a handle to the uobj (ib_uobject) which contains private data. There may be
+0 or more
+additional attributes, including other handles. The 'pointer' attribute must be
+specified as 'in' or 'out', depending on if it is an input from userspace, or
+meant to return a value to userspace.
+
+The method also needs to be bound to an object, which is done with the
+DECLARE_UVERBS_NAMED_OBJECT macro. This macro takes a variable
+number of methods and stores them in an array attached to the object.
+
+Objects are declared using DECLARE_UVERBS_NAMED_OBJECT macro. Most of the
+objects (including pd, mw, cq, etc.) are defined in uverbs_std_types.c,
+and the remaining objects are declared in files that are prefixed with the
+name 'uverbs_std_types_'.
+
+Objects trees are declared using the DECLARE_UVERBS_OBJECT_TREE macro. This
+combines all of the objects.
 
 Resource management
 ===================
-- 
2.7.4

Re: [PATCH v2 2/2] docs-rst: userspace: update verbs API details

[Matthew Wilcox]

On Tue, Jan 15, 2019 at 12:26:31PM +0200, Joel Nider wrote:
> It is important to understand the existing framework when implementing
> a new verb. The majority of existing API functions are implemented using
> the write syscall, but this has been superceded by the ioctl syscall
> for new commands. This patch updates the documentation regarding how
> to go about implementing a new verb, focusing on the new ioctl
> interface.
> 
> The documentation is far from complete, but this is a good step in the
> right direction. Future patches can add more detail according to need.
> Also, the interface is still undergoing substantial changes so an
> effort was made to document only the stable parts so as to avoid
> incorrect information since documentation changes tend to lag behind
> code changes.

I think this is a horrible direction to take.  The current document is
clearly for _users_.  All this documentation you've added is for kernel
hackers.  It needs to go in a different file, or not be added at all.

Re: [PATCH v2 2/2] docs-rst: userspace: update verbs API details

[Joel Nider]

Matthew Wilcox <willy@infradead.org> wrote on 01/15/2019 05:31:28 PM:

> From: Matthew Wilcox <willy@infradead.org>
> To: Joel Nider <joeln@il.ibm.com>
> Cc: Jonathan Corbet <corbet@lwn.net>, Jason Gunthorpe <jgg@ziepe.ca>, 
Leon 
> Romanovsky <leon@kernel.org>, Doug Ledford <dledford@redhat.com>, Mike 
> Rapoport <rppt@linux.ibm.com>, linux-doc@vger.kernel.org, 
linux-kernel@vger.kernel.org
> Date: 01/15/2019 05:31 PM
> Subject: Re: [PATCH v2 2/2] docs-rst: userspace: update verbs API 
details
> 
> On Tue, Jan 15, 2019 at 12:26:31PM +0200, Joel Nider wrote:
> > It is important to understand the existing framework when implementing
> > a new verb. The majority of existing API functions are implemented 
using
> > the write syscall, but this has been superceded by the ioctl syscall
> > for new commands. This patch updates the documentation regarding how
> > to go about implementing a new verb, focusing on the new ioctl
> > interface.
> > 
> > The documentation is far from complete, but this is a good step in the
> > right direction. Future patches can add more detail according to need.
> > Also, the interface is still undergoing substantial changes so an
> > effort was made to document only the stable parts so as to avoid
> > incorrect information since documentation changes tend to lag behind
> > code changes.
> 
> I think this is a horrible direction to take.  The current document is
> clearly for _users_.  All this documentation you've added is for kernel
> hackers.  It needs to go in a different file, or not be added at all.
> 
Hmm, that's a bit heavy-handed in my opinion. If you look at what is 
currently under "Userspace API", there are a total of 4 files - not 
exactly what I would call comprehensive documentation of the Linux kernel 
interface. So the option of not adding more documentation simply because 
it is in the wrong section seems a bit defeatist (I think we can all agree 
more kernel docs is a good thing). So let's assume for the moment that it 
_should_ be added, and that we are discussing _where_. Yes, the document I 
am modifying has a bit of a mash of pure userspace API - functions and 
details that application developers must know - and the lower level 
details that are more useful for someone who wishes to extend this 
interface. The existing documentation (specifically unshare) also suffers 
from the same problem. There is a section (7) called "Low Level Design" 
which documents very much the same level of detail as the Infiniband doc, 
so this seems to be at least consistent.
I think there is a deeper issue - as an application developer, I would 
only look at this kernel documentation as a last resort. #1 is the 'man' 
pages, #2 is web search for an example (I know for many it's the other way 
around). So who really looks at this documentation? I say the vast 
majority is kernel hackers. So yes, this is about the user-facing API, but 
not from the application writer's perspective - it should be about how to 
create a consistent API for users, from the kernel hacker's perspective.

Joel

Re: [PATCH v2 2/2] docs-rst: userspace: update verbs API details

[Jonathan Corbet]

On Tue, 15 Jan 2019 18:29:59 +0200
"Joel Nider" <JOELN@il.ibm.com> wrote:

> > I think this is a horrible direction to take.  The current document is
> > clearly for _users_.  All this documentation you've added is for kernel
> > hackers.  It needs to go in a different file, or not be added at all.
> >   
> Hmm, that's a bit heavy-handed in my opinion. If you look at what is 
> currently under "Userspace API", there are a total of 4 files - not 
> exactly what I would call comprehensive documentation of the Linux kernel 
> interface. 

One has to start somewhere - I'm glad to see you adding to it.

> So the option of not adding more documentation simply because 
> it is in the wrong section seems a bit defeatist (I think we can all agree 
> more kernel docs is a good thing). So let's assume for the moment that it 
> _should_ be added, and that we are discussing _where_.

I think we definitely want to add it.

> Yes, the document I 
> am modifying has a bit of a mash of pure userspace API - functions and 
> details that application developers must know - and the lower level 
> details that are more useful for someone who wishes to extend this 
> interface. The existing documentation (specifically unshare) also suffers 
> from the same problem. There is a section (7) called "Low Level Design" 
> which documents very much the same level of detail as the Infiniband doc, 
> so this seems to be at least consistent.
> I think there is a deeper issue - as an application developer, I would 
> only look at this kernel documentation as a last resort. #1 is the 'man' 
> pages, #2 is web search for an example (I know for many it's the other way 
> around). So who really looks at this documentation? I say the vast 
> majority is kernel hackers. So yes, this is about the user-facing API, but 
> not from the application writer's perspective - it should be about how to 
> create a consistent API for users, from the kernel hacker's perspective.

The intent behind the user-space API manual is to document the user-space
API; it's meant to be read by people writing applications and such.
Perhaps they find it with a web search, but it will be there for them, one
hopes, when they want it.

The project of reworking the kernel documentation to be more comprehensive
and more focused on its readers, rather than, as Rob Landley once put it,
"a gigantic mess, currently organized based on where random passers-by put
things down last" is still in an early stage.  But that doesn't mean that
we shouldn't try to always move in the right direction.  So I agree with
Willy that this particular text is better suited to the driver-API
manual.  Even if a bare bit of information on its own there for now, it's
a starting place.  Can I ask you to do that, please?

Thanks,

jon

Re: [PATCH v2 2/2] docs-rst: userspace: update verbs API details

[Joel Nider]

Jonathan Corbet <corbet@lwn.net> wrote on 01/15/2019 08:08:54 PM:

> From: Jonathan Corbet <corbet@lwn.net>
> To: "Joel Nider" <JOELN@il.ibm.com>
> Cc: Matthew Wilcox <willy@infradead.org>, Doug Ledford 
<dledford@redhat.com>,
> Jason Gunthorpe <jgg@ziepe.ca>, Leon Romanovsky <leon@kernel.org>, 
linux-
> doc@vger.kernel.org, linux-kernel@vger.kernel.org, Mike Rapoport 
<rppt@linux.ibm.com>
> Date: 01/15/2019 08:09 PM
> Subject: Re: [PATCH v2 2/2] docs-rst: userspace: update verbs API 
details
> 
> On Tue, 15 Jan 2019 18:29:59 +0200
> "Joel Nider" <JOELN@il.ibm.com> wrote:
> 
> > > I think this is a horrible direction to take.  The current document 
is
> > > clearly for _users_.  All this documentation you've added is for 
kernel
> > > hackers.  It needs to go in a different file, or not be added at 
all.
> > > 
> > Hmm, that's a bit heavy-handed in my opinion. If you look at what is 
> > currently under "Userspace API", there are a total of 4 files - not 
> > exactly what I would call comprehensive documentation of the Linux 
kernel 
> > interface. 
> 
> One has to start somewhere - I'm glad to see you adding to it.
> 
> > So the option of not adding more documentation simply because 
> > it is in the wrong section seems a bit defeatist (I think we can all 
agree 
> > more kernel docs is a good thing). So let's assume for the moment that 
it 
> > _should_ be added, and that we are discussing _where_.
> 
> I think we definitely want to add it.
> 
> > Yes, the document I 
> > am modifying has a bit of a mash of pure userspace API - functions and 

> > details that application developers must know - and the lower level 
> > details that are more useful for someone who wishes to extend this 
> > interface. The existing documentation (specifically unshare) also 
suffers 
> > from the same problem. There is a section (7) called "Low Level 
Design" 
> > which documents very much the same level of detail as the Infiniband 
doc, 
> > so this seems to be at least consistent.
> > I think there is a deeper issue - as an application developer, I would 

> > only look at this kernel documentation as a last resort. #1 is the 
'man' 
> > pages, #2 is web search for an example (I know for many it's the other 
way 
> > around). So who really looks at this documentation? I say the vast 
> > majority is kernel hackers. So yes, this is about the user-facing API, 
but 
> > not from the application writer's perspective - it should be about how 
to 
> > create a consistent API for users, from the kernel hacker's 
perspective.
> 
> The intent behind the user-space API manual is to document the 
user-space
> API; it's meant to be read by people writing applications and such.
> Perhaps they find it with a web search, but it will be there for them, 
one
> hopes, when they want it.

So is the intent then to duplicate what is already in 'man'? Why would we 
want 2 different locations for basically the same information? Wouldn't it 
make more sense to just put these few details into appropriate 'man' 
pages?

> The project of reworking the kernel documentation to be more 
comprehensive
> and more focused on its readers, rather than, as Rob Landley once put 
it,
> "a gigantic mess, currently organized based on where random passers-by 
put
> things down last" is still in an early stage.  But that doesn't mean 
that
> we shouldn't try to always move in the right direction. 

It looks like I misunderstood the purpose of the "userspace API" section 
then. So is there a section that is meant for a guide on how to write an 
interface function?

> So I agree with
> Willy that this particular text is better suited to the driver-API
> manual.  Even if a bare bit of information on its own there for now, 
it's
> a starting place.  Can I ask you to do that, please?

OK, just to be clear - you are asking me to leave the original file as-is 
(well, after .rst conversion) but move it to the new location 
(Documentation/userspace-api), and put my new content into a new file in 
the old location (Documentation/infiniband)?
 
> Thanks,
> 
> jon
> 

Re: [PATCH v2 2/2] docs-rst: userspace: update verbs API details

[Jonathan Corbet]

On Tue, 15 Jan 2019 22:37:01 +0200
"Joel Nider" <JOELN@il.ibm.com> wrote:

> Jonathan Corbet <corbet@lwn.net> wrote on 01/15/2019 08:08:54 PM:
> > The intent behind the user-space API manual is to document the user-space
> > API; it's meant to be read by people writing applications and such.
> > Perhaps they find it with a web search, but it will be there for them, one
> > hopes, when they want it.  
> 
> So is the intent then to duplicate what is already in 'man'? Why would we 
> want 2 different locations for basically the same information? Wouldn't it 
> make more sense to just put these few details into appropriate 'man' 
> pages?

We are not attempting to duplicate the man pages; there's been occasional
talk of bringing them into the kernel tree, but enthusiasm for that is
scarce for a number of good reasons.  But there's a lot of information
about the user-space API that isn't in the man pages, including the piece
you are converting to RST.

For a huge example, look at the massive v4l2 UAPI documentation that's in
the kernel; that will never really fit into the man-page model.  (And yes,
it belongs in the userspace-api directory but isn't there for $REASONS).

> It looks like I misunderstood the purpose of the "userspace API" section 
> then. So is there a section that is meant for a guide on how to write an 
> interface function?

For something like this, driver-api seems like the right place.  Someday,
in some glorious future, it could contain a full manual on how these
drivers are written, using all of the nice kerneldoc comments that already
exist under drivers/infiniband.

> OK, just to be clear - you are asking me to leave the original file as-is 
> (well, after .rst conversion) but move it to the new location 
> (Documentation/userspace-api), and put my new content into a new file in 
> the old location (Documentation/infiniband)?

I'd rather see you put the new stuff under Documentation/driver-api, either
in a standalone file or in a new subdirectory.

Thanks for your patience with this!  We really do want people to contribute
more documentation, and I feel bad when it seems like I'm making it
harder.  My hope is that it will pay off in the long run; I think there are
signs already that this is happening.

Thanks,

jon

Re: [PATCH v2 2/2] docs-rst: userspace: update verbs API details

[Mike Rapoport]

Hi Jon,

On Tue, Jan 15, 2019 at 02:38:59PM -0700, Jonathan Corbet wrote:
> On Tue, 15 Jan 2019 22:37:01 +0200
> 
> I'd rather see you put the new stuff under Documentation/driver-api, either
> in a standalone file or in a new subdirectory.
> 
> Thanks for your patience with this!  We really do want people to contribute
> more documentation, and I feel bad when it seems like I'm making it
> harder.  My hope is that it will pay off in the long run; I think there are
> signs already that this is happening.

I believe its worth adding Documentation/doc-guide/where-to-put-stuff.rst
with at least basic guidelines for the desired organization of
Documentation/.

> Thanks,
> 
> jon
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 2/2] docs-rst: userspace: update verbs API details

[Joel Nider]

Jonathan Corbet <corbet@lwn.net> wrote on 01/15/2019 11:38:59 PM:

> We are not attempting to duplicate the man pages; there's been 
occasional
> talk of bringing them into the kernel tree, but enthusiasm for that is
> scarce for a number of good reasons.  But there's a lot of information
> about the user-space API that isn't in the man pages, including the 
piece
> you are converting to RST.

My next task is to update the man pages for rdma-core since I touched 
libibverbs as well (the _other_ side of this API). So I just want to make 
sure everything is documented exactly once (no more, no less) and in the 
correct place.

> For something like this, driver-api seems like the right place. Someday,
> in some glorious future, it could contain a full manual on how these
> drivers are written, using all of the nice kerneldoc comments that 
already
> exist under drivers/infiniband.
> 
> > OK, just to be clear - you are asking me to leave the original file 
as-is 
> > (well, after .rst conversion) but move it to the new location 
> > (Documentation/userspace-api), and put my new content into a new file 
in 
> > the old location (Documentation/infiniband)?
> 
> I'd rather see you put the new stuff under Documentation/driver-api, 
either
> in a standalone file or in a new subdirectory.

I took a quick look at what is already there, and I don't see how the RDMA 
stuff fits. It looks like the majority is about various busses (SPI, PCI, 
I2C, USB, etc) or other general platform support (clk, edac) that I would 
use when writing a driver. My changes are very infiniband-specific, and 
user-facing and don't really seem to fit in with the aforementioned 
modules. It seems to me that if it does not belong in userspace-api, then 
leaving it where it is is the best choice.

Regards,
Joel