Experimental PR for introducing klint by nbdd0121 · Pull Request #958 · Rust-for-Linux/linux

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Waker::clone must not sleep, could you elaborate?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is called by the callback passed to init_waitqueue_func_entry, which isn't allowed to sleep.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make senses, however, why do we need clone in wake_callback?

/me goes to search PR history

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course @wedsonaf may know the answer ;-)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is done to invoke the waker after unlocking the mutex protecting the waker is released. I would expect this is to prevent a deadlock or something like that.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me that we can just do the following

        if let Some(mut guard) = s.waker.try_lock() {
            if let Some(w) = guard.take() {
                drop(guard);
                w.wake();
                return 1;
            }
        |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's what I suggested in the meeting. But I didn't make the change because I think the whole NoWaitLock<Option<Waker>> thing should be replaced with AtomicWaker instead.