mbox series

[0/7] File abstractions needed by Rust Binder

Message ID 20231129-alice-file-v1-0-f81afe8c7261@google.com
Headers
Series File abstractions needed by Rust Binder |

Message

Alice Ryhl Nov. 29, 2023, 12:51 p.m. UTC 
  This patchset contains the file abstractions needed by the Rust
implementation of the Binder driver.

Please see the Rust Binder RFC for usage examples:
https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-0-08ba9197f637@google.com/

Users of "rust: file: add Rust abstraction for `struct file`":
	[PATCH RFC 02/20] rust_binder: add binderfs support to Rust binder
	[PATCH RFC 03/20] rust_binder: add threading support

Users of "rust: cred: add Rust abstraction for `struct cred`":
	[PATCH RFC 05/20] rust_binder: add nodes and context managers
	[PATCH RFC 06/20] rust_binder: add oneway transactions
	[PATCH RFC 11/20] rust_binder: send nodes in transaction
	[PATCH RFC 13/20] rust_binder: add BINDER_TYPE_FD support

Users of "rust: security: add abstraction for security_secid_to_secctx":
	[PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: file: add `FileDescriptorReservation`":
	[PATCH RFC 13/20] rust_binder: add BINDER_TYPE_FD support
	[PATCH RFC 14/20] rust_binder: add BINDER_TYPE_FDA support

Users of "rust: file: add kuid getters":
	[PATCH RFC 05/20] rust_binder: add nodes and context managers
	[PATCH RFC 06/20] rust_binder: add oneway transactions

Users of "rust: file: add `DeferredFdCloser`":
	[PATCH RFC 14/20] rust_binder: add BINDER_TYPE_FDA support

Users of "rust: file: add abstraction for `poll_table`":
	[PATCH RFC 07/20] rust_binder: add epoll support

This patchset has some uses of read_volatile in place of READ_ONCE.
Please see the following rfc for context on this:
https://lore.kernel.org/all/20231025195339.1431894-1-boqun.feng@gmail.com/

This was previously sent as an rfc:
https://lore.kernel.org/all/20230720152820.3566078-1-aliceryhl@google.com/

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Alice Ryhl (4):
      rust: security: add abstraction for security_secid_to_secctx
      rust: file: add `Kuid` wrapper
      rust: file: add `DeferredFdCloser`
      rust: file: add abstraction for `poll_table`

Wedson Almeida Filho (3):
      rust: file: add Rust abstraction for `struct file`
      rust: cred: add Rust abstraction for `struct cred`
      rust: file: add `FileDescriptorReservation`

 rust/bindings/bindings_helper.h |   9 ++
 rust/bindings/lib.rs            |   1 +
 rust/helpers.c                  |  94 +++++++++++
 rust/kernel/cred.rs             |  73 +++++++++
 rust/kernel/file.rs             | 345 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/file/poll_table.rs  |  97 +++++++++++
 rust/kernel/lib.rs              |   3 +
 rust/kernel/security.rs         |  78 +++++++++
 rust/kernel/sync/condvar.rs     |   2 +-
 rust/kernel/task.rs             |  71 ++++++++-
 10 files changed, 771 insertions(+), 2 deletions(-)
---
base-commit: 98b1cc82c4affc16f5598d4fa14b1858671b2263
change-id: 20231123-alice-file-525b98e8a724

Best regards,

Comments

Christian Brauner Nov. 29, 2023, 4:31 p.m. UTC | #1 
On Wed, Nov 29, 2023 at 12:51:06PM +0000, Alice Ryhl wrote:
> This patchset contains the file abstractions needed by the Rust
> implementation of the Binder driver.
> 
> Please see the Rust Binder RFC for usage examples:
> https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-0-08ba9197f637@google.com/
> 
> Users of "rust: file: add Rust abstraction for `struct file`":
> 	[PATCH RFC 02/20] rust_binder: add binderfs support to Rust binder
> 	[PATCH RFC 03/20] rust_binder: add threading support
> 
> Users of "rust: cred: add Rust abstraction for `struct cred`":
> 	[PATCH RFC 05/20] rust_binder: add nodes and context managers
> 	[PATCH RFC 06/20] rust_binder: add oneway transactions
> 	[PATCH RFC 11/20] rust_binder: send nodes in transaction
> 	[PATCH RFC 13/20] rust_binder: add BINDER_TYPE_FD support
> 
> Users of "rust: security: add abstraction for security_secid_to_secctx":
> 	[PATCH RFC 06/20] rust_binder: add oneway transactions
> 
> Users of "rust: file: add `FileDescriptorReservation`":
> 	[PATCH RFC 13/20] rust_binder: add BINDER_TYPE_FD support
> 	[PATCH RFC 14/20] rust_binder: add BINDER_TYPE_FDA support
> 
> Users of "rust: file: add kuid getters":
> 	[PATCH RFC 05/20] rust_binder: add nodes and context managers
> 	[PATCH RFC 06/20] rust_binder: add oneway transactions
> 
> Users of "rust: file: add `DeferredFdCloser`":
> 	[PATCH RFC 14/20] rust_binder: add BINDER_TYPE_FDA support
> 
> Users of "rust: file: add abstraction for `poll_table`":
> 	[PATCH RFC 07/20] rust_binder: add epoll support
> 
> This patchset has some uses of read_volatile in place of READ_ONCE.
> Please see the following rfc for context on this:
> https://lore.kernel.org/all/20231025195339.1431894-1-boqun.feng@gmail.com/
> 
> This was previously sent as an rfc:
> https://lore.kernel.org/all/20230720152820.3566078-1-aliceryhl@google.com/
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Alice Ryhl (4):
>       rust: security: add abstraction for security_secid_to_secctx
>       rust: file: add `Kuid` wrapper
>       rust: file: add `DeferredFdCloser`
>       rust: file: add abstraction for `poll_table`
> 
> Wedson Almeida Filho (3):
>       rust: file: add Rust abstraction for `struct file`
>       rust: cred: add Rust abstraction for `struct cred`
>       rust: file: add `FileDescriptorReservation`
> 
>  rust/bindings/bindings_helper.h |   9 ++
>  rust/bindings/lib.rs            |   1 +
>  rust/helpers.c                  |  94 +++++++++++
>  rust/kernel/cred.rs             |  73 +++++++++
>  rust/kernel/file.rs             | 345 ++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/file/poll_table.rs  |  97 +++++++++++

That's pretty far away from the subsystem these wrappers belong to. I
would prefer if wrappers such as this would live directly in fs/rust/
and so live within the subsystem they belong to. I think I mentioned
that before. Maybe I missed some sort of agreement here?

>  rust/kernel/lib.rs              |   3 +
>  rust/kernel/security.rs         |  78 +++++++++
>  rust/kernel/sync/condvar.rs     |   2 +-
>  rust/kernel/task.rs             |  71 ++++++++-
>  10 files changed, 771 insertions(+), 2 deletions(-)
> ---
> base-commit: 98b1cc82c4affc16f5598d4fa14b1858671b2263
> change-id: 20231123-alice-file-525b98e8a724
> 
> Best regards,
> -- 
> Alice Ryhl <aliceryhl@google.com>
>
Miguel Ojeda Nov. 29, 2023, 4:48 p.m. UTC | #2 
On Wed, Nov 29, 2023 at 5:31 PM Christian Brauner <brauner@kernel.org> wrote:
>
> That's pretty far away from the subsystem these wrappers belong to. I
> would prefer if wrappers such as this would live directly in fs/rust/
> and so live within the subsystem they belong to. I think I mentioned
> that before. Maybe I missed some sort of agreement here?

The plan is that the code will be moved to the right places when the
new build system is in place (WIP). Currently the "abstractions" go
inside the `kernel` crate.

Cheers,
Miguel
Kent Overstreet Dec. 6, 2023, 8:05 p.m. UTC | #3 
On Wed, Nov 29, 2023 at 05:31:44PM +0100, Christian Brauner wrote:
> On Wed, Nov 29, 2023 at 12:51:06PM +0000, Alice Ryhl wrote:
> > This patchset contains the file abstractions needed by the Rust
> > implementation of the Binder driver.
> > 
> > Please see the Rust Binder RFC for usage examples:
> > https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-0-08ba9197f637@google.com/
> > 
> > Users of "rust: file: add Rust abstraction for `struct file`":
> > 	[PATCH RFC 02/20] rust_binder: add binderfs support to Rust binder
> > 	[PATCH RFC 03/20] rust_binder: add threading support
> > 
> > Users of "rust: cred: add Rust abstraction for `struct cred`":
> > 	[PATCH RFC 05/20] rust_binder: add nodes and context managers
> > 	[PATCH RFC 06/20] rust_binder: add oneway transactions
> > 	[PATCH RFC 11/20] rust_binder: send nodes in transaction
> > 	[PATCH RFC 13/20] rust_binder: add BINDER_TYPE_FD support
> > 
> > Users of "rust: security: add abstraction for security_secid_to_secctx":
> > 	[PATCH RFC 06/20] rust_binder: add oneway transactions
> > 
> > Users of "rust: file: add `FileDescriptorReservation`":
> > 	[PATCH RFC 13/20] rust_binder: add BINDER_TYPE_FD support
> > 	[PATCH RFC 14/20] rust_binder: add BINDER_TYPE_FDA support
> > 
> > Users of "rust: file: add kuid getters":
> > 	[PATCH RFC 05/20] rust_binder: add nodes and context managers
> > 	[PATCH RFC 06/20] rust_binder: add oneway transactions
> > 
> > Users of "rust: file: add `DeferredFdCloser`":
> > 	[PATCH RFC 14/20] rust_binder: add BINDER_TYPE_FDA support
> > 
> > Users of "rust: file: add abstraction for `poll_table`":
> > 	[PATCH RFC 07/20] rust_binder: add epoll support
> > 
> > This patchset has some uses of read_volatile in place of READ_ONCE.
> > Please see the following rfc for context on this:
> > https://lore.kernel.org/all/20231025195339.1431894-1-boqun.feng@gmail.com/
> > 
> > This was previously sent as an rfc:
> > https://lore.kernel.org/all/20230720152820.3566078-1-aliceryhl@google.com/
> > 
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > Alice Ryhl (4):
> >       rust: security: add abstraction for security_secid_to_secctx
> >       rust: file: add `Kuid` wrapper
> >       rust: file: add `DeferredFdCloser`
> >       rust: file: add abstraction for `poll_table`
> > 
> > Wedson Almeida Filho (3):
> >       rust: file: add Rust abstraction for `struct file`
> >       rust: cred: add Rust abstraction for `struct cred`
> >       rust: file: add `FileDescriptorReservation`
> > 
> >  rust/bindings/bindings_helper.h |   9 ++
> >  rust/bindings/lib.rs            |   1 +
> >  rust/helpers.c                  |  94 +++++++++++
> >  rust/kernel/cred.rs             |  73 +++++++++
> >  rust/kernel/file.rs             | 345 ++++++++++++++++++++++++++++++++++++++++
> >  rust/kernel/file/poll_table.rs  |  97 +++++++++++
> 
> That's pretty far away from the subsystem these wrappers belong to. I
> would prefer if wrappers such as this would live directly in fs/rust/
> and so live within the subsystem they belong to. I think I mentioned
> that before. Maybe I missed some sort of agreement here?

I spoke to Miguel about this and it was my understanding that everything
was in place for moving Rust wrappers to the proper directory -
previously there was build system stuff blocking, but he said that's all
working now. Perhaps the memo just didn't get passed down?

(My vote would actually be for fs/ directly, not fs/rust, and a 1:1
mapping between .c files and the .rs files that wrap them).
Miguel Ojeda Dec. 8, 2023, 4:59 p.m. UTC | #4 
On Wed, Dec 6, 2023 at 9:05 PM Kent Overstreet
<kent.overstreet@linux.dev> wrote:
>
> I spoke to Miguel about this and it was my understanding that everything
> was in place for moving Rust wrappers to the proper directory -
> previously there was build system stuff blocking, but he said that's all
> working now. Perhaps the memo just didn't get passed down?

No, it is being worked on (please see my sibling reply).

> (My vote would actually be for fs/ directly, not fs/rust, and a 1:1
> mapping between .c files and the .rs files that wrap them).

Thanks Kent for voting :)

Though note that an exact 1:1 mapping is going to be hard, e.g.
consider nested Rust submodules which would go in folders or
abstractions that you may arrange differently even if they wrap the
same concepts.

But, yeah, one should try to avoid to diverge without a good reason,
of course, especially in the beginning.

Cheers,
Miguel