Comments for "https://os.phil-opp.com/async-await/"

[utterances-bot]

This is a general purpose comment thread for the Async/Await post.

[Cerber-Ursi]

The thing I was trying to wrap my head around for quite some time, thanks for so detailed explanation!

Two minor points:

1. The following code in run_ready_tasks:

if !self.waker_cache.contains_key(&task_id) {
    self.waker_cache.insert(task_id, self.create_waker(task_id));
}
let waker = self.waker_cache.get(&task_id).expect("should exist");

- why not use the Entry API instead?
2. "it's both simpler and safer to instead implement the Arc-based Wake trait" - the link on "Wake" leads to wake method on Waker - probably it should be this trait instead?

[jounathaen]

The statemachine in https://os.phil-opp.com/async-await/#the-async-await-pattern

    fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
        loop {
            match self { // TODO: handle pinning
                ExampleStateMachine::Start(state) => {…}
                ExampleStateMachine::WaitingOnFooTxt(state) => {…}
                ExampleStateMachine::WaitingOnFooTxt(state) => {…}
                ExampleStateMachine::End(state) => {…}
            }
        }
    }

Contains the state ExampleStateMachine::WaitingOnFooTxt twice. Should probably be ExampleStateMachine::WaitingOnBarTxt 😉

edit: Fix in #769

[senseiod]

Excellent! Looking forward to the next article

[phil-opp]

@Cerberuser Great to hear that the post is useful to you!

• why not use the Entry API instead?

Good question! I tried using it, but it leads to borrowing errors here because we need to borrow self again for the create_waker method. Of course we could work around this, for example by destructuring self and defining the create_waker methods in a way that it only needs access to the wake_queue field. However, the variant with expect was shorter and easier to explain, so I found it the better choice for this post.

the link on "Wake" leads to wake method on Waker - probably it should be this trait instead?

Thanks for reporting! Fixed in f32ee7f.

[phil-opp]

@jounathaen Thanks for reporting and providing the fix!

[phil-opp]

@senseiod I already started working on it :).

[p2mate]

I'm surprised that on x86 you need to re-enable interrupts before doing a hlt. On cortex m you can do:
cortex_m::interrupt::free(|_| {
if self.wake_queue.is_empty() {
cortex_m::asm::wfi()
}
})

So the CPU will wake from wfi even with the interrupts disabled. Ofcourse the interrupt handler will only be executed when the interrupts are re-enabled.

[phil-opp]

@p2mate Yeah, I think the approach of ARM is definitely better. As far as I know, there is no equivalent to the wfi instruction on x86 unfortunately.

[andyx719]

thank you , it's very detailed/clear and very useful.

[AlphaHot]

Thank you very much for the detailed lesson, and I look forward to continuing.

[LordSparhawk]

There is a repetition typo: "have to have to" in the first paragraph of
Executors and Wakers

Thanks for a fascinating article.

[ZoeyR]

Isn't the uniqueness of TaskId not be guaranteed for zero-sized futures? Box would give the same address for multiple "instances" of the zero sized type.

Edit: See this playground link for an example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=72f510e1edcb7657ce46becab9179977

[camorimd]

Great article! Thank you so much :)

[phil-opp]

Thank you all for the nice feedback :).

@LordSparhawk

There is a repetition typo: "have to have to" in the first paragraph of Executors and Wakers

Thanks for reporting! Fixed in b6d09c8.

[phil-opp]

@ZoeyR

Isn't the uniqueness of TaskId not be guaranteed for zero-sized futures? Box would give the same address for multiple "instances" of the zero sized type.

Very good catch! While zero-sized futures are rare in practice, we should still ensure that they can be used without problems. I opened #782 and #783 to create task IDs using a static counter instead.

[guswynn]

https://doc.rust-lang.org/std/future/trait.Future.html doesn't seem to suggest that when you are returning ready that you must use the waker to say you are ready.

I have found future impl's (like https://docs.rs/futures-util/0.3.4/src/futures_util/future/future/map.rs.html#42) that pass the cx down to a lower future, but don't actually call wake themselves. Are they assuming a lower future will call wake?

Should an executor have a timeout after which if it hasn't gotten a .wake() from a task, it polls the future anyways?

[phil-opp]

@guswynn

https://doc.rust-lang.org/std/future/trait.Future.html doesn't seem to suggest that when you are returning ready that you must use the waker to say you are ready.

I'm not sure if I understand your question. You don't need to do anything with the waker if you're returning Poll::Ready because the task can continue execution in this case. It is only paused after a future returned Poll::Pending. In that case, the asynchronous task must wake the Waker to signal that the task should be continued.

I have found future impl's (like https://docs.rs/futures-util/0.3.4/src/futures_util/future/future/map.rs.html#42) that pass the cx down to a lower future, but don't actually call wake themselves. Are they assuming a lower future will call wake?

Yes, exactly. Only the futures at the lowest level need to handle the Waker. Futures that call poll on an inner future can rely on the fact that the inner future will perform the wakeup in case it returns Poll::Pending.

Should an executor have a timeout after which if it hasn't gotten a .wake() from a task, it polls the future anyways?

This might be a good idea for robustness, but normally you should be able to rely on the waker notification.

[hismito]

Thanks for the great post.

In "The Problem with Self-Referential Structs"

element: 0x1001a, // address of the last array element

should this address be 0x1001c to match the diagram ?

edit: array in diagram starts at 0x10014, the last element sits at 0x10014 + 4 * 2 = 0x1001c.
if that's true, the diagram itself might need to be updated to 0x1001c as well.

[kennystrawnmusic]

Isn’t going from cooperative multitasking to preemptive multitasking as simple and straightforward here as locking each Task with a Mutex? I mean, that’s really all threads really are: cases where each new process instance is locked down.

[kennystrawnmusic]

^ Oops, I meant adding a MutexGuard attribute to the Executor struct. Isn’t that really the only necessary step here, or is there something else?

[phil-opp]

@realKennyStrawn93 I'm not quite sure what you mean with adding a MutexGuard to the Executor. Preemptive multitasking means that each task can be interrupted at arbitrary points in time, not just when it returns Poll::Pending. This means that a task might be paused between any two assembly instructions. Since it needs to continue exetution normally when it is resumed, the OS must ensure that the complete CPU state including all registers and stack content is exactly the same as when the task was paused. This imposes some challenges and e.g. requires a separate stack per thread.

[phil-opp]

@hismito Good catch! You're right that it should be 0x1001c instead of 0x1001a. Fixed in a7a5757.

[Mandragorian]

Why do we use an Arc, instead of an Rc wrapper? Everything runs on the same thread so how do atomic operations help us?

It might have something to do with the asynchronicity of interrupts but I am not sure how this is relevant. Then again, I have gotten quite rusty, no pun intended, with low level asynchronous programming.

edit: I now realize that Arc is probably used because of how Wake uses it in the blanket implementation.

[phil-opp]

@Mandragorian The Waker type of the core library implements Sync, so we can't create a Waker that uses the non-Sync Rc type internally, even if we don't use the Wake trait.

Apart from the type system requirements, using a waker type that isn't Sync would lead to race conditions in our code even though we're not using multiple threads yet. The reason is, as you assumed, that interrupts are asynchronous. So they might arrive right when the main thread modifies the reference count. If this modification wasn't atomic, a data race would occur if the interrupt handler modifies the reference count too (which it does when it calls wake on the AtomicWaker).

[mtnygard]

The code block under "The Wake Trait" says it goes in simple_executors.rs. Should that be executors.rs?

[cshuaimin]

If I understand correctly, you push the task ID to wake_queue when the Future wakes up, and then move the ID from wake_queue to task_queue in the run() method. Why use indirect wake_queue? It will be much clearer if we push the task ID directly to task_queue when the Future wakes up.

[phil-opp]

@mtnygard Good catch, fixed in 199c3b4.

[phil-opp]

@SnowyCoder Great to hear that you like the blog!

Is there a reason why in the executor you splitted the tasks and the waker_cache? Since each task once running should have a waker I would've written them as a single BTreeMap<TaskId, (Task, Optional<Waker>)> (or a struct with two fields).

It's been a while since I wrote that code, so I don't remember my exact reasons for using a separate waker cache anymore. I know that I changed the executor design a few times while writing this post, so maybe the reasons for keeping it separate don't exist anymore with the current design.

Is it possible to create the keyboard queue without dynamic allocation? It just seems more efficient since the space requirements don't change (although I imagine it's harder to implement)

Yes, this should be possible. The ArrayQueue implementation in crossbeam only does a single allocation in it's new method, which could be replaced with a static array. However, this would require modifying the crossbeam code, which was not worth the effort to me.

It's also worth mentioning that there is ongoing work on adding support for heap allocations in constants in Rust itself, which might at some point allow to mark the ArrayQueue::new function as const without removing the allocation. Then we could directly initialize a static at compile time and the Rust compiler would automatically transform the allocation to a static memory region.

[RMuskovets]

It's also worth mentioning that there is ongoing work on adding support for heap allocations in constants in Rust itself, which might at some point allow to mark the ArrayQueue::new function as const without removing the allocation. Then we could directly initialize a static at compile time and the Rust compiler would automatically transform the allocation to a static memory region.

Did you mean that compiler will optimize the allocation away and we won't need a dynamic allocator for this?

[phil-opp]

@RMuskovets This is only about const and static items. So you might be able to do static FOO: &String = &String::from("test") at some point. In this case, the compiler would intercept the allocation and replace it with normal static memory.

Normal let statements are not effected by this. So let foo = String::from("test") will still use the dynamic allocator.

[RMuskovets]

Oh, that's really cool! But will it work in environment like lazy_static?
And for me this is a bit unclear: if I declare a const in a function (if it doesn't result in a compile error...), will it still use dynamic allocations? I mean something like this:

fn function() {
    const foo: &String = &String::from("test");
}

[phil-opp]

It doesn't matter where you define a const in your code, it is always evaluated at compile time. So if allocations in constants are permitted at some time, your code example should work and not do any actual allocations.

Lazy static evaluates expressions lazily at runtime, so it would still perform a normal dynamic allocation at runtime.

[Gabriel-Alves-Cunha]

Hey, super duper amazing series. You'r AWESOME. Can't wait for more. :)

Btw, in:

[dependencies.crossbeam-queue]
version = "0.2.1"

it does not work if you pump it up to the newest "0.3.0" version as I first did. Just a warning if someone wastes any time on it like me.

[jedel1043]

@Gabriel-Alves-Cunha That's because Crossbeam changed its pop() api from Ok(val) to Some(val). Just change that and it should work on ver. 0.3.x

[ethindp]

I'm still confused on how to make asynchronous tasks. from what I've read I just do: ```rust async fn func_name(args...) -> ret_value { // code... } ``` But my executor still runs each task in sequence. I'm not very good at async programming, so I don't know how to explicitly indiate to the executor that I'm busy doing something and it can switch to another one and things like that. (I need to work on my ATA Routines... Ugh.)

…

On 12/3/20, jedel1043 ***@***.***> wrote: @Gabriel-Alves-Cunha That's because Crossbeam changed its ```pop()``` api from ```Ok(val)``` to ```Some(val)```. Just change that and it should work on ver. 0.3.x -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: #768 (comment)

-- Signed, Ethin D. Probst

[phil-opp]

@Gabriel-Alves-Cunha

Hey, super duper amazing series. You'r AWESOME. Can't wait for more. :)

Thanks a lot, I'm glad that you like it :).

@ethindp

Tasks are only paused at .await points if they can't directly continue. If a task doesn't need to wait, it just runs till it finishes. Only if an awaited future returns Poll::Pending, the task pauses itself and returns control to the executor.

[brightly-salty]

I would just like to point out that instructions::enable_interrupts_and_hlt as used in the post is deprecated and probably should be changed to instructions::enable_and_hlt.
Thanks for all your hard work in making this series!

[phil-opp]

@brightly-salty Thanks for reporting! Yes I'm aware of that and I try to update it when I have time.

[ethindp]

@phil-opp I know. But how do I explicitly get a task to return Poll::Pending? I'd love to be able to do that on demand, for example if I'm waiting for an NVM format to finish.

…

On 12/28/20, Philipp Oppermann ***@***.***> wrote: @brightly-salty Thanks for reporting! Yes I'm aware of that and I try to update it when I have time. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #768 (comment)

-- Signed, Ethin D. Probst

[bjorn3]

You will need to implement Future on a type and then .await a value of that type. Make sure to use the Waker that is passed in through the context that you get as argument to tell the executor to run your task again when it is ready.

[paulzhang5511]

Excellent!

[mkbanchi]

Hi and congratulations for the post! Hi found a little error in the paragraph 'The Problem with Self-Referential Structs'.
After moving struct WaitingOnWriteState at address 0x10024, the last array element will be at address 0x1002c and not at address 0x1002a.

[phil-opp]

Good catch, thanks! Fixed in cd8e139.

[TANDEXX]

Try to make disk support next mbr or gpt and finally: exfat, fat32 fat16 and fat12 file systems support. This can be very good!!!

[phil-opp]

@TANDEXX We might cover disk access and file systems at some point, but there are some other topics that I plan to cover before that (threads, processes, userspace, etc).

[tux7k]

maybe in the future you could do guis?? 🤔

[phil-opp]

I'm currently working on a new edition of the blog, which will use a pixel-based framebuffer for display output. This makes it possible to set individual pixels and also create some GUI if you like. For the blog, I plan to only cover a simple shell and some games at the beginning, but as soon as we have support for userspace processes, a GUI might make sense. (This won't happen in the near future though.)

[TANDEXX]

I have problem while compiling system. this throws linking with cc failed with many arguments and next output is:
= note: /bin/ld: /home/jonatan/code/rs/blogOs/target/debug/deps/blog_os-dcd5841984a347cd.3zk56vg27gs3kpga.rcgu.o: in function '_start':
/home/jonatan/.cargo/registry/src/github.com-1ecc6299db9ec823/bootloader-0.9.12/src/lib.rs:35: multiple definition of '_start'; /usr/lib/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu/Scrt1.o:(.text+0x0): first defined here
/bin/ld: /usr/lib/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu/Scrt1.o: in function '_start':
(.text+0x12): undefined reference to '__libc_csu_fini'
/bin/ld: (.text+0x19): undefined reference to '__libc_csu_init'
/bin/ld: (.text+0x20): undefined reference to 'main'
/bin/ld: (.text+0x26): undefined reference to '__libc_start_main'
collect2: error: ld returned 1 exit status
only one error and one warning. I installed needed target and rust nightly and I am on linux mx (debian like system). What is wrong? (I replaced ` characters with ')

[phil-opp]

You need to compile the project for a bare-metal target, e.g. by passing a --target argument.

[ethindp]

To be fair about the disk access, disk access is a fairly complicated topic and requires (a lot) of code. As someone who's working on an NVMe driver, my implementation already spans at least nearly 3k LoC, and I haven't even started reading and writing the media yet -- I'm just implementing the admin command set right now. The PCIe code is another 400-500 lines, though that just might be the way I did it. But either way, it would undoubtedly take at least 10-15 blog posts to cover raw disk IO alone -- and we haven't even gotten on file systems. So I definitely understand why there's no rush there, and I'd even go as far as to say to leave it out and let the reader figure it out; we're not trying to write a book about OS development, after all, just a path that newcomers can travel to get going. Right?

…

On 2/23/21, Philipp Oppermann ***@***.***> wrote: You need to compile the project for a bare-metal target, e.g. by passing a `--target` argument. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #768 (comment)

-- Signed, Ethin D. Probst

[nanoadam]

When I run the kerenl, I get the warning: WARNING: Scancode queue uninitialized. I tried looking for mistakes but couldn't find any.

Here is my github repo: /~https://github.com/nanoproductions/dwn-os

Thanks!

[ethindp]

Okay, so I'm trying to make it possible to spawn tasks when the executor is not owned by any function. As it is right now, I have it like this:

    let mut executor = Executor::new();
    executor.spawn(AsyncTask::new(gdt::init()));
    executor.spawn(AsyncTask::new(interrupts::init_stage2()));
    executor.spawn(AsyncTask::new(pci::init()));
    executor.spawn(AsyncTask::new(rtc::init()));
    executor.run();

However, not only does .run() consume the executor, but its blocking too. If I store it in a mutex, then that mutex will be held forever. So you won't then be able to spawn anything. I could store it in an atomic, but that's not very adequate either.
I thought of altering the .run() method of the executor to be called on an interrupt, and making it not loop at all, but that presents me with a problem -- the interrupt will be hung until run() completes, and only Rust knows when that'll be. But I definitely need to be able to do this, because I need to be able to call async code from non-async functions. I'm just not sure how to do it. Thoughts?

[tux7k]

I'm currently working on a new edition of the blog, which will use a pixel-based framebuffer for display output. This makes it possible to set individual pixels and also create some GUI if you like. For the blog, I plan to only cover a simple shell and some games at the beginning, but as soon as we have support for userspace processes, a GUI might make sense. (This won't happen in the near future though.)

Wow, thanks

[TANDEXX]

so I rewrited this to basic shell and here is download link: https://drive.google.com/file/d/1uDwCqCpfBzrz93v9AGXWjQ2xp2aVAitF/view?usp=sharing. messages displayed before modification are not deleted and this isn't finished because my linux on pendrive was broken and finally I wanted to make my own system (file system also was broken and I writed my own system in virtual machine because I just can't write .bat files and I know only the: sh, javascript, bash, python (very low) and php as script languages)

[HKalbasi]

I didn't understand why we need handle multiple tasks in executor. Is it possible to combine all futures in one, for example using join_all? I think one task executer is way simpler, with same result.