2019-01-15 10:28:08

by Joel Nider

[permalink] [raw]
Subject: docs-rst: update infiniband uverbs doc

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.



2019-01-15 10:29:13

by Joel Nider

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

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 <[email protected]>
---
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


2019-01-15 10:30:05

by Joel Nider

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

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 <[email protected]>
---
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


2019-01-15 17:00:26

by Joel Nider

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

Matthew Wilcox <[email protected]> wrote on 01/15/2019 05:31:28 PM:

> From: Matthew Wilcox <[email protected]>
> To: Joel Nider <[email protected]>
> Cc: Jonathan Corbet <[email protected]>, Jason Gunthorpe <[email protected]>,
Leon
> Romanovsky <[email protected]>, Doug Ledford <[email protected]>, Mike
> Rapoport <[email protected]>, [email protected],
[email protected]
> 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


2019-01-15 17:37:59

by Jason Gunthorpe

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

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 <[email protected]>
> ---
> 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

2019-01-15 17:52:27

by Matthew Wilcox

[permalink] [raw]
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.


2019-01-16 06:32:52

by Jonathan Corbet

[permalink] [raw]
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 <[email protected]> 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 <[email protected]>
> > ---
> > 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

2019-01-16 06:43:45

by Jonathan Corbet

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

On Tue, 15 Jan 2019 18:29:59 +0200
"Joel Nider" <[email protected]> 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

2019-01-16 11:14:40

by Joel Nider

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

Jonathan Corbet <[email protected]> wrote on 01/15/2019 08:08:54 PM:

> From: Jonathan Corbet <[email protected]>
> To: "Joel Nider" <[email protected]>
> Cc: Matthew Wilcox <[email protected]>, Doug Ledford
<[email protected]>,
> Jason Gunthorpe <[email protected]>, Leon Romanovsky <[email protected]>,
linux-
> [email protected], [email protected], Mike Rapoport
<[email protected]>
> 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" <[email protected]> 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
>



2019-01-16 11:15:29

by Joel Nider

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

Jonathan Corbet <[email protected]> wrote on 01/15/2019 08:02:18 PM:

> From: Jonathan Corbet <[email protected]>
> To: Jason Gunthorpe <[email protected]>
> Cc: Joel Nider <[email protected]>, Leon Romanovsky <[email protected]>,
Doug
> Ledford <[email protected]>, Mike Rapoport <[email protected]>,
linux-
> [email protected], [email protected]
> 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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
>



2019-01-16 13:37:00

by Jonathan Corbet

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

On Tue, 15 Jan 2019 22:37:01 +0200
"Joel Nider" <[email protected]> wrote:

> Jonathan Corbet <[email protected]> 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

2019-01-17 01:23:42

by Mike Rapoport

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

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.


2019-01-17 11:31:50

by Joel Nider

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

Jonathan Corbet <[email protected]> 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