Hi,
Just a few comments while I am still reviewing:
On 7/3/25 7:10 AM, Jonathan Corbet wrote:
> Thanks for doing this!
>
> I have a few comments, most of which are just nits. I think we should
> be able to get this in for 6.17.
>
> Nicolas Frattaroli <nicolas.frattaroli@xxxxxxxxxxxxx> writes:
>
>> diff --git a/Documentation/core-api/list.rst b/Documentation/core-api/list.rst
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..b0586056abb04d2bcc4518f7238ff9a94d3dd774
>> --- /dev/null
>> +++ b/Documentation/core-api/list.rst
>> @@ -0,0 +1,847 @@
>> +.. SPDX-License-Identifier: GPL-2.0+
>> +
>> +=====================
>> +Linked Lists in Linux
>> +=====================
>> +
>> +:Author: Nicolas Frattaroli <nicolas.frattaroli@xxxxxxxxxxxxx>
>
> I do wonder if you should start by showing the list_head structure
> itself? It is simple enough and not a secret that needs to be kept.
+1
>> +Declaring and initializing a list
>> +---------------------------------
>> +
>> +A doubly-linked list can then be declared as just another ``struct list_head``,
>> +and initialised with the LIST_HEAD_INIT() macro during initial assignment, or
>> +with the INIT_LIST_HEAD() function later:
>> +
>> +.. code-block:: c
>> +
>> + struct clown_car {
>> + int tyre_pressure[4];
>> + struct list_head clowns; /* Looks like a node! */
>> + };
>> +
>> + /* ... Somewhere later in our driver ... */
>> +
>> + static int circus_init(struct circus_priv *circus)
>> + {
>> + struct clown_car other_car = {
>> + .tyre_pressure = {10, 12, 11, 9},
>> + .clowns = LIST_HEAD_INIT(other_car.clowns)
>> + };
>> +
>> + circus->car.clowns = INIT_LIST_HEAD(&circus->car.clowns);
linked_lists.c: In function ‘circus_init’:
linked_lists.c:35:30: error: invalid use of void expression
35 | circus->car.clowns = INIT_LIST_HEAD(&circus->car.clowns);
due to
static inline void INIT_LIST_HEAD(struct list_head *list);
Should it just be:
INIT_LIST_HEAD(&circus->car.clowns);
?
>> +
>> + return 0;
>> + }
>> +
>> +A further point of confusion to some may be that the list itself doesn't really
>> +have its own type. The concept of the entire linked list and a
>> +``struct list_head`` member that points to other entries in the list are one and
>> +the same.
> [...]
>
>
>> +Further reading
>> +---------------
>> +
>> +* `How does the kernel implements Linked Lists? - KernelNewbies <https://kernelnewbies.org/FAQ/LinkedLists>`_
>
> I do still think you should move the kerneldoc for lists over from
> kernel-api.rst; just tack it onto the end here.
Ack.
--
~Randy
