Joel Fernandes
2025-Nov-25 18:16 UTC
[PATCH v2 1/3] rust: helpers: Add list helpers for C linked list operations
> On Nov 25, 2025, at 9:52?AM, Alexandre Courbot <acourbot at nvidia.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 at nvidia.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). Thanks.
John Hubbard
2025-Nov-25 19:48 UTC
[PATCH v2 1/3] rust: helpers: Add list helpers for C linked list operations
On 11/25/25 10:16 AM, Joel Fernandes wrote:>> On Nov 25, 2025, at 9:52?AM, Alexandre Courbot <acourbot at nvidia.com> wrote: >> ?On Wed Nov 12, 2025 at 2:13 AM JST, Joel Fernandes wrote:...>> 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's important to exercise the code. Let's please add that coverage, seeing as how it is quite easy to do, right? thanks, -- John Hubbard
Alexandre Courbot
2025-Nov-26 01:16 UTC
[PATCH v2 1/3] rust: helpers: Add list helpers for C linked list operations
On Wed Nov 26, 2025 at 3:16 AM JST, Joel Fernandes wrote:> > >> On Nov 25, 2025, at 9:52?AM, Alexandre Courbot <acourbot at nvidia.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 at nvidia.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?