[RFC PATCH 1/2] rust: list: Implement normal initializer for ListLinks

I Hsin Cheng posted 2 patches 11 months ago
There is a newer version of this series
[RFC PATCH 1/2] rust: list: Implement normal initializer for ListLinks
Posted by I Hsin Cheng 11 months ago
Currently ListLinks only supports to create an initializer through
"new()", which will need further initialization because the return type
of "new()" is "impl Pininit<Self>". Not even "ListLinksSlefPtr" use the
method to create a new instance of "ListLinks".

Implement a normal method to create a new instance of type "ListLinks".
This may be redundant as long as there exist a convenient and proper way
to deal with "ListLinks::new()".

For now it's introduce for the simplicity of examples in the following
patches.

Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
---
 rust/kernel/list.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index fb93330f4af4..57d75ca16434 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -158,6 +158,16 @@ unsafe impl<const ID: u64> Send for ListLinks<ID> {}
 unsafe impl<const ID: u64> Sync for ListLinks<ID> {}
 
 impl<const ID: u64> ListLinks<ID> {
+    /// Create a new instance of this type.
+    pub fn new_link() -> Self {
+        ListLinks {
+            inner: Opaque::new(ListLinksFields {
+                prev: ptr::null_mut(),
+                next: ptr::null_mut(),
+            }),
+        }
+    }
+
     /// Creates a new initializer for this type.
     pub fn new() -> impl PinInit<Self> {
         // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
-- 
2.43.0
Re: [RFC PATCH 1/2] rust: list: Implement normal initializer for ListLinks
Posted by Alice Ryhl 11 months ago
On Mon, Mar 10, 2025 at 03:30:39PM +0800, I Hsin Cheng wrote:
> Currently ListLinks only supports to create an initializer through
> "new()", which will need further initialization because the return type
> of "new()" is "impl Pininit<Self>". Not even "ListLinksSlefPtr" use the
> method to create a new instance of "ListLinks".
> 
> Implement a normal method to create a new instance of type "ListLinks".
> This may be redundant as long as there exist a convenient and proper way
> to deal with "ListLinks::new()".
> 
> For now it's introduce for the simplicity of examples in the following
> patches.
> 
> Signed-off-by: I Hsin Cheng <richard120310@gmail.com>

This change is not good. The ListLinks type has an invariant about when
the pointers are null. The existing constructor argues that the
invariant is satisfied because pin-init initializers can't be used in an
existing Arc. Why is that satisfied here?

Alice
Re: [RFC PATCH 1/2] rust: list: Implement normal initializer for ListLinks
Posted by I Hsin Cheng 11 months ago
On Mon, Mar 10, 2025 at 09:44:41AM +0000, Alice Ryhl wrote:
> On Mon, Mar 10, 2025 at 03:30:39PM +0800, I Hsin Cheng wrote:
> > Currently ListLinks only supports to create an initializer through
> > "new()", which will need further initialization because the return type
> > of "new()" is "impl Pininit<Self>". Not even "ListLinksSlefPtr" use the
> > method to create a new instance of "ListLinks".
> > 
> > Implement a normal method to create a new instance of type "ListLinks".
> > This may be redundant as long as there exist a convenient and proper way
> > to deal with "ListLinks::new()".
> > 
> > For now it's introduce for the simplicity of examples in the following
> > patches.
> > 
> > Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
> 
> This change is not good. The ListLinks type has an invariant about when
> the pointers are null. The existing constructor argues that the
> invariant is satisfied because pin-init initializers can't be used in an
> existing Arc. Why is that satisfied here?
> 
> Alice

Hi Alice,

Thanks for your kindly review. Indeed , I was trying to find a way to
cope with "ListLinks::new()", wondering if there's any macros like
"ListLinks::try_pin_init!()" to help us deal with "impl PinInit", so I
try to send a RFC patch for it.

Sorry I overlooked "commit 52ae96f"[1], it demonstrate a good way to
handle the basic structure for "List", I'll fix the patch and send a v2
as soon as possible.

Still I would love to ask why don't we provide "ListLinks::pin_init" or
"ListLinks::try_pin_init" to support for the pin-init initializer
returned by "ListLink::new()" ? Maybe there're special reasons behind
the pin-init initializer here ? I would love to learn if that's
possible.

I tried "Kbox::pint_init" but it
wouldn't give variable of type "ListLinks".

Best regards,
I Hsin Cheng
Re: [RFC PATCH 1/2] rust: list: Implement normal initializer for ListLinks
Posted by Alice Ryhl 11 months ago
On Mon, Mar 10, 2025 at 12:14 PM I Hsin Cheng <richard120310@gmail.com> wrote:
>
> On Mon, Mar 10, 2025 at 09:44:41AM +0000, Alice Ryhl wrote:
> > On Mon, Mar 10, 2025 at 03:30:39PM +0800, I Hsin Cheng wrote:
> > > Currently ListLinks only supports to create an initializer through
> > > "new()", which will need further initialization because the return type
> > > of "new()" is "impl Pininit<Self>". Not even "ListLinksSlefPtr" use the
> > > method to create a new instance of "ListLinks".
> > >
> > > Implement a normal method to create a new instance of type "ListLinks".
> > > This may be redundant as long as there exist a convenient and proper way
> > > to deal with "ListLinks::new()".
> > >
> > > For now it's introduce for the simplicity of examples in the following
> > > patches.
> > >
> > > Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
> >
> > This change is not good. The ListLinks type has an invariant about when
> > the pointers are null. The existing constructor argues that the
> > invariant is satisfied because pin-init initializers can't be used in an
> > existing Arc. Why is that satisfied here?
> >
> > Alice
>
> Hi Alice,
>
> Thanks for your kindly review. Indeed , I was trying to find a way to
> cope with "ListLinks::new()", wondering if there's any macros like
> "ListLinks::try_pin_init!()" to help us deal with "impl PinInit", so I
> try to send a RFC patch for it.
>
> Sorry I overlooked "commit 52ae96f"[1], it demonstrate a good way to
> handle the basic structure for "List", I'll fix the patch and send a v2
> as soon as possible.
>
> Still I would love to ask why don't we provide "ListLinks::pin_init" or
> "ListLinks::try_pin_init" to support for the pin-init initializer
> returned by "ListLink::new()" ? Maybe there're special reasons behind
> the pin-init initializer here ? I would love to learn if that's
> possible.
>
> I tried "Kbox::pint_init" but it
> wouldn't give variable of type "ListLinks".

Please see the examples from:
https://lore.kernel.org/rust-for-linux/20250210-cursor-between-v7-2-36f0215181ed@google.com/

Alice