[PATCH] Documentation: Reference kernel-doc for container_of

[PATCH] Documentation: Reference kernel-doc for container_of

[René Nyffenegger]

The file include/linux/container_of.h contained kernel-doc but was not
referenced in any .rst file. In addition, the file
Documentation/core-api/kobject.rst wrongly located the definition
of the macro `container_of` in linux/kernel.h while in reality
it is defined in linux/container_of.h

This patch adds a new .rst file that includes the kernel-doc of
container_of.h and rectifies the wrong reference of the header
file.

Signed-off-by: René Nyffenegger <mail@renenyffenegger.ch>
---
 Documentation/core-api/container_of.rst | 64 +++++++++++++++++++++++++
 Documentation/core-api/index.rst        |  1 +
 Documentation/core-api/kobject.rst      | 58 +++++++++++-----------
 3 files changed, 93 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/core-api/container_of.rst

diff --git a/Documentation/core-api/container_of.rst b/Documentation/core-api/container_of.rst
new file mode 100644
index 000000000000..f063d3f9e536
--- /dev/null
+++ b/Documentation/core-api/container_of.rst
@@ -0,0 +1,64 @@
+=====================================================================================
+Given a pointer to a member of a struct, returning a pointer to the containing struct
+=====================================================================================
+
+.. _container_of:
+
+Overview
+========
+
+The two macros ``container_of`` and ``container_of_const``, defined in ``<include/linux/container_of.h>``, return a pointer to the ``struct`` (i. e. the *container*) from a pointer to a member of this ``struct``.
+
+These macros might be used when :ref:`embedding kobjects<embedding_kobjects>`, but see also :ref:`usage<container_of_usage>`.
+
+
+.. kernel-doc:: include/linux/container_of.h
+
+Usage
+=====
+
+.. _container_of_usage:
+
+The following simple code demonstrates the usage of ``container_of``
+
+.. code-block:: c
+
+ struct inner_struct
+ {
+ 	int abc;
+ 	int def;
+ }
+ 
+ struct container_struct
+ {
+ 	struct inner_struct inner;
+ 	char               *member_xyz;
+ 	int                 member_val;
+ };
+ 
+ static void f(struct inner_struct *inr)
+ {
+ 	struct container_struct *cont;
+ 
+ 	cont = container_of(inr, struct container_struct, inner);
+ 
+        /* Use cont and its members */
+ }
+ 
+ static void g(char** member)
+ {
+ 	struct container_struct *cont;
+ 
+ 	cont = container_of(member, struct container_struct, member_xyz);
+        /* Use cont and its members */
+ }
+ 
+ void somewhere()
+ {
+ 	struct container_struct cont;
+ 
+ 	/* Initialize cont */
+ 
+ 	f(& cont.inner      );
+ 	g(& cont.member_xyz );
+ }
diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
index 7a3a08d81f11..595af47d0d5f 100644
--- a/Documentation/core-api/index.rst
+++ b/Documentation/core-api/index.rst
@@ -37,6 +37,7 @@ Library functionality that is used throughout the kernel.
    kref
    assoc_array
    xarray
+   container_of
    maple_tree
    idr
    circular-buffers
diff --git a/Documentation/core-api/kobject.rst b/Documentation/core-api/kobject.rst
index 7310247310a0..058570694418 100644
--- a/Documentation/core-api/kobject.rst
+++ b/Documentation/core-api/kobject.rst
@@ -49,6 +49,8 @@ approach will be taken, so we'll go back to kobjects.
 Embedding kobjects
 ==================
 
+.. _embedding_kobjects:
+
 It is rare for kernel code to create a standalone kobject, with one major
 exception explained below.  Instead, kobjects are used to control access to
 a larger, domain-specific object.  To this end, kobjects will be found
@@ -66,50 +68,46 @@ their own, but are invariably found embedded in the larger objects of
 interest.)
 
 So, for example, the UIO code in ``drivers/uio/uio.c`` has a structure that
-defines the memory region associated with a uio device::
+defines the memory region associated with a uio device:
 
-    struct uio_map {
-            struct kobject kobj;
-            struct uio_mem *mem;
-    };
+.. code-block:: c
 
-If you have a struct uio_map structure, finding its embedded kobject is
-just a matter of using the kobj member.  Code that works with kobjects will
-often have the opposite problem, however: given a struct kobject pointer,
+ struct uio_map {
+ 	struct kobject kobj;
+ 	struct uio_mem *mem;
+ };
+
+If you have a ``struct uio_map`` structure, finding its embedded kobject is
+just a matter of using the ``kobj`` member.  Code that works with kobjects will
+often have the opposite problem, however: given a ``struct kobject *``,
 what is the pointer to the containing structure?  You must avoid tricks
 (such as assuming that the kobject is at the beginning of the structure)
-and, instead, use the container_of() macro, found in ``<linux/kernel.h>``::
-
-    container_of(ptr, type, member)
+and, instead, use the container_of() macro:
 
-where:
+.. code-block:: c
 
-  * ``ptr`` is the pointer to the embedded kobject,
-  * ``type`` is the type of the containing structure, and
-  * ``member`` is the name of the structure field to which ``pointer`` points.
-
-The return value from container_of() is a pointer to the corresponding
-container type. So, for example, a pointer ``kp`` to a struct kobject
-embedded **within** a struct uio_map could be converted to a pointer to the
-**containing** uio_map structure with::
-
-    struct uio_map *u_map = container_of(kp, struct uio_map, kobj);
+ struct kobject *kp =  ; /* Pointer to a kobj member of a uio_map */
+ struct uio_map *u_map = container_of(kp, struct uio_map, kobj);
 
 For convenience, programmers often define a simple macro for **back-casting**
 kobject pointers to the containing type.  Exactly this happens in the
-earlier ``drivers/uio/uio.c``, as you can see here::
+earlier ``drivers/uio/uio.c``, as you can see here:
 
-    struct uio_map {
-            struct kobject kobj;
-            struct uio_mem *mem;
-    };
+.. code-block:: c
 
-    #define to_map(map) container_of(map, struct uio_map, kobj)
+ struct uio_map {
+ 	struct kobject kobj;
+ 	struct uio_mem *mem;
+ };
+ 
+ #define to_map(map) container_of(map, struct uio_map, kobj)
 
 where the macro argument "map" is a pointer to the struct kobject in
-question.  That macro is subsequently invoked with::
+question.  That macro is subsequently invoked with:
+
+.. code-block:: c
 
-    struct uio_map *map = to_map(kobj);
+ struct uio_map *map = to_map(kobj);
 
 
 Initialization of kobjects
-- 
2.30.2

Re: [PATCH] Documentation: Reference kernel-doc for container_of

[Jonathan Corbet]

René Nyffenegger <mail@renenyffenegger.ch> writes:

> The file include/linux/container_of.h contained kernel-doc but was not
> referenced in any .rst file. In addition, the file
> Documentation/core-api/kobject.rst wrongly located the definition
> of the macro `container_of` in linux/kernel.h while in reality
> it is defined in linux/container_of.h
>
> This patch adds a new .rst file that includes the kernel-doc of
> container_of.h and rectifies the wrong reference of the header
> file.
>
> Signed-off-by: René Nyffenegger <mail@renenyffenegger.ch>

Thank you for working to improve the kernel documentation!

There are, though, a few problems with this patch that will need to be
addressed before it can be accepted.  To start, please cc the maintainer
(i.e. me) on documentation changes.

>  Documentation/core-api/container_of.rst | 64 +++++++++++++++++++++++++
>  Documentation/core-api/index.rst        |  1 +
>  Documentation/core-api/kobject.rst      | 58 +++++++++++-----------
>  3 files changed, 93 insertions(+), 30 deletions(-)
>  create mode 100644 Documentation/core-api/container_of.rst
>
> diff --git a/Documentation/core-api/container_of.rst b/Documentation/core-api/container_of.rst
> new file mode 100644
> index 000000000000..f063d3f9e536
> --- /dev/null
> +++ b/Documentation/core-api/container_of.rst
> @@ -0,0 +1,64 @@
> +=====================================================================================
> +Given a pointer to a member of a struct, returning a pointer to the containing struct
> +=====================================================================================

Sticking to the 80-character limit is important for documentation, as it
greatly affects its readability.

> +.. _container_of:

This label is never referenced, so please do not add it.

> +Overview
> +========
> +
> +The two macros ``container_of`` and ``container_of_const``, defined in ``<include/linux/container_of.h>``, return a pointer to the ``struct`` (i. e. the *container*) from a pointer to a member of this ``struct``.

Please just refer to macros as, for example, container_of(), with
parentheses and without literal markup.  The build code will then do the
right thing.

> +These macros might be used when :ref:`embedding kobjects<embedding_kobjects>`, but see also :ref:`usage<container_of_usage>`.

Referencing a label that is immediately below is kind of pointless.  I'd
delete the label and say "see below".

Also, kobjects are far from the most common use of container_of(), not
sure why the are called out here.

> +
> +.. kernel-doc:: include/linux/container_of.h
> +
> +Usage
> +=====
> +
> +.. _container_of_usage:
> +
> +The following simple code demonstrates the usage of ``container_of``
> +
> +.. code-block:: c
> +
> + struct inner_struct
> + {
> + 	int abc;
> + 	int def;
> + }
> + 
> + struct container_struct
> + {
> + 	struct inner_struct inner;
> + 	char               *member_xyz;
> + 	int                 member_val;
> + };
> + 
> + static void f(struct inner_struct *inr)
> + {
> + 	struct container_struct *cont;
> + 
> + 	cont = container_of(inr, struct container_struct, inner);
> + 
> +        /* Use cont and its members */
> + }
> + 
> + static void g(char** member)
> + {
> + 	struct container_struct *cont;
> + 
> + 	cont = container_of(member, struct container_struct, member_xyz);
> +        /* Use cont and its members */
> + }

Not sure why two examples are needed, they are showing the same thing?

> + void somewhere()
> + {
> + 	struct container_struct cont;
> + 
> + 	/* Initialize cont */
> + 
> + 	f(& cont.inner      );
> + 	g(& cont.member_xyz );
> + }

Sample code should follow the kernel coding style.

> diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
> index 7a3a08d81f11..595af47d0d5f 100644
> --- a/Documentation/core-api/index.rst
> +++ b/Documentation/core-api/index.rst
> @@ -37,6 +37,7 @@ Library functionality that is used throughout the kernel.
>     kref
>     assoc_array
>     xarray
> +   container_of
>     maple_tree
>     idr
>     circular-buffers
> diff --git a/Documentation/core-api/kobject.rst b/Documentation/core-api/kobject.rst
> index 7310247310a0..058570694418 100644
> --- a/Documentation/core-api/kobject.rst
> +++ b/Documentation/core-api/kobject.rst
> @@ -49,6 +49,8 @@ approach will be taken, so we'll go back to kobjects.
>  Embedding kobjects
>  ==================
>  
> +.. _embedding_kobjects:
> +
>  It is rare for kernel code to create a standalone kobject, with one major
>  exception explained below.  Instead, kobjects are used to control access to
>  a larger, domain-specific object.  To this end, kobjects will be found
> @@ -66,50 +68,46 @@ their own, but are invariably found embedded in the larger objects of
>  interest.)
>  
>  So, for example, the UIO code in ``drivers/uio/uio.c`` has a structure that
> -defines the memory region associated with a uio device::
> +defines the memory region associated with a uio device:
>  
> -    struct uio_map {
> -            struct kobject kobj;
> -            struct uio_mem *mem;
> -    };
> +.. code-block:: c
>  
> -If you have a struct uio_map structure, finding its embedded kobject is
> -just a matter of using the kobj member.  Code that works with kobjects will
> -often have the opposite problem, however: given a struct kobject pointer,
> + struct uio_map {
> + 	struct kobject kobj;
> + 	struct uio_mem *mem;
> + };

At this point you are making changes to documentation unrelated to
container_of().  I'm not entirely sure why you are doing that.  If you
must, though, these changes need to be in their own patch with their own
justification.

> +If you have a ``struct uio_map`` structure, finding its embedded kobject is
> +just a matter of using the ``kobj`` member.  Code that works with kobjects will
> +often have the opposite problem, however: given a ``struct kobject *``,
>  what is the pointer to the containing structure?  You must avoid tricks
>  (such as assuming that the kobject is at the beginning of the structure)
> -and, instead, use the container_of() macro, found in ``<linux/kernel.h>``::
> -
> -    container_of(ptr, type, member)
> +and, instead, use the container_of() macro:
>  
> -where:
> +.. code-block:: c
>  
> -  * ``ptr`` is the pointer to the embedded kobject,
> -  * ``type`` is the type of the containing structure, and
> -  * ``member`` is the name of the structure field to which ``pointer`` points.
> -
> -The return value from container_of() is a pointer to the corresponding
> -container type. So, for example, a pointer ``kp`` to a struct kobject
> -embedded **within** a struct uio_map could be converted to a pointer to the
> -**containing** uio_map structure with::
> -
> -    struct uio_map *u_map = container_of(kp, struct uio_map, kobj);
> + struct kobject *kp =  ; /* Pointer to a kobj member of a uio_map */
> + struct uio_map *u_map = container_of(kp, struct uio_map, kobj);
>  
>  For convenience, programmers often define a simple macro for **back-casting**
>  kobject pointers to the containing type.  Exactly this happens in the
> -earlier ``drivers/uio/uio.c``, as you can see here::
> +earlier ``drivers/uio/uio.c``, as you can see here:
>  
> -    struct uio_map {
> -            struct kobject kobj;
> -            struct uio_mem *mem;
> -    };
> +.. code-block:: c
>  
> -    #define to_map(map) container_of(map, struct uio_map, kobj)
> + struct uio_map {
> + 	struct kobject kobj;
> + 	struct uio_mem *mem;
> + };
> + 
> + #define to_map(map) container_of(map, struct uio_map, kobj)

Gratuitous formatting changes aren't really useful.

Thanks,

jon

[PATCH] Documentation: Reference kernel-doc for container_of

[René Nyffenegger]

The file include/linux/container_of.h contained kernel-doc but was not
referenced in any .rst file. In addition, the file
Documentation/core-api/kobject.rst wrongly located the definition
of the macro `container_of` in linux/kernel.h while in reality
it is defined in linux/container_of.h

This patch
  1) tries to implement Jonathan Corbet's feedback and suggestions
     of v1, see
        https://lore.kernel.org/linux-doc/87ledqm0qs.fsf@meer.lwn.net/
  2) adds a container_of.rst file that includes the kernel-doc of
     container_of.h
  3) removes the reference of the wrong header file
  4) removes some explanatory notes on the usage of
     container_of in kobject.rst as this is now covered in
     container_of.rst

Signed-off-by: René Nyffenegger <mail@renenyffenegger.ch>
---
 Documentation/core-api/container_of.rst | 54 +++++++++++++++++++++++++
 Documentation/core-api/index.rst        |  1 +
 Documentation/core-api/kobject.rst      | 17 +++-----
 3 files changed, 60 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/core-api/container_of.rst

diff --git a/Documentation/core-api/container_of.rst b/Documentation/core-api/container_of.rst
new file mode 100644
index 000000000000..b7d273ccf6fa
--- /dev/null
+++ b/Documentation/core-api/container_of.rst
@@ -0,0 +1,54 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===============================================================================
+Given a pointer to a member of a struct, get a pointer to the containing struct
+===============================================================================
+
+Overview
+========
+
+The two macros container_of and container_of_const, defined in
+``<include/linux/container_of.h>``, return a pointer to the ``struct``
+(i. e. the *container*) from a pointer to a member of this ``struct``. A
+contrived example is provided below.
+
+.. kernel-doc:: include/linux/container_of.h
+
+Usage
+=====
+
+The following simple code demonstrates the usage of ``container_of``
+
+.. code-block:: c
+
+ struct inner_struct {
+ 	int abc;
+ 	int def;
+ }
+ 
+ struct container_struct {
+ 	struct inner_struct inner;
+ 	char               *foo;
+ 	int                 bar;
+ };
+ 
+ static void f(struct inner_struct *inr)
+ {
+ 	struct container_struct *cont;
+ 
+ 	cont = container_of(inr, struct container_struct, inner);
+ 
+        /* Use cont and its members */
+ }
+ 
+ 
+ void somewhere()
+ {
+ 	struct container_struct cont;
+ 
+ 	cont.inner = ...
+        cont.foo   = ...
+        cont.bar   = ...
+ 
+ 	f(&cont.inner);
+ }
diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
index 7a3a08d81f11..595af47d0d5f 100644
--- a/Documentation/core-api/index.rst
+++ b/Documentation/core-api/index.rst
@@ -37,6 +37,7 @@ Library functionality that is used throughout the kernel.
    kref
    assoc_array
    xarray
+   container_of
    maple_tree
    idr
    circular-buffers
diff --git a/Documentation/core-api/kobject.rst b/Documentation/core-api/kobject.rst
index 7310247310a0..fac4c359e2ce 100644
--- a/Documentation/core-api/kobject.rst
+++ b/Documentation/core-api/kobject.rst
@@ -78,22 +78,15 @@ just a matter of using the kobj member.  Code that works with kobjects will
 often have the opposite problem, however: given a struct kobject pointer,
 what is the pointer to the containing structure?  You must avoid tricks
 (such as assuming that the kobject is at the beginning of the structure)
-and, instead, use the container_of() macro, found in ``<linux/kernel.h>``::
+and, instead, use the container_of() macro which returns a pointer to the
+corresponding container type.
 
-    container_of(ptr, type, member)
-
-where:
-
-  * ``ptr`` is the pointer to the embedded kobject,
-  * ``type`` is the type of the containing structure, and
-  * ``member`` is the name of the structure field to which ``pointer`` points.
-
-The return value from container_of() is a pointer to the corresponding
-container type. So, for example, a pointer ``kp`` to a struct kobject
+So, for example, a pointer ``kp`` to a struct kobject
 embedded **within** a struct uio_map could be converted to a pointer to the
 **containing** uio_map structure with::
 
-    struct uio_map *u_map = container_of(kp, struct uio_map, kobj);
+ struct kobject *kp = ... ; /* Pointer to a kobj member of a uio_map */
+ struct uio_map *u_map = container_of(kp, struct uio_map, kobj);
 
 For convenience, programmers often define a simple macro for **back-casting**
 kobject pointers to the containing type.  Exactly this happens in the
-- 
2.30.2

Re: [PATCH] Documentation: Reference kernel-doc for container_of

[René Nyffenegger]

On 9/1/23 16:22, Jonathan Corbet wrote:
> René Nyffenegger <mail@renenyffenegger.ch> writes:
>
>> The file include/linux/container_of.h contained kernel-doc but was not
>> referenced in any .rst file. In addition, the file
>> Documentation/core-api/kobject.rst wrongly located the definition
>> of the macro `container_of` in linux/kernel.h while in reality
>> it is defined in linux/container_of.h
>>
>> This patch adds a new .rst file that includes the kernel-doc of
>> container_of.h and rectifies the wrong reference of the header
>> file.
>>
>> Signed-off-by: René Nyffenegger <mail@renenyffenegger.ch>
> Thank you for working to improve the kernel documentation!
>
> There are, though, a few problems with this patch that will need to be
> addressed before it can be accepted.  To start, please cc the maintainer
> (i.e. me) on documentation changes.
>
Thanks for looking at my attempt to patch the documentations.

I tried to create an improved patch:
https://lore.kernel.org/linux-doc/20230902210422.8092-1-mail@renenyffenegger.ch/T/

However, I did not receive any feedback. So, I am wondering if I am
still making mistakes. In order to improve, I'd appreciate a feedback
what I need to change to create the patch.

Thanks

René

Re: [PATCH] Documentation: Reference kernel-doc for container_of

[Jonathan Corbet]

René Nyffenegger <mail@renenyffenegger.ch> writes:

> On 9/1/23 16:22, Jonathan Corbet wrote:
>> René Nyffenegger <mail@renenyffenegger.ch> writes:
>>
>>> The file include/linux/container_of.h contained kernel-doc but was not
>>> referenced in any .rst file. In addition, the file
>>> Documentation/core-api/kobject.rst wrongly located the definition
>>> of the macro `container_of` in linux/kernel.h while in reality
>>> it is defined in linux/container_of.h
>>>
>>> This patch adds a new .rst file that includes the kernel-doc of
>>> container_of.h and rectifies the wrong reference of the header
>>> file.
>>>
>>> Signed-off-by: René Nyffenegger <mail@renenyffenegger.ch>
>> Thank you for working to improve the kernel documentation!
>>
>> There are, though, a few problems with this patch that will need to be
>> addressed before it can be accepted.  To start, please cc the maintainer
>> (i.e. me) on documentation changes.
>>
> Thanks for looking at my attempt to patch the documentations.
>
> I tried to create an improved patch:
> https://lore.kernel.org/linux-doc/20230902210422.8092-1-mail@renenyffenegger.ch/T/
>
> However, I did not receive any feedback. So, I am wondering if I am
> still making mistakes. In order to improve, I'd appreciate a feedback
> what I need to change to create the patch.

Sorry, I do have feedback, but between the merge window and travel I've
fallen behind on things.  I'll get there, honest.

Thanks,

jon