[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <5F21F7D7-D0E7-4B25-AC3E-D21331EE4A1F@collabora.com>
Date: Fri, 28 Nov 2025 15:17:07 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: 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 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.
— Daniel
Powered by blists - more mailing lists