[v1,0/7] Bindings for the workqueue

Message ID 20230517203119.3160435-1-aliceryhl@google.com
Headers
Series Bindings for the workqueue |

Message

Alice Ryhl May 17, 2023, 8:31 p.m. UTC
  This patchset contains bindings for the kernel workqueue.

One of the primary goals behind the design used in this patch is that we
must support embedding the `work_struct` as a field in user-provided
types, because this allows you to submit things to the workqueue without
having to allocate, making the submission infallible. If we didn't have
to support this, then the patch would be much simpler. One of the main
things that make it complicated is that we must ensure that the function
pointer in the `work_struct` is compatible with the struct it is
contained within.

This patch could be significantly simplified if we already had the field
projection bindings. However, we have decided to upstream the current
version that does not depend on field projection first - the PR that
introduces field projections will then include a commit that simplifies
the workqueue implementation. (In particular, it would simplify the 5th
patch in this series.)

The first version of the workqueue bindings was written by Wedson, but
I have rewritten much of it so that it uses the pin-init infrastructure
and can be used with containers other than `Arc`.

Alice Ryhl (4):
  rust: workqueue: add low-level workqueue bindings
  rust: workqueue: add helper for defining work_struct fields
  rust: workqueue: add safe API to workqueue
  rust: workqueue: add `try_spawn` helper method

Wedson Almeida Filho (3):
  rust: add offset_of! macro
  rust: sync: add `Arc::{from_raw, into_raw}`
  rust: workqueue: define built-in queues

 rust/helpers.c           |   8 +
 rust/kernel/lib.rs       |  37 ++
 rust/kernel/sync/arc.rs  |  44 +++
 rust/kernel/workqueue.rs | 715 +++++++++++++++++++++++++++++++++++++++
 scripts/Makefile.build   |   2 +-
 5 files changed, 805 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/workqueue.rs


base-commit: ac9a78681b921877518763ba0e89202254349d1b
  

Comments

Tejun Heo May 17, 2023, 9:48 p.m. UTC | #1
Hello, Alice.

On Wed, May 17, 2023 at 08:31:12PM +0000, Alice Ryhl wrote:
> This patchset contains bindings for the kernel workqueue.
> 
> One of the primary goals behind the design used in this patch is that we
> must support embedding the `work_struct` as a field in user-provided
> types, because this allows you to submit things to the workqueue without
> having to allocate, making the submission infallible. If we didn't have
> to support this, then the patch would be much simpler. One of the main
> things that make it complicated is that we must ensure that the function
> pointer in the `work_struct` is compatible with the struct it is
> contained within.
> 
> This patch could be significantly simplified if we already had the field
> projection bindings. However, we have decided to upstream the current
> version that does not depend on field projection first - the PR that
> introduces field projections will then include a commit that simplifies
> the workqueue implementation. (In particular, it would simplify the 5th
> patch in this series.)
> 
> The first version of the workqueue bindings was written by Wedson, but
> I have rewritten much of it so that it uses the pin-init infrastructure
> and can be used with containers other than `Arc`.

I tried to read the patches but am too dumb to understand much. Any chance
you can provide some examples so that I can at least imagine how workqueue
would be used from rust side?

Thanks.
  
Alice Ryhl May 17, 2023, 10:22 p.m. UTC | #2
On Wed, 17 May 2023 11:48:19 -1000, Tejun Heo wrote:
> I tried to read the patches but am too dumb to understand much.

The patch is more complicated than I would have liked, unfortunately.
However, as I mentioned in the cover letter, simplifications should be
on their way.

Luckily, using the workqueue bindings is simpler than the bindings
themselves.

> Any chance you can provide some examples so that I can at least
> imagine how workqueue would be used from rust side?

Yes, of course!

The simplest way to use the workqueue is to use the `try_spawn` method
introduced by the last patch in the series. With this function, you just
pass a function pointer to the `try_spawn` method, and it schedules the
function for execution. Unfortunately this allocates memory, making it
a fallible operation.

To avoid allocation memory, we do something else. As an example, we can
look at the Rust binder driver that I am currently working on. Here is
how it will be used in the binder driver: First, the `Process` struct
will be given a `work_struct` field:

#[pin_data]
pub(crate) struct Process {
    // Work node for deferred work item.
    #[pin]
    defer_work: Work<Arc<Process>>,

    // Other fields follow...
}

Here, we use the type `Work<Arc<Process>>` for our field. This type is
the Rust wrapper for `work_struct`. The generic parameter to `Work`
should be the pointer type used to access `Process`, and in this case it
is `Arc<Process>`. The pointer type `Arc` is used for reference
counting, and its a pointer type that owns a ref-count to the inner
value. (So e.g., it decrements the ref-cout when the arc goes out of
scope.) Arc is an abbreviation of "atomic reference count". This means
that while it is enqueued in the workqueue, the workqueue owns a
ref-count to the process.

Next, binder will use the `impl_has_work!` macro to declare that it
wants to use `defer_work` as its `work_struct` field. That looks like
this:

kernel::impl_has_work! {
    impl HasWork<Arc<Process>> for Process { self.defer_work }
}

To define the code that should run when the work item is executed on the
workqueue, binder does the following:

impl workqueue::ArcWorkItem for Process {
    fn run(self: Arc<Process>) {
        // this runs when the work item is executed
    }
}

Finally to schedule it to the system workqueue, it does the following:

let _ = workqueue::system().enqueue(process);

Here, the `enqueue` call is fallible, since it might fail if the process
has already been enqueued to a work queue. However, binder just uses
`let _ =` to ignore the failure, since it doesn't need to do anything
special in that case.

I hope that helps, and let me know if you have any further questions.

Alice
  
Andreas Hindborg May 23, 2023, 2:08 p.m. UTC | #3
Alice Ryhl <aliceryhl@google.com> writes:

> On Wed, 17 May 2023 11:48:19 -1000, Tejun Heo wrote:
>> I tried to read the patches but am too dumb to understand much.
>
> The patch is more complicated than I would have liked, unfortunately.
> However, as I mentioned in the cover letter, simplifications should be
> on their way.
>
> Luckily, using the workqueue bindings is simpler than the bindings
> themselves.
>
>> Any chance you can provide some examples so that I can at least
>> imagine how workqueue would be used from rust side?
>
> Yes, of course!

If you have bandwidth for it, it would be awesome to see some examples
in the series as well (for /samples/rust).

BR Andreas

>
> The simplest way to use the workqueue is to use the `try_spawn` method
> introduced by the last patch in the series. With this function, you just
> pass a function pointer to the `try_spawn` method, and it schedules the
> function for execution. Unfortunately this allocates memory, making it
> a fallible operation.
>
> To avoid allocation memory, we do something else. As an example, we can
> look at the Rust binder driver that I am currently working on. Here is
> how it will be used in the binder driver: First, the `Process` struct
> will be given a `work_struct` field:
>
> #[pin_data]
> pub(crate) struct Process {
>     // Work node for deferred work item.
>     #[pin]
>     defer_work: Work<Arc<Process>>,
>
>     // Other fields follow...
> }
>
> Here, we use the type `Work<Arc<Process>>` for our field. This type is
> the Rust wrapper for `work_struct`. The generic parameter to `Work`
> should be the pointer type used to access `Process`, and in this case it
> is `Arc<Process>`. The pointer type `Arc` is used for reference
> counting, and its a pointer type that owns a ref-count to the inner
> value. (So e.g., it decrements the ref-cout when the arc goes out of
> scope.) Arc is an abbreviation of "atomic reference count". This means
> that while it is enqueued in the workqueue, the workqueue owns a
> ref-count to the process.
>
> Next, binder will use the `impl_has_work!` macro to declare that it
> wants to use `defer_work` as its `work_struct` field. That looks like
> this:
>
> kernel::impl_has_work! {
>     impl HasWork<Arc<Process>> for Process { self.defer_work }
> }
>
> To define the code that should run when the work item is executed on the
> workqueue, binder does the following:
>
> impl workqueue::ArcWorkItem for Process {
>     fn run(self: Arc<Process>) {
>         // this runs when the work item is executed
>     }
> }
>
> Finally to schedule it to the system workqueue, it does the following:
>
> let _ = workqueue::system().enqueue(process);
>
> Here, the `enqueue` call is fallible, since it might fail if the process
> has already been enqueued to a work queue. However, binder just uses
> `let _ =` to ignore the failure, since it doesn't need to do anything
> special in that case.
>
> I hope that helps, and let me know if you have any further questions.
>
> Alice
  
Andreas Hindborg May 23, 2023, 2:14 p.m. UTC | #4
Alice Ryhl <aliceryhl@google.com> writes:

> This patchset contains bindings for the kernel workqueue.
>
> One of the primary goals behind the design used in this patch is that we
> must support embedding the `work_struct` as a field in user-provided
> types, because this allows you to submit things to the workqueue without
> having to allocate, making the submission infallible. If we didn't have
> to support this, then the patch would be much simpler. One of the main
> things that make it complicated is that we must ensure that the function
> pointer in the `work_struct` is compatible with the struct it is
> contained within.
>
> This patch could be significantly simplified if we already had the field
> projection bindings. However, we have decided to upstream the current
> version that does not depend on field projection first - the PR that
> introduces field projections will then include a commit that simplifies
> the workqueue implementation. (In particular, it would simplify the 5th
> patch in this series.)
>
> The first version of the workqueue bindings was written by Wedson, but
> I have rewritten much of it so that it uses the pin-init infrastructure
> and can be used with containers other than `Arc`.
>
> Alice Ryhl (4):
>   rust: workqueue: add low-level workqueue bindings
>   rust: workqueue: add helper for defining work_struct fields
>   rust: workqueue: add safe API to workqueue
>   rust: workqueue: add `try_spawn` helper method
>
> Wedson Almeida Filho (3):
>   rust: add offset_of! macro
>   rust: sync: add `Arc::{from_raw, into_raw}`
>   rust: workqueue: define built-in queues
>
>  rust/helpers.c           |   8 +
>  rust/kernel/lib.rs       |  37 ++
>  rust/kernel/sync/arc.rs  |  44 +++
>  rust/kernel/workqueue.rs | 715 +++++++++++++++++++++++++++++++++++++++
>  scripts/Makefile.build   |   2 +-
>  5 files changed, 805 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/workqueue.rs
>
>
> base-commit: ac9a78681b921877518763ba0e89202254349d1b

This does not compile for me. Could you link dependencies to be applied
first?

Best regards,
Andreas
  
Alice Ryhl May 24, 2023, 12:33 p.m. UTC | #5
Andreas Hindborg <nmi@metaspace.dk> writes:
> This does not compile for me. Could you link dependencies to be applied
> first?

I messed up something related to the patch for specifying error type on
`Result` [1]. This patch doesn't _need_ to depend on that though, so the
next version of this patchset should compile without it.

Alice

[1]: https://lore.kernel.org/all/20230502124015.356001-1-aliceryhl@google.com/