Message-Id: <DEO9I1XCK1ZF.3MX7M6OVB9GND@nvidia.com>
Date: Wed, 03 Dec 2025 12:30:11 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Daniel Almeida" <daniel.almeida@...labora.com>, "Alexandre Courbot"
 <acourbot@...dia.com>
Cc: "Joel Fernandes" <joelagnelf@...dia.com>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "rust-for-linux@...r.kernel.org"
 <rust-for-linux@...r.kernel.org>, "dri-devel@...ts.freedesktop.org"
 <dri-devel@...ts.freedesktop.org>, "dakr@...nel.org" <dakr@...nel.org>,
 "airlied@...il.com" <airlied@...il.com>, "Alistair Popple"
 <apopple@...dia.com>, "ojeda@...nel.org" <ojeda@...nel.org>,
 "alex.gaynor@...il.com" <alex.gaynor@...il.com>, "boqun.feng@...il.com"
 <boqun.feng@...il.com>, "gary@...yguo.net" <gary@...yguo.net>,
 "bjorn3_gh@...tonmail.com" <bjorn3_gh@...tonmail.com>, "lossin@...nel.org"
 <lossin@...nel.org>, "a.hindborg@...nel.org" <a.hindborg@...nel.org>,
 "aliceryhl@...gle.com" <aliceryhl@...gle.com>, "tmgross@...ch.edu"
 <tmgross@...ch.edu>, "simona@...ll.ch" <simona@...ll.ch>,
 "maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
 "mripard@...nel.org" <mripard@...nel.org>, "tzimmermann@...e.de"
 <tzimmermann@...e.de>, "John Hubbard" <jhubbard@...dia.com>, "Timur Tabi"
 <ttabi@...dia.com>, "joel@...lfernandes.org" <joel@...lfernandes.org>,
 "elle@...thered-steel.dev" <elle@...thered-steel.dev>, "Andrea Righi"
 <arighi@...dia.com>, "phasta@...nel.org" <phasta@...nel.org>,
 "nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>, "Nouveau"
 <nouveau-bounces@...ts.freedesktop.org>
Subject: Re: [PATCH v2 1/3] rust: helpers: Add list helpers for C linked
 list operations

On Sat Nov 29, 2025 at 3:17 AM JST, Daniel Almeida wrote:
>
>
>> On 25 Nov 2025, at 22:16, Alexandre Courbot <acourbot@...dia.com> wrote:
>> 
>> On Wed Nov 26, 2025 at 3:16 AM JST, Joel Fernandes wrote:
>>> 
>>> 
>>>> On Nov 25, 2025, at 9:52 AM, Alexandre Courbot <acourbot@...dia.com> wrote:
>>>> 
>>>> On Wed Nov 12, 2025 at 2:13 AM JST, Joel Fernandes wrote:
>>>>> Add Rust helper functions for common C linked list operations
>>>>> that are implemented as macros or inline functions and thus not
>>>>> directly accessible from Rust.
>>>>> 
>>>>> Signed-off-by: Joel Fernandes <joelagnelf@...dia.com>
>>>>> ---
>>>>> rust/helpers/helpers.c |  1 +
>>>>> rust/helpers/list.c    | 32 ++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 33 insertions(+)
>>>>> create mode 100644 rust/helpers/list.c
>>>>> 
>>>>> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
>>>>> index 79c72762ad9c..634fa2386bbb 100644
>>>>> --- a/rust/helpers/helpers.c
>>>>> +++ b/rust/helpers/helpers.c
>>>>> @@ -32,6 +32,7 @@
>>>>> #include "io.c"
>>>>> #include "jump_label.c"
>>>>> #include "kunit.c"
>>>>> +#include "list.c"
>>>>> #include "maple_tree.c"
>>>>> #include "mm.c"
>>>>> #include "mutex.c"
>>>>> diff --git a/rust/helpers/list.c b/rust/helpers/list.c
>>>>> new file mode 100644
>>>>> index 000000000000..fea2a18621da
>>>>> --- /dev/null
>>>>> +++ b/rust/helpers/list.c
>>>>> @@ -0,0 +1,32 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +
>>>>> +/*
>>>>> + * Helpers for C Circular doubly linked list implementation.
>>>>> + */
>>>>> +
>>>>> +#include <linux/list.h>
>>>>> +
>>>>> +bool rust_helper_list_empty(const struct list_head *head)
>>>>> +{
>>>>> +    return list_empty(head);
>>>>> +}
>>>>> +
>>>>> +void rust_helper_list_del(struct list_head *entry)
>>>>> +{
>>>>> +    list_del(entry);
>>>>> +}
>>>>> +
>>>>> +void rust_helper_INIT_LIST_HEAD(struct list_head *list)
>>>>> +{
>>>>> +    INIT_LIST_HEAD(list);
>>>>> +}
>>>>> +
>>>>> +void rust_helper_list_add(struct list_head *new, struct list_head *head)
>>>>> +{
>>>>> +    list_add(new, head);
>>>>> +}
>>>>> +
>>>>> +void rust_helper_list_add_tail(struct list_head *new, struct list_head *head)
>>>>> +{
>>>>> +    list_add_tail(new, head);
>>>>> +}
>>>> 
>>>> Just noticed, but of these helpers only `INIT_LIST_HEAD` and
>>>> `list_add_tail` seem to be used (in doccomments).
>>> 
>>> Correct, but it makes sense to add the most obvious/common ones (also to make it clear that using these are supported).
>> 
>> "It makes sense" is subjective, and in this case I am confident it is
>> not the right intuition to add dead code just for the sake of it.
>> 
>> Each of these helpers adds a potential breakage point from the C API
>> should the latter change, so we should only add them if they are indeed
>> necessary.
>> 
>> Actually, some of these helpers are not used when they could have been -
>> you have a `is_empty` method that rewrites the C function instead of
>> calling the helper. The only helpers that are unjustified as of now as
>> `list_add` and `list_del`, and these are easy to add when they become
>> necessary.
>> 
>> But this raises an interesting dilemma: these helpers cannot be inlined
>> and add the overhead of a function call. On the other hand, the
>> definition of `list_head` is so excessively simple that manipulating it
>> directly is virtually as intuitive as invoking the helper - and doesn't
>> bear the overhead. So should we double-down on these helpers, or just
>> drop them completely and re-implement the list functionality we need for
>> increased performance?
>
> IIRC, there is someone working to fix this overhead by working on LTO support, or at
> least I remember this talk at some iteration of Kangrejos.
>
> If you use the helpers, you’ll be covered in the future.

Looks like the series has just been posted:

https://lore.kernel.org/all/20251202-inline-helpers-v1-0-879dae33a66a@google.com/

... which I guess means we can/should use helpers whenever they are
available then. Joel, please ignore my rant above (except the part about
unused helpers) and feel free to use the C helpers whenever relevant. I
think the argument about the structure of list_head not changing can
also apply to the helper API.

Powered by blists - more mailing lists