`LockIniter` API concerns

[y86-dev]

Dear Rust-for-Linux team,

I want to raise some important concerns regarding some of the API design of this project.
If this is the wrong way to communicate this concern, then please direct me to right way of raising my .

My background

I am mainly a Rust developer and only have limited experience with the Linux code base. I have programmed in Rust extensively, so I probably have a rust-centric perspective on the project and might not know the constraints you are working under.

My view on this project

Let me first state this: I love this project and only wish it the best!
I personally use Linux and love the thought of integrating the safety of rust in the most often written parts of the code.

Yet, I am deeply frustrated with some of the choices made during API design. I feel like if these were pushed into mainline Linux code base hastily, it would become a legacy nightmare in the future, because you cannot remove a unsafe, already deprecated API, because a lot of existing code will depend on it. I want to avoid this future:

use kernel::sync2::Mutex;

The LockIniter API

I am singling out the LockIniter API, because that is all I managed to look at in the short time span. Locks are one of the most important primitives, so if their API has poor design that will have severe implications for all future code in the kernel.

I think it is a fatal flaw that you are required to use unsafe when you want to create and initialize a lock. This issue has been previously raised in a more general form (#290). I want to shortly illustrate the issue using a Mutex as an example (but other locks have the same problem, because they need to be pinned to be used), go over the suggested solution and its problems from the previous thread and my proposal to fix this.

(The following code snippets have been copied from samples/rust/rust_sync.rs)

In the init function of the kernel module, a Mutex is created inside of a Box and then initialized.

// SAFETY: `init` is called below
let mut data = Pin::from(Box::try_new(unsafe { Mutex::new(0) })?);
mutex_init!(data.as_mut(), "RustSync::init::data1");

After this point no more unsafe is needed to use the mutex, which is good. However in order to call Mutex::new, we need to uphold the invariant that we call Mutex::init_lock before we use it. Because locks are a fundamental tool that will be used in a majority of modules, this creates many unsafe blocks that could be avoided with better API design.

This piece of code is in my eyes not really an improvement over the C code. Yes, there is a safety comment and a clear identification of an unsafe operation. But this has two problems: It annoys C programmers, because while they still program using their old ways, they now need to occasionally sprinkle in an unsafe block, which does not really seem beneficial. Rust devs are annoyed, because there exist nicer abstractions that could solve this in a way more idiomatic way.
When we integrate Rust into the Linux codebase, we should use all advantages that the language can give us, like compiler-checked invariants and memory safety. But when we use unsafe in major places like this, we basically end up with C-style code in the Rust syntax.

Solution 1: Extra Allocations

Just store the lock in Pin<Box<_>> (or other smart pointers) and only let the lock hand out those instances.
But the additional allocation and indirection are obviously inefficient.

Solution 2: nbdd0121's Approach

While I have not extensively looked at their approach, I can say a few things:

• It achieves safe pinned initialization using a single function call
• It achieves safe wrapping of multiple types that need pinned init
• It uses closures, which seems weird and a little bit verbose to me
• There is no way to store an unpinned mutex that you want to initialize later
• One needs to use a macro to compose objects

While it seems like this approach works as intended, the discussion died down, probably because of the proc-macro integration issue.

Solution 3: My personal approach

In the short time that I had since I came across this project, I only partially managed to implemented my own solution. It is similar to nbdd0121's approach, as it also uses a proc-macro. See below for the discussion.

My approach centers around using transmutation of types to allow a separation of the creation of MutexUninit<T> and its subsequent initialization to a Mutex<T>, such that one is only able to initialize if they have access to Pin<P> where P: DerefMut<MutexUninit<T>>.

This allows the call site to look like this:

let data = Pin::from(Box::try_new(MutexUninit::new(0))?);
let mut data = data.init();

It leverages the type system to ensure that one cannot call lock on MutexUninit. This way, forgetting to initialize the lock or attempting to initialize it twice results in a compilation error.
Now, unsafe is only needed to create bottom level primitives that need to call unsafe functions themselves. At the declaration site (e.g. the cases where you want to create a struct containing multiple mutexes) you can simply use a proc-macro on the struct and mark the types that require initialization.

I created a crate as a proof-of-concept. If you want to take a look at it, I published it here. I am currently still working to improve it.

The main problem with this approach may be the usage of a proc-macro, similar to nbdd0121's solution. I'm a little late for the discussion, but I still have a few additional points to add:

• syn is very dependable, big parts of the rust ecosystem (including the rustc itself!) depend on it. There should be no problem using it, as bindgen is already integrated.
• proc-macros are the idiomatic way in Rust to implement features that are not part of the language. When a new feature for the language is proposed, one of the many questions that comes up is "Could this also be done with a proc-macro?" If the answer is yes, then it becomes increasingly unlikely that the feature is added as a language construct.
• Just like the C ecosystem, the rust ecosystem also adds its tools. Proc macros are an invaluable tool for any rust programmer and very much part of the language, just like declarative macros, functions etc.
• If the unsafe code is largely contained within some select few proc macros, then that code can be checked carefully. If an error is found, it does not need to be fixed at every invocation site. This might not seem too important in this specific case, but always following this design guideline will pay off in the long term.

Obviously, I do not think that proc-macros are the right tool for every problem, but this is definitely an area where a proc macro is the preferred and idiomatic way.

Summary

I apologize for the late interruption in your progress. I am very much aware that you probably cannot wait to integrate your progress into the mainline kernel and finally add support for Rust in Linux. And believe me, I am as excited as you are and I am very thankful for your work. I just wanted to raise my concerns over the API design and I hope you can understand why I am writing you here.

[ojeda]

Let me first state this: I love this project and only wish it the best!

Thanks! I think I speak for everyone when I say we appreciate that.

Yet, I am deeply frustrated with some of the choices made during API design. I feel like if these were pushed into mainline Linux code base hastily, it would become a legacy nightmare in the future, because you cannot remove a unsafe, already deprecated API, because a lot of existing code will depend on it. I want to avoid this future:

use kernel::sync2::Mutex;

This is not a concern, because kernel internal APIs are unstable, thus a kernel::sync2 should not happen.

I think it is a fatal flaw that you are required to use unsafe when you want to create and initialize a lock.

We want to minimize unsafe code in "leaf modules" (e.g. drivers) as much as possible, and we are aware there are a few suboptimal things at the moment, e.g. some were discussed with upstream Rust in November CTCFT. Of course, we welcome new ideas, input and contributors.

However, this is not a "fatal flaw" at all, for two reasons: the main one being that this can be improved at any time in the future as mentioned above (and we do hope that happens for this and other APIs!), like any other internals in the kernel.

The other reason is that unsafe code is not bad on its own: while we hope most modules/drivers can be written in fully safe Rust code and while unsafe code should not be used without good reason, unsafe code may still be needed in some cases -- that is fine, and it is a big improvement over C.

This piece of code is in my eyes not really an improvement over the C code. Yes, there is a safety comment and a clear identification of an unsafe operation. But this has two problems: It annoys C programmers, because while they still program using their old ways, they now need to occasionally sprinkle in an unsafe block, which does not really seem beneficial.

I disagree. Having unsafe operations clearly delimited and explained is a major improvement over C (notwithstanding that this is an area we want to improve anyway).

But when we use unsafe in major places like this,

I appreciate that you are trying to warn us about using unsafe blocks, but please note that a main goal of the project is to minimize unsafe code (and why the project exists to begin with!).

In other words, it is not that we are "happy" to have unsafe code -- quite the opposite!

The main problem with this approach may be the usage of a proc-macro

There are several concerns at play here. We are not against proc-macros (in fact, we use them), but we do try to minimize their usage; and we would prefer language-level solutions wherever possible.

• proc-macros are the idiomatic way in Rust to implement features that are not part of the language. When a new feature for the language is proposed, one of the many questions that comes up is "Could this also be done with a proc-macro?" If the answer is yes, then it becomes increasingly unlikely that the feature is added as a language construct.

I disagree. There are things that should not be proc-macros -- at all -- and I hope Rust does not make the mistake of leaving everything up to a proc-macro. For instance, I hope let else is not rejected just because it could be done as a proc macro.

I apologize for the late interruption in your progress.

No need to apologize!

[y86-dev]

This is not a concern, because kernel internal APIs are unstable, thus a kernel::sync2 should not happen.

That is relieving to hear, I was under the impression that if it were added to mainline, it would be a somewhat stable API. This is definitely changes some of my points.

We want to minimize unsafe code in "leaf modules" (e.g. drivers) as much as possible, and we are aware there are a few suboptimal things at the moment, e.g. some were discussed with upstream Rust in November CTCFT. Of course, we welcome new ideas, input and contributors.

However, this is not a "fatal flaw" at all, for two reasons: the main one being that this can be improved at any time in the future as mentioned above (and we do hope that happens for this and other APIs!), like any other internals in the kernel.

Yes, if the API is unstable, then this is not a fatal flaw.
I have only built my solution partially and I am currently experimenting how using my library actually looks like. I have found some important limitations that I would like to fix, so it will still take a little while until I have a fully working version. But I feel like one can already see that it is indeed possible to remove unsafe from all call sites and many declaration sites.

I appreciate that you are trying to warn us about using unsafe blocks, but please note that a main goal of the project is to minimize unsafe code (and why the project exists to begin with!).

In other words, it is not that we are "happy" to have unsafe code -- quite the opposite!

That is why I wanted to raise these concerns, if it is possible to avoid using unsafe, then you would probably want to use that way.

There are several concerns at play here. We are not against proc-macros (in fact, we use them), but we do try to minimize their usage; and we would prefer language-level solutions wherever possible.

I disagree. There are things that should not be proc-macros -- at all -- and I hope Rust does not make the mistake of leaving everything up to a proc-macro. For instance, I hope let else is not rejected just because it could be done as a proc macro.

Minimizing the use of proc-macros is of course a good idea, I am not advocating for the use of proc-macros wherever possible. As I already said, if something can be done using a proc-macro, it becomes more unlikely that it is instead implemented as a language construct. This is because we do not want to clutter the rust language with so many features that it becomes difficult to know what features to use and how they interact with each other (C++ has shown us what happens in that case).

There are of course situations where having a language-level solution to something that can be also done with a proc-macro is beneficial. I do not know if this is the case with pinned initialization. But the general workflow has been that a library would provide the needed feature for the time that the specifics of the language feature would be discussed. It also is a testing ground for what works and what does not.
Writing my library, I have learned that it is possible to achieve safe pinned initialization using only features already present in the language. It only is very verbose (and requires unsafe) to implement, hence I created a proc-macro that would do that work for you. This is generally a good indicator that a proc-macro is the correct call.

The problem with providing this as a language level solution is that I have no idea how it should look like. What would the usage of it look at the construction site of a pinned init type? How does it look like when you want to create a Mutex type? etc.
I feel like I have gotten a better picture while writing my library, but you might have your own ideas and it would be great to hear them!

Could you tell me some of the other concerns, or are they mostly discussed in #302 ?

[wedsonaf]

Hey @y86-dev, thanks for bringing this up. We're very happy to have passionate people join the effort!

I was writing a longer response earlier on but since Miguel already clarified a few points, I'm removing most of what I had already written.

Just to add some more context: we are of course aware of the poor ergonomics here, and we talked about this in CTCFT (link to the specific part here) and in more detail in Rust Linz (link to the specific part here). One outcome of this was a discussion on how the process would look like on the rust side (link to the beginning here) for people/project (like us) to bring issues up but not necessarily solutions -- you're the experts after all.

Anyway, since you ask about how a language solution could look like, here's a link to the part of the video where we talk about one possible way to go about this. I also stress in the end that we'd be happy with alternative solutions that address the current poor ergonomics (and aligns with our current goals of minimal external dependencies).

[y86-dev]

Just to add some more context: we are of course aware of the poor ergonomics here, and we talked about this in CTCFT (link to the specific part here) and in more detail in Rust Linz (link to the specific part here). One outcome of this was a discussion on how the process would look like on the rust side (link to the beginning here) for people/project (like us) to bring issues up but not necessarily solutions -- you're the experts after all.

Thanks for the resources.

Anyway, since you ask about how a language solution could look like, here's a link to the part of the video where we talk about one possible way to go about this. I also stress in the end that we'd be happy with alternative solutions that address the current poor ergonomics (and aligns with our current goals of minimal external dependencies).

I have thought a little bit about what that could look like. But the biggest problem that I found was that I have no idea how to generalize and combine this new feature with existing ones. While you have explicitly stated that you do not need much (just verify that every field was assigned to), rust should be designed in such a way that the features play nicely with each other (there are some exceptions, but I think we should avoid adding more). Before we add some difficult to interact with feature, we would like to explore some more of the solution space.
You explicitly said that you also would accept anything that solves the issue adequately, so I will just need to think about what alternatives there are. I just wanted to clarify my rust standpoint a bit (I expect that you have already heard this).

Is syn really such a "bad" dependency? As rustc already depends on it, it already is a transitive dependency. I might overlook something here, but from my perspective this does not seem like it should matter (but it might, given that you have not added it).

[y86-dev]

I have been exploring if what you detailed in the talk can be still be done with a macro and I think it is feasible. It should even work without a proc-macro. Could you have a look at the examples at https://github.com/y86-dev/simple-safe-init ? I will try to explain it here, as I plan to change some things:

At the center of the library is a macro called init!. This macro can initialize all types that implement a specific trait. Normal users will not need to implement it. The syntax is as follows:

init! { expr => Type {
    foo();
    let a = 10 + 12;
    .b = a;
    .c = *b;
    init_other(.d);
}}

expr is the expression that yields the value that we want to initialize. Type is the type that we want to initialize, it does not have to be the same type of expr. For example if expr is a Pin<Box<MaybeUninit<Mutex<String>>>>, then Type is Mutex<String>.
Then comes a body with some special syntax. You can execute arbitrary rust statements inside of the body. If you want to initialize any field of the given struct then you can just assign them like .b = <expr>;. Specifying such an assignment multiple times results in a compile error.
After initializing a field, you have access to a mutable pointer to that field with the same name.
You can also call a function using the . syntax, but there are some restrictions:

• the function needs to take the field as the first parameter
• the function takes a InitMe<'_, T> or PinInitMe<'_, T> where T is the field type and the field is not pinned/pinned (see later, as to how we mark a field as pinned)
• the function returns InitProof<R> for any type R

It can take other parameters (and be generic, but you cannot pass generic args at the moment) and you are also allowed to bind the result (but at the moment the syntax needs to be different, due to ambiguity).
When you forget to specify a field through an assignment, a compile time error occurs.

Specifying pinned fields

Because we want good pin interaction, the macro should also work on types that are pinned. But then the problem of pin-projection arises, what fields inherit the pin invariant and what fields do not?
You need to wrap your struct inside of pin_data! to allow calls to functions with your fields.

pin_data! {
    pub struct Foo {
        #pin
        a: u64,
        b: u64,
    }
}

This macro should at the same time also provide pin-projection, similar to pin-project/pin-project-lite, as that is also another place where a macro can avoid unsafe.

Limitations

• no storage of partially initialized types

I want to add more documentation and try to find some solutions for the ambiguity. I have not thought about enums, but maybe something similar can be done (you would just need to specify the variant first), but I have not tested that.

[nbdd0121]

This sounds very similar to my pin-init.

[y86-dev]

There are some similarities, but also some differences:

• pin-init can handle fallible initialization
• pin-init requires unsafe at some points where my approach does not:

Example from your documentation:

struct NeedPin {
    // Must points to itself
    address: *const NeedPin,
    _pinned: PhantomPinned,
}

// pin-init:
impl NeedPin {
    pub fn new() -> impl Init<Self, Infallible> {
        init_from_closure(|mut this: PinUninit<'_, Self>| -> InitResult<'_, Self, Infallible> {
            let v = this.get_mut().as_mut_ptr();
            unsafe { *ptr::addr_of_mut!((*v).address) = v };
            Ok(unsafe { this.init_ok() })
        })
    }
}

// simple-safe-init:
impl NeedPin {
    pub fn init_need_pin(this: PinInitMe<'_, Self>) -> InitProof<()> {
        let ptr = unsafe { this.as_mut_ptr() };
        init! { this => Self {
            .address = ptr;
            ._pinned = PhantomPinned;
        }}
    }
}

There is also an argument to be made to make as_mut_ptr a safe function (I was conservative) and then my approach would have no unsafe.

[y86-dev]

Also what was the main concern with pin-init? Was it the requirement of pin-project(-lite)?

[wedsonaf]

I haven't looked at all the details yet, but I really like the idea of using regular macros to initialise fields and then use a dead-code initialisation of the struct to have the compiler catch missing/duplicate fields for us.

The fact that this approach doesn't require proc macros is great. @y86-dev if you can polish your solution, I see no reason for us not to use it.

[y86-dev]

I have started integrating the library into the kernel code. This way I will discover anything that will need some more work. You can look at my current progress here: https://github.com/y86-dev/linux/tree/simple-init

Disclaimer: I wanted to only evaluate what my library needs in order to be useful, so I just tried to get it to work as hastily as possible. I rewrote the parts that are in conflict with my design and did not think if their new implementation is a good idea. When you think that the library is ready, this should be done again with more API considerations.

Is your rust branch the current branch, or should I also integrate it into the rust-next branch?

At the moment I have discovered the following shortcomings/issues open for discussion:

1. It would be a very point in time to add pin projection via a macro. My solution requires the pin_data! macro to know what type of initialization is performed. It should be very easy to also add the functionality of pin_project_lite! to further remove pin-projection related unsafes. My solution's soundness also relies upon those pin projection invariants.
2. The current API for initializing locks will need a rework, because at the moment one creates a partially initialized object and then initializes it. With my solution, one creates a fully uninitialized object and initializes it in one go. So traits like NeedsLockClass need to be redesigned.
3. Static initialization still needs the old way of initializing (partially init obj with function to full), because the type should not be MaybeUninit. I have not given this problem enough thought, so there might be a nice workaround.
4. I want to get longer paths working. At the moment the init! macro can only handle paths of length 2.
5. I think it might be a good idea to have an immediate way of creating an initialized value inside of a smart pointer without first creating it pointing to MaybeUninit::uninit().

I would greatly appreciate any feedback!

Lastly here are some examples from the samples:

rust_semaphore.rs

old:

mpl kernel::Module for RustSemaphore {
    fn init(name: &'static CStr, _module: &'static ThisModule) -> Result<Self> {
        pr_info!("Rust semaphore sample (init)\n");

        let mut sema = Pin::from(UniqueRef::try_new(Semaphore {
            // SAFETY: `condvar_init!` is called below.
            changed: unsafe { CondVar::new() },

            // SAFETY: `mutex_init!` is called below.
            inner: unsafe {
                Mutex::new(SemaphoreInner {
                    count: 0,
                    max_seen: 0,
                })
            },
        })?);

        // SAFETY: `changed` is pinned when `sema` is.
        let pinned = unsafe { sema.as_mut().map_unchecked_mut(|s| &mut s.changed) };
        condvar_init!(pinned, "Semaphore::changed");

        // SAFETY: `inner` is pinned when `sema` is.
        let pinned = unsafe { sema.as_mut().map_unchecked_mut(|s| &mut s.inner) };
        mutex_init!(pinned, "Semaphore::inner");

        Ok(Self {
            _dev: Registration::new_pinned(fmt!("{name}"), sema.into())?,
        })
    }
}

new:

impl KernelModule for RustSemaphore {
    fn init(name: &'static CStr, _module: &'static ThisModule) -> Result<Self> {
        pr_info!("Rust semaphore sample (init)\n");

        let sema = Pin::from(UniqueRef::try_new(MaybeUninit::<Semaphore>::uninit())?);
        let sema = kernel::init! {sema => Semaphore {
            kernel::init_with_lockdep!(.changed, "Semaphore::changed");
            kernel::init_with_lockdep!(.inner, "Semaphore::inner", SemaphoreInner { count: 0, max_seen: 0 });
        }};

        Ok(Self {
            _dev: Registration::new_pinned(fmt!("{name}"), sema.into())?,
        })
    }
}

rust_miscdev.rs

old:

impl SharedState {
    fn try_new() -> Result<Ref<Self>> {
        let mut state = Pin::from(UniqueRef::try_new(Self {
            // SAFETY: `condvar_init!` is called below.
            state_changed: unsafe { CondVar::new() },
            // SAFETY: `mutex_init!` is called below.
            inner: unsafe { Mutex::new(SharedStateInner { token_count: 0 }) },
        })?);

        // SAFETY: `state_changed` is pinned when `state` is.
        let pinned = unsafe { state.as_mut().map_unchecked_mut(|s| &mut s.state_changed) };
        kernel::condvar_init!(pinned, "SharedState::state_changed");

        // SAFETY: `inner` is pinned when `state` is.
        let pinned = unsafe { state.as_mut().map_unchecked_mut(|s| &mut s.inner) };
        kernel::mutex_init!(pinned, "SharedState::inner");

        Ok(state.into())
    }
}

new:

impl SharedState {
    fn try_new() -> Result<Ref<Self>> {
        let mut state = Pin::from(UniqueRef::try_new(core::mem::MaybeUninit::<Self>::uninit())?);
        let mut state = kernel::init! {state => SharedState {
            kernel::init_with_lockdep!(.state_changed, "SharedState::state_changed");
            kernel::init_with_lockdep!(.inner, "SharedState::inner", SharedStateInner { token_count: 0 });
        }};

        Ok(state.into())
    }
}

rust_sync.rs

old:

impl kernel::Module for RustSync {
    fn init(_name: &'static CStr, _module: &'static ThisModule) -> Result<Self> {
        pr_info!("Rust synchronisation primitives sample (init)\n");

        // Test mutexes.
        {
            // SAFETY: `init` is called below.
            let mut data = Pin::from(Box::try_new(unsafe { Mutex::new(0) })?);
            mutex_init!(data.as_mut(), "RustSync::init::data1");
            *data.lock() = 10;
            pr_info!("Value: {}\n", *data.lock());

            // SAFETY: `init` is called below.
            let mut cv = Pin::from(Box::try_new(unsafe { CondVar::new() })?);
            condvar_init!(cv.as_mut(), "RustSync::init::cv1");

            {
                let mut guard = data.lock();
                while *guard != 10 {
                    let _ = cv.wait(&mut guard);
                }
            }
            cv.notify_one();
            cv.notify_all();
            cv.free_waiters();
        }

        // Test static mutex + condvar.
        *SAMPLE_MUTEX.lock() = 20;

        {
            let mut guard = SAMPLE_MUTEX.lock();
            while *guard != 20 {
                let _ = SAMPLE_CONDVAR.wait(&mut guard);
            }
        }

        // Test spinlocks.
        {
            // SAFETY: `init` is called below.
            let mut data = Pin::from(Box::try_new(unsafe { SpinLock::new(0) })?);
            spinlock_init!(data.as_mut(), "RustSync::init::data2");
            *data.lock() = 10;
            pr_info!("Value: {}\n", *data.lock());

            // SAFETY: `init` is called below.
            let mut cv = Pin::from(Box::try_new(unsafe { CondVar::new() })?);
            condvar_init!(cv.as_mut(), "RustSync::init::cv2");
            {
                let mut guard = data.lock();
                while *guard != 10 {
                    let _ = cv.wait(&mut guard);
                }
            }
            cv.notify_one();
            cv.notify_all();
            cv.free_waiters();
        }

        Ok(RustSync)
    }
}

new:

impl KernelModule for RustSync {
    fn init(_name: &'static CStr, _module: &'static ThisModule) -> Result<Self> {
        pr_info!("Rust synchronisation primitives sample (init)\n");

        // Test mutexes.
        {
            let data = Pin::from(Box::try_new(MaybeUninit::<Mutex<_>>::uninit())?);
            let data = init_with_lockdep!(data, "RustSync::init::data1", 0);
            *data.lock() = 10;
            pr_info!("Value: {}\n", *data.lock());

            let cv = Pin::from(Box::try_new(MaybeUninit::<CondVar>::uninit())?);
            let cv = init_with_lockdep!(cv, "RustSync::init::cv1");

            {
                let mut guard = data.lock();
                while *guard != 10 {
                    let _ = cv.wait(&mut guard);
                }
            }
            cv.notify_one();
            cv.notify_all();
            cv.free_waiters();
        }

        // Test static mutex + condvar.
        *SAMPLE_MUTEX.lock() = 20;

        {
            let mut guard = SAMPLE_MUTEX.lock();
            while *guard != 20 {
                let _ = SAMPLE_CONDVAR.wait(&mut guard);
            }
        }

        // Test spinlocks.
        {
            let data = Pin::from(Box::try_new(MaybeUninit::<SpinLock<_>>::uninit())?);
            let data = init_with_lockdep!(data, "RustSync::init::data2", 0);
            *data.lock() = 10;
            pr_info!("Value: {}\n", *data.lock());

            let mut cv = Pin::from(Box::try_new(MaybeUninit::<CondVar>::uninit())?);
            let mut cv = init_with_lockdep!(cv, "RustSync::init::cv2");
            {
                let mut guard = data.lock();
                while *guard != 10 {
                    let _ = cv.wait(&mut guard);
                }
            }
            cv.notify_one();
            cv.notify_all();
            cv.free_waiters();
        }

        Ok(RustSync)
    }
}

[ojeda]

Thanks a lot for this!

Is your rust branch the current branch, or should I also integrate it into the rust-next branch?

rust-next is the branch that gets picked up for linux-next, do not worry about it.

(By the way, do you have an email address to contact you?)

[y86-dev]

Yes, I forgot to set it to public, here it is: y86-dev@protonmail.com

[y86-dev]

The last few weeks have been pretty busy for me, but I think I can work on this in the next weeks.
In order for the macro to play well with the kernel types, I would need to make some changes to the way locks are created and initialized. How should I go about this? Are there any major constraints for the lock traits and or macros, or would it be alright, if I thought of something myself and make it work with the samples?

[y86-dev]

I have now implemented most of the changes to the lock traits that I think are necessary. You can take a look at them here: https://github.com/y86-dev/linux/tree/simple-init-new

While browsing some files searching for more places to apply the new API I also found some points of interest and questions:

1. In workqueue.rs: why are work_structs not pinned? I looked at the kernel code and saw the use of a list_head (iirc that needs to be pinned, because it is a linked list).
2. Also in workqueue.rs: why are work_structs initialized via &UniqueRef instead of &mut UniqueRef (there also seems to be a lot more pointer juggling going on, I am currently unsure about the use of the container_of!-esque behaviour and the creation of the Ref, I will try to get some more info about this, but maybe you can point me to it instead)?
3. In rbtree.rs the two structs RBTreeNode and RBTreeNodeReservation do not have the same layout. Because there is a transmutation between them, they need to have the same layout. You can ensure that by adding #[repr(C)] and enforce the same field types and order.
4. I think the Registration-pattern (found in miscdev.rs, hwrng.rs, net/filter.rs, gpio.rs) can make use of the init API, but it seems like that could require a little more modification, tell me if you want me to try to find a way to make it work!

Here are some locations where I have added compatibility with the new init API:

• rbtree.rs
• device.rs
• kasync/executor/workqueue.rs
• sync/*
• samples: rust_sync.rs, rust_semaphore.rs, rust_miscdev.rs

Let me know what you think about these changes!