* [PATCH 3/6] kbuild: remove sed commands after rustc rules
From: Masahiro Yamada @ 2022-12-31 6:42 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Miguel Ojeda, Masahiro Yamada, Alex Gaynor,
Björn Roy Baron, Boqun Feng, Gary Guo, Nathan Chancellor,
Nick Desaulniers, Nicolas Schier, Tom Rix, Wedson Almeida Filho,
llvm, rust-for-linux
In-Reply-To: <20221231064203.1623793-1-masahiroy@kernel.org>
rustc may put comments in dep-info, so sed is used to drop them before
passing it to fixdep.
Now that fixdep can remove comments, Makefiles do not need to run sed.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
rust/Makefile | 6 ++----
scripts/Makefile.build | 18 ++++--------------
scripts/Makefile.host | 3 +--
3 files changed, 7 insertions(+), 20 deletions(-)
diff --git a/rust/Makefile b/rust/Makefile
index 0e2a32f4b3e9..c8941fec6955 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -333,8 +333,7 @@ quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
--emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
--crate-type proc-macro \
- --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<; \
- sed -i '/^\#/d' $(depfile)
+ --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
# Procedural macros can only be used with the `rustc` that compiled it.
# Therefore, to get `libmacros.so` automatically recompiled when the compiler
@@ -349,8 +348,7 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
$(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
--emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
--crate-type rlib -L$(objtree)/$(obj) \
- --crate-name $(patsubst %.o,%,$(notdir $@)) $<; \
- sed -i '/^\#/d' $(depfile) \
+ --crate-name $(patsubst %.o,%,$(notdir $@)) $< \
$(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)
rust-analyzer:
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 40de20246e50..76323201232a 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -289,9 +289,6 @@ rust_common_cmd = \
--crate-name $(basename $(notdir $@)) \
--emit=dep-info=$(depfile)
-rust_handle_depfile = \
- sed -i '/^\#/d' $(depfile)
-
# `--emit=obj`, `--emit=asm` and `--emit=llvm-ir` imply a single codegen unit
# will be used. We explicitly request `-Ccodegen-units=1` in any case, and
# the compiler shows a warning if it is not 1. However, if we ever stop
@@ -301,9 +298,7 @@ rust_handle_depfile = \
# would not match each other.
quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
- cmd_rustc_o_rs = \
- $(rust_common_cmd) --emit=obj=$@ $<; \
- $(rust_handle_depfile)
+ cmd_rustc_o_rs = $(rust_common_cmd) --emit=obj=$@ $<
$(obj)/%.o: $(src)/%.rs FORCE
$(call if_changed_dep,rustc_o_rs)
@@ -311,24 +306,19 @@ $(obj)/%.o: $(src)/%.rs FORCE
quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_rsi_rs = \
$(rust_common_cmd) -Zunpretty=expanded $< >$@; \
- command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@; \
- $(rust_handle_depfile)
+ command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@
$(obj)/%.rsi: $(src)/%.rs FORCE
$(call if_changed_dep,rustc_rsi_rs)
quiet_cmd_rustc_s_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
- cmd_rustc_s_rs = \
- $(rust_common_cmd) --emit=asm=$@ $<; \
- $(rust_handle_depfile)
+ cmd_rustc_s_rs = $(rust_common_cmd) --emit=asm=$@ $<
$(obj)/%.s: $(src)/%.rs FORCE
$(call if_changed_dep,rustc_s_rs)
quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
- cmd_rustc_ll_rs = \
- $(rust_common_cmd) --emit=llvm-ir=$@ $<; \
- $(rust_handle_depfile)
+ cmd_rustc_ll_rs = $(rust_common_cmd) --emit=llvm-ir=$@ $<
$(obj)/%.ll: $(src)/%.rs FORCE
$(call if_changed_dep,rustc_ll_rs)
diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index 4434cdbf7b8e..bc782655d09e 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -148,8 +148,7 @@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE
# host-rust -> Executable
quiet_cmd_host-rust = HOSTRUSTC $@
cmd_host-rust = \
- $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<; \
- sed -i '/^\#/d' $(depfile)
+ $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<
$(host-rust): $(obj)/%: $(src)/%.rs FORCE
$(call if_changed_dep,host-rust)
--
2.34.1
^ permalink raw reply related
* [PATCH 1/6] kbuild: specify output names separately for each emission type from rustc
From: Masahiro Yamada @ 2022-12-31 6:41 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Miguel Ojeda, Masahiro Yamada, Alex Gaynor,
Björn Roy Baron, Boqun Feng, Gary Guo, Nathan Chancellor,
Nick Desaulniers, Nicolas Schier, Tom Rix, Wedson Almeida Filho,
llvm, rust-for-linux
In-Reply-To: <20221231064203.1623793-1-masahiroy@kernel.org>
In Kbuild, two different rules must not write to the same file, but
it happens when compiling rust source files.
For example, set CONFIG_SAMPLE_RUST_MINIMAL=m and run the following:
$ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \
samples/rust/rust_minimal.s samples/rust/rust_minimal.ll
[snip]
RUSTC [M] samples/rust/rust_minimal.o
RUSTC [M] samples/rust/rust_minimal.rsi
RUSTC [M] samples/rust/rust_minimal.s
RUSTC [M] samples/rust/rust_minimal.ll
mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
make[3]: *** [scripts/Makefile.build:334: samples/rust/rust_minimal.ll] Error 1
make[3]: *** Waiting for unfinished jobs....
mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
make[3]: *** [scripts/Makefile.build:309: samples/rust/rust_minimal.o] Error 1
mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
make[3]: *** [scripts/Makefile.build:326: samples/rust/rust_minimal.s] Error 1
make[2]: *** [scripts/Makefile.build:504: samples/rust] Error 2
make[1]: *** [scripts/Makefile.build:504: samples] Error 2
make: *** [Makefile:2008: .] Error 2
The reason for the error is that 4 threads running in parallel creates
and renames the same file path, samples/rust/rust_minimal.d.
This does not happen when compiling C or assembly files because we
explicitly specify the dependency filename by using the preprocessor
option, -Wp,-MMD,$(depfile). $(depfile) is a unique path for each target.
Currently, rustc is only given --out-dir and the list of emitted types.
So, all the rust build rules output the dep-info into the default
<CRATE_NAME>.d, causing the conflict.
Fortunately, the --emit option is able to specify the output path
individually, with the form --emit=<type>=<path>.
Add --emit=dep-info=$(depfile) to the common command part. Also, remove
the redundant --out-dir because we specify the output path for each type.
The code gets much cleaner because we do not need to rename *.d files.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
rust/Makefile | 10 ++++------
scripts/Makefile.build | 14 +++++++-------
scripts/Makefile.host | 9 +++------
3 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/rust/Makefile b/rust/Makefile
index ff70c4c916f8..0e2a32f4b3e9 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -331,10 +331,9 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
cmd_rustc_procmacro = \
$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
- --emit=dep-info,link --extern proc_macro \
- --crate-type proc-macro --out-dir $(objtree)/$(obj) \
+ --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
+ --crate-type proc-macro \
--crate-name $(patsubst lib%.so,%,$(notdir $@)) $<; \
- mv $(objtree)/$(obj)/$(patsubst lib%.so,%,$(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile)
# Procedural macros can only be used with the `rustc` that compiled it.
@@ -348,10 +347,9 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
OBJTREE=$(abspath $(objtree)) \
$(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \
$(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
- --emit=dep-info,obj,metadata --crate-type rlib \
- --out-dir $(objtree)/$(obj) -L$(objtree)/$(obj) \
+ --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
+ --crate-type rlib -L$(objtree)/$(obj) \
--crate-name $(patsubst %.o,%,$(notdir $@)) $<; \
- mv $(objtree)/$(obj)/$(patsubst %.o,%,$(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile) \
$(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index a0d5c6cca76d..40de20246e50 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -285,11 +285,11 @@ rust_common_cmd = \
-Zcrate-attr=no_std \
-Zcrate-attr='feature($(rust_allowed_features))' \
--extern alloc --extern kernel \
- --crate-type rlib --out-dir $(obj) -L $(objtree)/rust/ \
- --crate-name $(basename $(notdir $@))
+ --crate-type rlib -L $(objtree)/rust/ \
+ --crate-name $(basename $(notdir $@)) \
+ --emit=dep-info=$(depfile)
rust_handle_depfile = \
- mv $(obj)/$(basename $(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile)
# `--emit=obj`, `--emit=asm` and `--emit=llvm-ir` imply a single codegen unit
@@ -302,7 +302,7 @@ rust_handle_depfile = \
quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_o_rs = \
- $(rust_common_cmd) --emit=dep-info,obj $<; \
+ $(rust_common_cmd) --emit=obj=$@ $<; \
$(rust_handle_depfile)
$(obj)/%.o: $(src)/%.rs FORCE
@@ -310,7 +310,7 @@ $(obj)/%.o: $(src)/%.rs FORCE
quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_rsi_rs = \
- $(rust_common_cmd) --emit=dep-info -Zunpretty=expanded $< >$@; \
+ $(rust_common_cmd) -Zunpretty=expanded $< >$@; \
command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@; \
$(rust_handle_depfile)
@@ -319,7 +319,7 @@ $(obj)/%.rsi: $(src)/%.rs FORCE
quiet_cmd_rustc_s_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_s_rs = \
- $(rust_common_cmd) --emit=dep-info,asm $<; \
+ $(rust_common_cmd) --emit=asm=$@ $<; \
$(rust_handle_depfile)
$(obj)/%.s: $(src)/%.rs FORCE
@@ -327,7 +327,7 @@ $(obj)/%.s: $(src)/%.rs FORCE
quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_ll_rs = \
- $(rust_common_cmd) --emit=dep-info,llvm-ir $<; \
+ $(rust_common_cmd) --emit=llvm-ir=$@ $<; \
$(rust_handle_depfile)
$(obj)/%.ll: $(src)/%.rs FORCE
diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index da133780b751..4434cdbf7b8e 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -84,8 +84,8 @@ _hostc_flags = $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \
$(HOSTCFLAGS_$(target-stem).o)
_hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \
$(HOSTCXXFLAGS_$(target-stem).o)
-_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
- $(HOSTRUSTFLAGS_$(target-stem))
+hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
+ $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile)
# $(objtree)/$(obj) for including generated headers from checkin source files
ifeq ($(KBUILD_EXTMOD),)
@@ -97,7 +97,6 @@ endif
hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags)
hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
-hostrust_flags = $(_hostrust_flags)
#####
# Compile programs on the host
@@ -149,9 +148,7 @@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE
# host-rust -> Executable
quiet_cmd_host-rust = HOSTRUSTC $@
cmd_host-rust = \
- $(HOSTRUSTC) $(hostrust_flags) --emit=dep-info,link \
- --out-dir=$(obj)/ $<; \
- mv $(obj)/$(target-stem).d $(depfile); \
+ $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<; \
sed -i '/^\#/d' $(depfile)
$(host-rust): $(obj)/%: $(src)/%.rs FORCE
$(call if_changed_dep,host-rust)
--
2.34.1
^ permalink raw reply related
* [PATCH 0/6] kbuild: fix dep-file processing for rust
From: Masahiro Yamada @ 2022-12-31 6:41 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Miguel Ojeda, Masahiro Yamada, Alex Gaynor,
Björn Roy Baron, Boqun Feng, Gary Guo, Nathan Chancellor,
Nick Desaulniers, Nicolas Schier, Tom Rix, Wedson Almeida Filho,
llvm, rust-for-linux
Masahiro Yamada (6):
kbuild: specify output names separately for each emission type from
rustc
fixdep: parse Makefile more correctly to handle comments etc.
kbuild: remove sed commands after rustc rules
fixdep: refactor hash table lookup
fixdep: avoid parsing the same file over again
fixdep: do not parse *.so, *.rmeta, *.rlib
rust/Makefile | 16 ++-
scripts/Makefile.build | 26 ++---
scripts/Makefile.host | 10 +-
scripts/basic/fixdep.c | 234 +++++++++++++++++++++++++++--------------
4 files changed, 173 insertions(+), 113 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: Rust in-kernel TLS handshake
From: FUJITA Tomonori @ 2022-12-29 23:34 UTC (permalink / raw)
To: Alex Gaynor; +Cc: rust-for-linux
In-Reply-To: <CAFRnB2Wp18E_PNpcN3fjVDh0FjSGgC2ACfA+-cDwTBv4p1eZqQ@mail.gmail.com>
Hi,
Thanks for the comments!
On Wed, Dec 28, 2022 at 10:50 PM Alex Gaynor <alex.gaynor@gmail.com> wrote:
> There's many reasons to be leery of complexity in the kernel, but
> security vulnerabilities due to memory unsafety is definitely a big
> one, so I think you're completely correct that Rust can significantly
> change the equation on that risk.
Surely, I'll work on it to see how things work.
> > I worked on QUIC as a consumer of TLS. This can establish a QUIC connection
> > with Quinn's example client, Rust QUIC implementation. Only minimum
> > server side functionality and connection establishment are supported.
> >
> > From the perspective of Rust-for-Linux, the main work is implementing
> > APIs for crypto subsystem. Also libraries such as working with buffers
> > (like Tokio's bytes) would be helpful, I think (should be useful for
> > other projects). I'll work for mainline. Meanwhile you can compile
> > this kernel module
> > with my fork.
> >
>
> I think a Rust API for the crypto subsystem would be great, I'd be
> happy to help review patches for that.
Thanks, I'll try.
> When you say Tokio's bytes, what parts of it are you interested -- the
> lifetime management (e.g. refcounting), or the APIs that are useful
> for handling network protocols (e.g. put_u16)?
Probably, both.
For now I use something simple like the latter for playing with TLS
and QUIC messages.
I guess that a Rust API for the crypto might need the former; passing
buffers to the crypto subsystem via scatterlist.
^ permalink raw reply
* Re: Rust in-kernel TLS handshake
From: Alex Gaynor @ 2022-12-28 13:50 UTC (permalink / raw)
To: FUJITA Tomonori; +Cc: rust-for-linux
In-Reply-To: <CAJ+Zx=3OwXeJNsz8DLp_94ciWpQKoiV75v=XH3FoUQKW==_S8w@mail.gmail.com>
Very cool! I'm excited to see demos like this!
There's many reasons to be leery of complexity in the kernel, but
security vulnerabilities due to memory unsafety is definitely a big
one, so I think you're completely correct that Rust can significantly
change the equation on that risk.
On Tue, Dec 27, 2022 at 8:49 PM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> Hi,
>
> I've started in-kernel TLS handshake implementation in Rust.
> https://github.com/fujita/rust-tls
>
> There is some debate over in-kernel TLS handshake mainly because of
> the complexity. I'd like to see if Rust could help with auditing such
> complicated security-relevant code in the kernel.
>
> I worked on QUIC as a consumer of TLS. This can establish a QUIC connection
> with Quinn's example client, Rust QUIC implementation. Only minimum
> server side functionality and connection establishment are supported.
>
> From the perspective of Rust-for-Linux, the main work is implementing
> APIs for crypto subsystem. Also libraries such as working with buffers
> (like Tokio's bytes) would be helpful, I think (should be useful for
> other projects). I'll work for mainline. Meanwhile you can compile
> this kernel module
> with my fork.
>
I think a Rust API for the crypto subsystem would be great, I'd be
happy to help review patches for that.
When you say Tokio's bytes, what parts of it are you interested -- the
lifetime management (e.g. refcounting), or the APIs that are useful
for handling network protocols (e.g. put_u16)?
> Opinions?
>
> Resend due to a delivery issue. Sorry if you got this twice.
Cheers,
Alex
--
All that is necessary for evil to succeed is for good people to do nothing.
^ permalink raw reply
* Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations
From: Alice Ryhl @ 2022-12-28 10:38 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Will Deacon, Peter Zijlstra,
Mark Rutland
In-Reply-To: <CANeycqqRzOJSiHrSsrJi+VFKjssBtbpaEAWP-z4VVZyiSUnT-w@mail.gmail.com>
On 12/28/22 11:14, Wedson Almeida Filho wrote:
> On Wed, 28 Dec 2022 at 10:02, Alice Ryhl <alice@ryhl.io> wrote:
>>
>> On 12/28/22 07:03, Wedson Almeida Filho wrote:
>>> This is a basic implementation of `Arc` backed by C's `refcount_t`. It
>>> allows Rust code to idiomatically allocate memory that is ref-counted.
>>>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Boqun Feng <boqun.feng@gmail.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
>>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
> Thanks for reviewing!
>
>> Instead of Box::leak, it would be more idiomatic to use Box::into_raw,
>> but both approaches will work.
>
> `Box::into_raw` returns a `*mut T`, whose conversion to `NonNull<T>`
> is fallible (because it could be null). `Box::leak`, OTOH, returns an
> `&mut T`, which cannot be null so it can be converted to `NonNull<T>`
> infallibly.
>
The raw pointer returned by Box::into_raw is guaranteed to be non-null,
so the conversion isn't fallible. You can go through
NonNull::new_unchecked. (It's the same pointer as the one Box::leak
returns, so it must be non-null.)
Regardless, researching more on this topic, it appears that other people
think that going through leak *is* the idiomatic way, even though it
involves going through a reference and back, which is otherwise very
unidiomatic for code dealing with raw pointers.
I am fine with keeping it as-is.
>
>> Regards,
>> Alice Ryhl
>>
>>> ---
>>> rust/bindings/bindings_helper.h | 1 +
>>> rust/bindings/lib.rs | 1 +
>>> rust/helpers.c | 19 ++++
>>> rust/kernel/lib.rs | 1 +
>>> rust/kernel/sync.rs | 10 ++
>>> rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
>>> 6 files changed, 189 insertions(+)
>>> create mode 100644 rust/kernel/sync.rs
>>> create mode 100644 rust/kernel/sync/arc.rs
>>>
>>> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
>>> index c48bc284214a..75d85bd6c592 100644
>>> --- a/rust/bindings/bindings_helper.h
>>> +++ b/rust/bindings/bindings_helper.h
>>> @@ -7,6 +7,7 @@
>>> */
>>>
>>> #include <linux/slab.h>
>>> +#include <linux/refcount.h>
>>>
>>> /* `bindgen` gets confused at certain things. */
>>> const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
>>> diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
>>> index 6c50ee62c56b..7b246454e009 100644
>>> --- a/rust/bindings/lib.rs
>>> +++ b/rust/bindings/lib.rs
>>> @@ -41,6 +41,7 @@ mod bindings_raw {
>>> #[allow(dead_code)]
>>> mod bindings_helper {
>>> // Import the generated bindings for types.
>>> + use super::bindings_raw::*;
>>> include!(concat!(
>>> env!("OBJTREE"),
>>> "/rust/bindings/bindings_helpers_generated.rs"
>>> diff --git a/rust/helpers.c b/rust/helpers.c
>>> index b4f15eee2ffd..09a4d93f9d62 100644
>>> --- a/rust/helpers.c
>>> +++ b/rust/helpers.c
>>> @@ -20,6 +20,7 @@
>>>
>>> #include <linux/bug.h>
>>> #include <linux/build_bug.h>
>>> +#include <linux/refcount.h>
>>>
>>> __noreturn void rust_helper_BUG(void)
>>> {
>>> @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
>>> }
>>> EXPORT_SYMBOL_GPL(rust_helper_BUG);
>>>
>>> +refcount_t rust_helper_REFCOUNT_INIT(int n)
>>> +{
>>> + return (refcount_t)REFCOUNT_INIT(n);
>>> +}
>>> +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
>>> +
>>> +void rust_helper_refcount_inc(refcount_t *r)
>>> +{
>>> + refcount_inc(r);
>>> +}
>>> +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
>>> +
>>> +bool rust_helper_refcount_dec_and_test(refcount_t *r)
>>> +{
>>> + return refcount_dec_and_test(r);
>>> +}
>>> +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
>>> +
>>> /*
>>> * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
>>> * as the Rust `usize` type, so we can use it in contexts where Rust
>>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>>> index 53040fa9e897..ace064a3702a 100644
>>> --- a/rust/kernel/lib.rs
>>> +++ b/rust/kernel/lib.rs
>>> @@ -31,6 +31,7 @@ mod static_assert;
>>> #[doc(hidden)]
>>> pub mod std_vendor;
>>> pub mod str;
>>> +pub mod sync;
>>> pub mod types;
>>>
>>> #[doc(hidden)]
>>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
>>> new file mode 100644
>>> index 000000000000..39b379dd548f
>>> --- /dev/null
>>> +++ b/rust/kernel/sync.rs
>>> @@ -0,0 +1,10 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +//! Synchronisation primitives.
>>> +//!
>>> +//! This module contains the kernel APIs related to synchronisation that have been ported or
>>> +//! wrapped for usage by Rust code in the kernel.
>>> +
>>> +mod arc;
>>> +
>>> +pub use arc::Arc;
>>> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
>>> new file mode 100644
>>> index 000000000000..22290eb5ab9b
>>> --- /dev/null
>>> +++ b/rust/kernel/sync/arc.rs
>>> @@ -0,0 +1,157 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +//! A reference-counted pointer.
>>> +//!
>>> +//! This module implements a way for users to create reference-counted objects and pointers to
>>> +//! them. Such a pointer automatically increments and decrements the count, and drops the
>>> +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
>>> +//! threads.
>>> +//!
>>> +//! It is different from the standard library's [`Arc`] in a few ways:
>>> +//! 1. It is backed by the kernel's `refcount_t` type.
>>> +//! 2. It does not support weak references, which allows it to be half the size.
>>> +//! 3. It saturates the reference count instead of aborting when it goes over a threshold.
>>> +//! 4. It does not provide a `get_mut` method, so the ref counted object is pinned.
>>> +//!
>>> +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
>>> +
>>> +use crate::{bindings, error::Result, types::Opaque};
>>> +use alloc::boxed::Box;
>>> +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
>>> +
>>> +/// A reference-counted pointer to an instance of `T`.
>>> +///
>>> +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
>>> +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// The reference count on an instance of [`Arc`] is always non-zero.
>>> +/// The object pointed to by [`Arc`] is always pinned.
>>> +///
>>> +/// # Examples
>>> +///
>>> +/// ```
>>> +/// use kernel::sync::Arc;
>>> +///
>>> +/// struct Example {
>>> +/// a: u32,
>>> +/// b: u32,
>>> +/// }
>>> +///
>>> +/// // Create a ref-counted instance of `Example`.
>>> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
>>> +///
>>> +/// // Get a new pointer to `obj` and increment the refcount.
>>> +/// let cloned = obj.clone();
>>> +///
>>> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
>>> +/// assert!(core::ptr::eq(&*obj, &*cloned));
>>> +///
>>> +/// // Destroy `obj` and decrement its refcount.
>>> +/// drop(obj);
>>> +///
>>> +/// // Check that the values are still accessible through `cloned`.
>>> +/// assert_eq!(cloned.a, 10);
>>> +/// assert_eq!(cloned.b, 20);
>>> +///
>>> +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
>>> +/// ```
>>> +pub struct Arc<T: ?Sized> {
>>> + ptr: NonNull<ArcInner<T>>,
>>> + _p: PhantomData<ArcInner<T>>,
>>> +}
>>> +
>>> +#[repr(C)]
>>> +struct ArcInner<T: ?Sized> {
>>> + refcount: Opaque<bindings::refcount_t>,
>>> + data: T,
>>> +}
>>> +
>>> +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
>>> +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
>>> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
>>> +// example, when the reference count reaches zero and `T` is dropped.
>>> +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
>>> +
>>> +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
>>> +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
>>> +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
>>> +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
>>> +
>>> +impl<T> Arc<T> {
>>> + /// Constructs a new reference counted instance of `T`.
>>> + pub fn try_new(contents: T) -> Result<Self> {
>>> + // INVARIANT: The refcount is initialised to a non-zero value.
>>> + let value = ArcInner {
>>> + // SAFETY: There are no safety requirements for this FFI call.
>>> + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
>>> + data: contents,
>>> + };
>>> +
>>> + let inner = Box::try_new(value)?;
>>> +
>>> + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
>>> + // `Arc` object.
>>> + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
>>> + }
>>> +}
>>> +
>>> +impl<T: ?Sized> Arc<T> {
>>> + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
>>> + ///
>>> + /// # Safety
>>> + ///
>>> + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
>>> + /// count, one of which will be owned by the new [`Arc`] instance.
>>> + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
>>> + // INVARIANT: By the safety requirements, the invariants hold.
>>> + Arc {
>>> + ptr: inner,
>>> + _p: PhantomData,
>>> + }
>>> + }
>>> +}
>>> +
>>> +impl<T: ?Sized> Deref for Arc<T> {
>>> + type Target = T;
>>> +
>>> + fn deref(&self) -> &Self::Target {
>>> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
>>> + // safe to dereference it.
>>> + unsafe { &self.ptr.as_ref().data }
>>> + }
>>> +}
>>> +
>>> +impl<T: ?Sized> Clone for Arc<T> {
>>> + fn clone(&self) -> Self {
>>> + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
>>> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
>>> + // safe to increment the refcount.
>>> + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
>>> +
>>> + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
>>> + unsafe { Self::from_inner(self.ptr) }
>>> + }
>>> +}
>>> +
>>> +impl<T: ?Sized> Drop for Arc<T> {
>>> + fn drop(&mut self) {
>>> + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
>>> + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
>>> + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
>>> + // freed/invalid memory as long as it is never dereferenced.
>>> + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
>>> +
>>> + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
>>> + // this instance is being dropped, so the broken invariant is not observable.
>>> + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
>>> + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
>>> + if is_zero {
>>> + // The count reached zero, we must free the memory.
>>> + //
>>> + // SAFETY: The pointer was initialised from the result of `Box::leak`.
>>> + unsafe { Box::from_raw(self.ptr.as_ptr()) };
>>> + }
>>> + }
>>> +}
^ permalink raw reply
* Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations
From: Wedson Almeida Filho @ 2022-12-28 10:14 UTC (permalink / raw)
To: Alice Ryhl
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Will Deacon, Peter Zijlstra,
Mark Rutland
In-Reply-To: <a23d715d-19c1-dff4-f7e6-504ebb0e2c6c@ryhl.io>
On Wed, 28 Dec 2022 at 10:02, Alice Ryhl <alice@ryhl.io> wrote:
>
> On 12/28/22 07:03, Wedson Almeida Filho wrote:
> > This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> > allows Rust code to idiomatically allocate memory that is ref-counted.
> >
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Boqun Feng <boqun.feng@gmail.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Thanks for reviewing!
> Instead of Box::leak, it would be more idiomatic to use Box::into_raw,
> but both approaches will work.
`Box::into_raw` returns a `*mut T`, whose conversion to `NonNull<T>`
is fallible (because it could be null). `Box::leak`, OTOH, returns an
`&mut T`, which cannot be null so it can be converted to `NonNull<T>`
infallibly.
> Regards,
> Alice Ryhl
>
> > ---
> > rust/bindings/bindings_helper.h | 1 +
> > rust/bindings/lib.rs | 1 +
> > rust/helpers.c | 19 ++++
> > rust/kernel/lib.rs | 1 +
> > rust/kernel/sync.rs | 10 ++
> > rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> > 6 files changed, 189 insertions(+)
> > create mode 100644 rust/kernel/sync.rs
> > create mode 100644 rust/kernel/sync/arc.rs
> >
> > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> > index c48bc284214a..75d85bd6c592 100644
> > --- a/rust/bindings/bindings_helper.h
> > +++ b/rust/bindings/bindings_helper.h
> > @@ -7,6 +7,7 @@
> > */
> >
> > #include <linux/slab.h>
> > +#include <linux/refcount.h>
> >
> > /* `bindgen` gets confused at certain things. */
> > const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> > diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> > index 6c50ee62c56b..7b246454e009 100644
> > --- a/rust/bindings/lib.rs
> > +++ b/rust/bindings/lib.rs
> > @@ -41,6 +41,7 @@ mod bindings_raw {
> > #[allow(dead_code)]
> > mod bindings_helper {
> > // Import the generated bindings for types.
> > + use super::bindings_raw::*;
> > include!(concat!(
> > env!("OBJTREE"),
> > "/rust/bindings/bindings_helpers_generated.rs"
> > diff --git a/rust/helpers.c b/rust/helpers.c
> > index b4f15eee2ffd..09a4d93f9d62 100644
> > --- a/rust/helpers.c
> > +++ b/rust/helpers.c
> > @@ -20,6 +20,7 @@
> >
> > #include <linux/bug.h>
> > #include <linux/build_bug.h>
> > +#include <linux/refcount.h>
> >
> > __noreturn void rust_helper_BUG(void)
> > {
> > @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> > }
> > EXPORT_SYMBOL_GPL(rust_helper_BUG);
> >
> > +refcount_t rust_helper_REFCOUNT_INIT(int n)
> > +{
> > + return (refcount_t)REFCOUNT_INIT(n);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> > +
> > +void rust_helper_refcount_inc(refcount_t *r)
> > +{
> > + refcount_inc(r);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> > +
> > +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> > +{
> > + return refcount_dec_and_test(r);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> > +
> > /*
> > * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> > * as the Rust `usize` type, so we can use it in contexts where Rust
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index 53040fa9e897..ace064a3702a 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -31,6 +31,7 @@ mod static_assert;
> > #[doc(hidden)]
> > pub mod std_vendor;
> > pub mod str;
> > +pub mod sync;
> > pub mod types;
> >
> > #[doc(hidden)]
> > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > new file mode 100644
> > index 000000000000..39b379dd548f
> > --- /dev/null
> > +++ b/rust/kernel/sync.rs
> > @@ -0,0 +1,10 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Synchronisation primitives.
> > +//!
> > +//! This module contains the kernel APIs related to synchronisation that have been ported or
> > +//! wrapped for usage by Rust code in the kernel.
> > +
> > +mod arc;
> > +
> > +pub use arc::Arc;
> > diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> > new file mode 100644
> > index 000000000000..22290eb5ab9b
> > --- /dev/null
> > +++ b/rust/kernel/sync/arc.rs
> > @@ -0,0 +1,157 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! A reference-counted pointer.
> > +//!
> > +//! This module implements a way for users to create reference-counted objects and pointers to
> > +//! them. Such a pointer automatically increments and decrements the count, and drops the
> > +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> > +//! threads.
> > +//!
> > +//! It is different from the standard library's [`Arc`] in a few ways:
> > +//! 1. It is backed by the kernel's `refcount_t` type.
> > +//! 2. It does not support weak references, which allows it to be half the size.
> > +//! 3. It saturates the reference count instead of aborting when it goes over a threshold.
> > +//! 4. It does not provide a `get_mut` method, so the ref counted object is pinned.
> > +//!
> > +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> > +
> > +use crate::{bindings, error::Result, types::Opaque};
> > +use alloc::boxed::Box;
> > +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> > +
> > +/// A reference-counted pointer to an instance of `T`.
> > +///
> > +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> > +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> > +///
> > +/// # Invariants
> > +///
> > +/// The reference count on an instance of [`Arc`] is always non-zero.
> > +/// The object pointed to by [`Arc`] is always pinned.
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::sync::Arc;
> > +///
> > +/// struct Example {
> > +/// a: u32,
> > +/// b: u32,
> > +/// }
> > +///
> > +/// // Create a ref-counted instance of `Example`.
> > +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> > +///
> > +/// // Get a new pointer to `obj` and increment the refcount.
> > +/// let cloned = obj.clone();
> > +///
> > +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> > +/// assert!(core::ptr::eq(&*obj, &*cloned));
> > +///
> > +/// // Destroy `obj` and decrement its refcount.
> > +/// drop(obj);
> > +///
> > +/// // Check that the values are still accessible through `cloned`.
> > +/// assert_eq!(cloned.a, 10);
> > +/// assert_eq!(cloned.b, 20);
> > +///
> > +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> > +/// ```
> > +pub struct Arc<T: ?Sized> {
> > + ptr: NonNull<ArcInner<T>>,
> > + _p: PhantomData<ArcInner<T>>,
> > +}
> > +
> > +#[repr(C)]
> > +struct ArcInner<T: ?Sized> {
> > + refcount: Opaque<bindings::refcount_t>,
> > + data: T,
> > +}
> > +
> > +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> > +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> > +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> > +// example, when the reference count reaches zero and `T` is dropped.
> > +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> > +
> > +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> > +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> > +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> > +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> > +
> > +impl<T> Arc<T> {
> > + /// Constructs a new reference counted instance of `T`.
> > + pub fn try_new(contents: T) -> Result<Self> {
> > + // INVARIANT: The refcount is initialised to a non-zero value.
> > + let value = ArcInner {
> > + // SAFETY: There are no safety requirements for this FFI call.
> > + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> > + data: contents,
> > + };
> > +
> > + let inner = Box::try_new(value)?;
> > +
> > + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> > + // `Arc` object.
> > + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Arc<T> {
> > + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> > + /// count, one of which will be owned by the new [`Arc`] instance.
> > + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> > + // INVARIANT: By the safety requirements, the invariants hold.
> > + Arc {
> > + ptr: inner,
> > + _p: PhantomData,
> > + }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Deref for Arc<T> {
> > + type Target = T;
> > +
> > + fn deref(&self) -> &Self::Target {
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> > + // safe to dereference it.
> > + unsafe { &self.ptr.as_ref().data }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Clone for Arc<T> {
> > + fn clone(&self) -> Self {
> > + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> > + // safe to increment the refcount.
> > + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
> > +
> > + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> > + unsafe { Self::from_inner(self.ptr) }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Drop for Arc<T> {
> > + fn drop(&mut self) {
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> > + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> > + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> > + // freed/invalid memory as long as it is never dereferenced.
> > + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> > +
> > + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> > + // this instance is being dropped, so the broken invariant is not observable.
> > + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> > + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> > + if is_zero {
> > + // The count reached zero, we must free the memory.
> > + //
> > + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> > + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> > + }
> > + }
> > +}
^ permalink raw reply
* Re: [PATCH 7/7] rust: sync: add support for dispatching on Arc and ArcBorrow.
From: Alice Ryhl @ 2022-12-28 10:04 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel
In-Reply-To: <20221228060346.352362-7-wedsonaf@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> Trait objects (`dyn T`) require trait `T` to be "object safe". One of
> the requirements for "object safety" is that the receiver have one of
> the allowed types. This commit adds `Arc<T>` and `ArcBorrow<'_, T>` to
> the list of allowed types.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync/arc.rs | 20 ++++++++++++++++++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 4bde65e7b06b..e0b0e953907d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -15,6 +15,7 @@
> #![feature(allocator_api)]
> #![feature(coerce_unsized)]
> #![feature(core_ffi_c)]
> +#![feature(dispatch_from_dyn)]
> #![feature(receiver_trait)]
> #![feature(unsize)]
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 832bafc74a90..ff73f9240ca1 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -92,9 +92,15 @@ use core::{
> /// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
> ///
> /// ```
> -/// use kernel::sync::Arc;
> +/// use kernel::sync::{Arc, ArcBorrow};
> +///
> +/// trait MyTrait {
> +/// // Trait has a function whose `self` type is `Arc<Self>`.
> +/// fn example1(self: Arc<Self>) {}
> ///
> -/// trait MyTrait {}
> +/// // Trait has a function whose `self` type is `ArcBorrow<'_, Self>`.
> +/// fn example2(self: ArcBorrow<'_, Self>) {}
> +/// }
> ///
> /// struct Example;
> /// impl MyTrait for Example {}
> @@ -123,6 +129,9 @@ impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
> // dynamically-sized type (DST) `U`.
> impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
>
> +// This is to allow `Arc<U>` to be dispatched on when `Arc<T>` can be coerced into `Arc<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> @@ -297,6 +306,13 @@ pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> // This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}
>
> +// This is to allow `ArcBorrow<U>` to be dispatched on when `ArcBorrow<T>` can be coerced into
> +// `ArcBorrow<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'_, U>>
> + for ArcBorrow<'_, T>
> +{
> +}
> +
> impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> fn clone(&self) -> Self {
> *self
^ permalink raw reply
* Re: [PATCH 6/7] rust: sync: introduce `UniqueArc`
From: Alice Ryhl @ 2022-12-28 10:04 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel
In-Reply-To: <20221228060346.352362-6-wedsonaf@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> Since `Arc<T>` does not allow mutating `T` directly (i.e., without inner
> mutability), it is currently not possible to do some initialisation of
> `T` post construction but before being shared.
>
> `UniqueArc<T>` addresses this problem essentially being an `Arc<T>` that
> has a refcount of 1 and is therefore writable. Once initialisation is
> completed, it can be transitioned (without failure paths) into an
> `Arc<T>`.
>
> Suggested-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/sync.rs | 2 +-
> rust/kernel/sync/arc.rs | 152 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 151 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 5de03ea83ea1..33da23e3076d 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>
> mod arc;
>
> -pub use arc::{Arc, ArcBorrow};
> +pub use arc::{Arc, ArcBorrow, UniqueArc};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 84f31c85a513..832bafc74a90 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,8 +19,9 @@ use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> use core::{
> marker::{PhantomData, Unsize},
> - mem::ManuallyDrop,
> - ops::Deref,
> + mem::{ManuallyDrop, MaybeUninit},
> + ops::{Deref, DerefMut},
> + pin::Pin,
> ptr::NonNull,
> };
>
> @@ -222,6 +223,19 @@ impl<T: ?Sized> Drop for Arc<T> {
> }
> }
>
> +impl<T: ?Sized> From<UniqueArc<T>> for Arc<T> {
> + fn from(item: UniqueArc<T>) -> Self {
> + item.inner
> + }
> +}
> +
> +impl<T: ?Sized> From<Pin<UniqueArc<T>>> for Arc<T> {
> + fn from(item: Pin<UniqueArc<T>>) -> Self {
> + // SAFETY: The type invariants of `Arc` guarantee that the data is pinned.
> + unsafe { Pin::into_inner_unchecked(item).inner }
> + }
> +}
> +
> /// A borrowed reference to an [`Arc`] instance.
> ///
> /// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler
> @@ -328,3 +342,137 @@ impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> unsafe { &self.inner.as_ref().data }
> }
> }
> +
> +/// A refcounted object that is known to have a refcount of 1.
> +///
> +/// It is mutable and can be converted to an [`Arc`] so that it can be shared.
> +///
> +/// # Invariants
> +///
> +/// `inner` always has a reference count of 1.
> +///
> +/// # Examples
> +///
> +/// In the following example, we make changes to the inner object before turning it into an
> +/// `Arc<Test>` object (after which point, it cannot be mutated directly). Note that `x.into()`
> +/// cannot fail.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let mut x = UniqueArc::try_new(Example { a: 10, b: 20 })?;
> +/// x.a += 1;
> +/// x.b += 1;
> +/// Ok(x.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the following example we first allocate memory for a ref-counted `Example` but we don't
> +/// initialise it on allocation. We do initialise it later with a call to [`UniqueArc::write`],
> +/// followed by a conversion to `Arc<Example>`. This is particularly useful when allocation happens
> +/// in one context (e.g., sleepable) and initialisation in another (e.g., atomic):
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let x = UniqueArc::try_new_uninit()?;
> +/// Ok(x.write(Example { a: 10, b: 20 }).into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the last example below, the caller gets a pinned instance of `Example` while converting to
> +/// `Arc<Example>`; this is useful in scenarios where one needs a pinned reference during
> +/// initialisation, for example, when initialising fields that are wrapped in locks.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let mut pinned = Pin::from(UniqueArc::try_new(Example { a: 10, b: 20 })?);
> +/// // We can modify `pinned` because it is `Unpin`.
> +/// pinned.as_mut().a += 1;
> +/// Ok(pinned.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +pub struct UniqueArc<T: ?Sized> {
> + inner: Arc<T>,
> +}
> +
> +impl<T> UniqueArc<T> {
> + /// Tries to allocate a new [`UniqueArc`] instance.
> + pub fn try_new(value: T) -> Result<Self> {
> + Ok(Self {
> + // INVARIANT: The newly-created object has a ref-count of 1.
> + inner: Arc::try_new(value)?,
> + })
> + }
> +
> + /// Tries to allocate a new [`UniqueArc`] instance whose contents are not initialised yet.
> + pub fn try_new_uninit() -> Result<UniqueArc<MaybeUninit<T>>> {
> + Ok(UniqueArc::<MaybeUninit<T>> {
> + // INVARIANT: The newly-created object has a ref-count of 1.
> + inner: Arc::try_new(MaybeUninit::uninit())?,
> + })
> + }
> +}
> +
> +impl<T> UniqueArc<MaybeUninit<T>> {
> + /// Converts a `UniqueArc<MaybeUninit<T>>` into a `UniqueArc<T>` by writing a value into it.
> + pub fn write(mut self, value: T) -> UniqueArc<T> {
> + self.deref_mut().write(value);
> + let inner = ManuallyDrop::new(self).inner.ptr;
> + UniqueArc {
> + // SAFETY: The new `Arc` is taking over `ptr` from `self.inner` (which won't be
> + // dropped). The types are compatible because `MaybeUninit<T>` is compatible with `T`.
> + inner: unsafe { Arc::from_inner(inner.cast()) },
> + }
> + }
> +}
> +
> +impl<T: ?Sized> From<UniqueArc<T>> for Pin<UniqueArc<T>> {
> + fn from(obj: UniqueArc<T>) -> Self {
> + // SAFETY: It is not possible to move/replace `T` inside a `Pin<UniqueArc<T>>` (unless `T`
> + // is `Unpin`), so it is ok to convert it to `Pin<UniqueArc<T>>`.
> + unsafe { Pin::new_unchecked(obj) }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for UniqueArc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + self.inner.deref()
> + }
> +}
> +
> +impl<T: ?Sized> DerefMut for UniqueArc<T> {
> + fn deref_mut(&mut self) -> &mut Self::Target {
> + // SAFETY: By the `Arc` type invariant, there is necessarily a reference to the object, so
> + // it is safe to dereference it. Additionally, we know there is only one reference when
> + // it's inside a `UniqueArc`, so it is safe to get a mutable reference.
> + unsafe { &mut self.inner.ptr.as_mut().data }
> + }
> +}
^ permalink raw reply
* Re: [PATCH 5/7] rust: sync: allow type of `self` to be `ArcBorrow<T>`
From: Alice Ryhl @ 2022-12-28 10:04 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel
In-Reply-To: <20221228060346.352362-5-wedsonaf@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> This allows associated functions whose `self` argument has
> `ArcBorrow<T>` as their type. This, in turn, allows callers to use the
> dot syntax to make calls.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/sync/arc.rs | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index f68bfc02c81a..84f31c85a513 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -255,11 +255,34 @@ impl<T: ?Sized> Drop for Arc<T> {
> /// // Assert that both `obj` and `cloned` point to the same underlying object.
> /// assert!(core::ptr::eq(&*obj, &*cloned));
> /// ```
> +///
> +/// Using `ArcBorrow<T>` as the type of `self`:
> +///
> +/// ```
> +/// use crate::sync::{Arc, ArcBorrow};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// impl Example {
> +/// fn use_reference(self: ArcBorrow<'_, Self>) {
> +/// // ...
> +/// }
> +/// }
> +///
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +/// obj.as_arc_borrow().use_reference();
> +/// ```
> pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> inner: NonNull<ArcInner<T>>,
> _p: PhantomData<&'a ()>,
> }
>
> +// This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
> +impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}
> +
> impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> fn clone(&self) -> Self {
> *self
^ permalink raw reply
* Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`
From: Alice Ryhl @ 2022-12-28 10:03 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel
In-Reply-To: <20221228060346.352362-4-wedsonaf@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> This allows us to create references to a ref-counted allocation without
> double-indirection and that still allow us to increment the refcount to
> a new `Arc<T>`.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/sync.rs | 2 +-
> rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 39b379dd548f..5de03ea83ea1 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>
> mod arc;
>
> -pub use arc::Arc;
> +pub use arc::{Arc, ArcBorrow};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index dbc7596cc3ce..f68bfc02c81a 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> use core::{
> marker::{PhantomData, Unsize},
> + mem::ManuallyDrop,
> ops::Deref,
> ptr::NonNull,
> };
> @@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
> _p: PhantomData,
> }
> }
> +
> + /// Returns an [`ArcBorrow`] from the given [`Arc`].
> + ///
> + /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
> + /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
> + #[inline]
> + pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
> + // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
> + // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
> + // reference can be created.
> + unsafe { ArcBorrow::new(self.ptr) }
> + }
> }
>
> impl<T: ?Sized> Deref for Arc<T> {
> @@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
> }
> }
> }
> +
> +/// A borrowed reference to an [`Arc`] instance.
> +///
> +/// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler
> +/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
> +///
> +/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
> +/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
> +/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
> +/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
> +/// needed.
> +///
> +/// # Invariants
> +///
> +/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
> +/// lifetime of the [`ArcBorrow`] instance.
> +///
> +/// # Example
> +///
> +/// ```
> +/// use crate::sync::{Arc, ArcBorrow};
> +///
> +/// struct Example;
> +///
> +/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
> +/// e.into()
> +/// }
> +///
> +/// let obj = Arc::try_new(Example)?;
> +/// let cloned = do_something(obj.as_arc_borrow());
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +/// ```
> +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> + inner: NonNull<ArcInner<T>>,
> + _p: PhantomData<&'a ()>,
> +}
> +
> +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> + fn clone(&self) -> Self {
> + *self
> + }
> +}
> +
> +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
> +
> +impl<T: ?Sized> ArcBorrow<'_, T> {
> + /// Creates a new [`ArcBorrow`] instance.
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance:
> + /// 1. That `inner` remains valid;
> + /// 2. That no mutable references to `inner` are created.
> + unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
> + // INVARIANT: The safety requirements guarantee the invariants.
> + Self {
> + inner,
> + _p: PhantomData,
> + }
> + }
> +}
> +
> +impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
> + fn from(b: ArcBorrow<'_, T>) -> Self {
> + // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
> + // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
> + // increment.
> + ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) })
> + .deref()
> + .clone()
> + }
> +}
> +
> +impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + // SAFETY: By the type invariant, the underlying object is still alive with no mutable
> + // references to it, so it is safe to create a shared reference.
> + unsafe { &self.inner.as_ref().data }
> + }
> +}
^ permalink raw reply
* Re: [PATCH 3/7] rust: sync: allow coercion from `Arc<T>` to `Arc<U>`
From: Alice Ryhl @ 2022-12-28 10:03 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel
In-Reply-To: <20221228060346.352362-3-wedsonaf@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> The coercion is only allowed if `U` is a compatible dynamically-sized
> type (DST). For example, if we have some type `X` that implements trait
> `Y`, then this allows `Arc<X>` to be coerced into `Arc<dyn Y>`.
>
> Suggested-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/lib.rs | 2 ++
> rust/kernel/sync/arc.rs | 27 ++++++++++++++++++++++++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 1a10f7c0ddd9..4bde65e7b06b 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -13,8 +13,10 @@
>
> #![no_std]
> #![feature(allocator_api)]
> +#![feature(coerce_unsized)]
> #![feature(core_ffi_c)]
> #![feature(receiver_trait)]
> +#![feature(unsize)]
>
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index e2eb0e67d483..dbc7596cc3ce 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -17,7 +17,11 @@
>
> use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> -use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> +use core::{
> + marker::{PhantomData, Unsize},
> + ops::Deref,
> + ptr::NonNull,
> +};
>
> /// A reference-counted pointer to an instance of `T`.
> ///
> @@ -82,6 +86,23 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> /// obj.use_reference();
> /// obj.take_over();
> /// ```
> +///
> +/// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// trait MyTrait {}
> +///
> +/// struct Example;
> +/// impl MyTrait for Example {}
> +///
> +/// // `obj` has type `Arc<Example>`.
> +/// let obj: Arc<Example> = Arc::try_new(Example)?;
> +///
> +/// // `coerced` has type `Arc<dyn MyTrait>`.
> +/// let coerced: Arc<dyn MyTrait> = obj;
> +/// ```
> pub struct Arc<T: ?Sized> {
> ptr: NonNull<ArcInner<T>>,
> _p: PhantomData<ArcInner<T>>,
> @@ -96,6 +117,10 @@ struct ArcInner<T: ?Sized> {
> // This is to allow [`Arc`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
>
> +// This is to allow coercion from `Arc<T>` to `Arc<U>` if `T` can be converted to the
> +// dynamically-sized type (DST) `U`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
^ permalink raw reply
* Re: [PATCH 2/7] rust: sync: allow type of `self` to be `Arc<T>` or variants
From: Alice Ryhl @ 2022-12-28 10:03 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel
In-Reply-To: <20221228060346.352362-2-wedsonaf@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> This allows associated functions whose `self` argument has `Arc<T>` or
> variants as their type. This, in turn, allows callers to use the dot
> syntax to make calls.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync/arc.rs | 28 ++++++++++++++++++++++++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index ace064a3702a..1a10f7c0ddd9 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -14,6 +14,7 @@
> #![no_std]
> #![feature(allocator_api)]
> #![feature(core_ffi_c)]
> +#![feature(receiver_trait)]
>
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 22290eb5ab9b..e2eb0e67d483 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -57,6 +57,31 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> ///
> /// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> /// ```
> +///
> +/// Using `Arc<T>` as the type of `self`:
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// impl Example {
> +/// fn take_over(self: Arc<Self>) {
> +/// // ...
> +/// }
> +///
> +/// fn use_reference(self: &Arc<Self>) {
> +/// // ...
> +/// }
> +/// }
> +///
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +/// obj.use_reference();
> +/// obj.take_over();
> +/// ```
> pub struct Arc<T: ?Sized> {
> ptr: NonNull<ArcInner<T>>,
> _p: PhantomData<ArcInner<T>>,
> @@ -68,6 +93,9 @@ struct ArcInner<T: ?Sized> {
> data: T,
> }
>
> +// This is to allow [`Arc`] (and variants) to be used as the type of `self`.
> +impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
^ permalink raw reply
* Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations
From: Alice Ryhl @ 2022-12-28 10:03 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Will Deacon, Peter Zijlstra,
Mark Rutland
In-Reply-To: <20221228060346.352362-1-wedsonaf@gmail.com>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> allows Rust code to idiomatically allocate memory that is ref-counted.
>
> Cc: Will Deacon <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Instead of Box::leak, it would be more idiomatic to use Box::into_raw,
but both approaches will work.
Regards,
Alice Ryhl
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/bindings/lib.rs | 1 +
> rust/helpers.c | 19 ++++
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync.rs | 10 ++
> rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> 6 files changed, 189 insertions(+)
> create mode 100644 rust/kernel/sync.rs
> create mode 100644 rust/kernel/sync/arc.rs
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index c48bc284214a..75d85bd6c592 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/slab.h>
> +#include <linux/refcount.h>
>
> /* `bindgen` gets confused at certain things. */
> const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> index 6c50ee62c56b..7b246454e009 100644
> --- a/rust/bindings/lib.rs
> +++ b/rust/bindings/lib.rs
> @@ -41,6 +41,7 @@ mod bindings_raw {
> #[allow(dead_code)]
> mod bindings_helper {
> // Import the generated bindings for types.
> + use super::bindings_raw::*;
> include!(concat!(
> env!("OBJTREE"),
> "/rust/bindings/bindings_helpers_generated.rs"
> diff --git a/rust/helpers.c b/rust/helpers.c
> index b4f15eee2ffd..09a4d93f9d62 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -20,6 +20,7 @@
>
> #include <linux/bug.h>
> #include <linux/build_bug.h>
> +#include <linux/refcount.h>
>
> __noreturn void rust_helper_BUG(void)
> {
> @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> }
> EXPORT_SYMBOL_GPL(rust_helper_BUG);
>
> +refcount_t rust_helper_REFCOUNT_INIT(int n)
> +{
> + return (refcount_t)REFCOUNT_INIT(n);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> +
> +void rust_helper_refcount_inc(refcount_t *r)
> +{
> + refcount_inc(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> +
> +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> +{
> + return refcount_dec_and_test(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> +
> /*
> * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 53040fa9e897..ace064a3702a 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -31,6 +31,7 @@ mod static_assert;
> #[doc(hidden)]
> pub mod std_vendor;
> pub mod str;
> +pub mod sync;
> pub mod types;
>
> #[doc(hidden)]
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> new file mode 100644
> index 000000000000..39b379dd548f
> --- /dev/null
> +++ b/rust/kernel/sync.rs
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Synchronisation primitives.
> +//!
> +//! This module contains the kernel APIs related to synchronisation that have been ported or
> +//! wrapped for usage by Rust code in the kernel.
> +
> +mod arc;
> +
> +pub use arc::Arc;
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> new file mode 100644
> index 000000000000..22290eb5ab9b
> --- /dev/null
> +++ b/rust/kernel/sync/arc.rs
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A reference-counted pointer.
> +//!
> +//! This module implements a way for users to create reference-counted objects and pointers to
> +//! them. Such a pointer automatically increments and decrements the count, and drops the
> +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> +//! threads.
> +//!
> +//! It is different from the standard library's [`Arc`] in a few ways:
> +//! 1. It is backed by the kernel's `refcount_t` type.
> +//! 2. It does not support weak references, which allows it to be half the size.
> +//! 3. It saturates the reference count instead of aborting when it goes over a threshold.
> +//! 4. It does not provide a `get_mut` method, so the ref counted object is pinned.
> +//!
> +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> +
> +use crate::{bindings, error::Result, types::Opaque};
> +use alloc::boxed::Box;
> +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> +
> +/// A reference-counted pointer to an instance of `T`.
> +///
> +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> +///
> +/// # Invariants
> +///
> +/// The reference count on an instance of [`Arc`] is always non-zero.
> +/// The object pointed to by [`Arc`] is always pinned.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// // Create a ref-counted instance of `Example`.
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +///
> +/// // Get a new pointer to `obj` and increment the refcount.
> +/// let cloned = obj.clone();
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +///
> +/// // Destroy `obj` and decrement its refcount.
> +/// drop(obj);
> +///
> +/// // Check that the values are still accessible through `cloned`.
> +/// assert_eq!(cloned.a, 10);
> +/// assert_eq!(cloned.b, 20);
> +///
> +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> +/// ```
> +pub struct Arc<T: ?Sized> {
> + ptr: NonNull<ArcInner<T>>,
> + _p: PhantomData<ArcInner<T>>,
> +}
> +
> +#[repr(C)]
> +struct ArcInner<T: ?Sized> {
> + refcount: Opaque<bindings::refcount_t>,
> + data: T,
> +}
> +
> +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> +// example, when the reference count reaches zero and `T` is dropped.
> +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> +
> +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> +
> +impl<T> Arc<T> {
> + /// Constructs a new reference counted instance of `T`.
> + pub fn try_new(contents: T) -> Result<Self> {
> + // INVARIANT: The refcount is initialised to a non-zero value.
> + let value = ArcInner {
> + // SAFETY: There are no safety requirements for this FFI call.
> + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> + data: contents,
> + };
> +
> + let inner = Box::try_new(value)?;
> +
> + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> + // `Arc` object.
> + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> + }
> +}
> +
> +impl<T: ?Sized> Arc<T> {
> + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> + /// count, one of which will be owned by the new [`Arc`] instance.
> + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> + // INVARIANT: By the safety requirements, the invariants hold.
> + Arc {
> + ptr: inner,
> + _p: PhantomData,
> + }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for Arc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to dereference it.
> + unsafe { &self.ptr.as_ref().data }
> + }
> +}
> +
> +impl<T: ?Sized> Clone for Arc<T> {
> + fn clone(&self) -> Self {
> + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to increment the refcount.
> + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
> +
> + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> + unsafe { Self::from_inner(self.ptr) }
> + }
> +}
> +
> +impl<T: ?Sized> Drop for Arc<T> {
> + fn drop(&mut self) {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> + // freed/invalid memory as long as it is never dereferenced.
> + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> +
> + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> + // this instance is being dropped, so the broken invariant is not observable.
> + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> + if is_zero {
> + // The count reached zero, we must free the memory.
> + //
> + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> + }
> + }
> +}
^ permalink raw reply
* Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations
From: Laine Taffin Altman @ 2022-12-28 7:32 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Will Deacon, Peter Zijlstra,
Mark Rutland
In-Reply-To: <Y6vvxWUkJD7rCbgP@wedsonaf-dev>
On Dec 27, 2022, at 11:27 PM, Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> I suggest that you read the
> comments at the top of refcount.h?
Thank you for correcting me! You’re absolutely right, and I completely misunderstood how `refcount_t` works. Sorry for the noise! I retract my comments on those two patches (the other two comments are unrelated).
— Laine Taffin Altman
^ permalink raw reply
* Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations
From: Wedson Almeida Filho @ 2022-12-28 7:27 UTC (permalink / raw)
To: Laine Taffin Altman
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Will Deacon, Peter Zijlstra,
Mark Rutland
In-Reply-To: <B3B7C271-4C96-455E-A990-2AC7C52F703E@me.com>
On Tue, Dec 27, 2022 at 11:09:57PM -0800, Laine Taffin Altman wrote:
> On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
> >
> > This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> > allows Rust code to idiomatically allocate memory that is ref-counted.
> >
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Boqun Feng <boqun.feng@gmail.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> > ---
> > rust/bindings/bindings_helper.h | 1 +
> > rust/bindings/lib.rs | 1 +
> > rust/helpers.c | 19 ++++
> > rust/kernel/lib.rs | 1 +
> > rust/kernel/sync.rs | 10 ++
> > rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> > 6 files changed, 189 insertions(+)
> > create mode 100644 rust/kernel/sync.rs
> > create mode 100644 rust/kernel/sync/arc.rs
> >
> > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> > index c48bc284214a..75d85bd6c592 100644
> > --- a/rust/bindings/bindings_helper.h
> > +++ b/rust/bindings/bindings_helper.h
> > @@ -7,6 +7,7 @@
> > */
> >
> > #include <linux/slab.h>
> > +#include <linux/refcount.h>
> >
> > /* `bindgen` gets confused at certain things. */
> > const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> > diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> > index 6c50ee62c56b..7b246454e009 100644
> > --- a/rust/bindings/lib.rs
> > +++ b/rust/bindings/lib.rs
> > @@ -41,6 +41,7 @@ mod bindings_raw {
> > #[allow(dead_code)]
> > mod bindings_helper {
> > // Import the generated bindings for types.
> > + use super::bindings_raw::*;
> > include!(concat!(
> > env!("OBJTREE"),
> > "/rust/bindings/bindings_helpers_generated.rs"
> > diff --git a/rust/helpers.c b/rust/helpers.c
> > index b4f15eee2ffd..09a4d93f9d62 100644
> > --- a/rust/helpers.c
> > +++ b/rust/helpers.c
> > @@ -20,6 +20,7 @@
> >
> > #include <linux/bug.h>
> > #include <linux/build_bug.h>
> > +#include <linux/refcount.h>
> >
> > __noreturn void rust_helper_BUG(void)
> > {
> > @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> > }
> > EXPORT_SYMBOL_GPL(rust_helper_BUG);
> >
> > +refcount_t rust_helper_REFCOUNT_INIT(int n)
> > +{
> > + return (refcount_t)REFCOUNT_INIT(n);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> > +
> > +void rust_helper_refcount_inc(refcount_t *r)
> > +{
> > + refcount_inc(r);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> > +
> > +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> > +{
> > + return refcount_dec_and_test(r);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> > +
> > /*
> > * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> > * as the Rust `usize` type, so we can use it in contexts where Rust
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index 53040fa9e897..ace064a3702a 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -31,6 +31,7 @@ mod static_assert;
> > #[doc(hidden)]
> > pub mod std_vendor;
> > pub mod str;
> > +pub mod sync;
> > pub mod types;
> >
> > #[doc(hidden)]
> > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > new file mode 100644
> > index 000000000000..39b379dd548f
> > --- /dev/null
> > +++ b/rust/kernel/sync.rs
> > @@ -0,0 +1,10 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Synchronisation primitives.
> > +//!
> > +//! This module contains the kernel APIs related to synchronisation that have been ported or
> > +//! wrapped for usage by Rust code in the kernel.
> > +
> > +mod arc;
> > +
> > +pub use arc::Arc;
> > diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> > new file mode 100644
> > index 000000000000..22290eb5ab9b
> > --- /dev/null
> > +++ b/rust/kernel/sync/arc.rs
> > @@ -0,0 +1,157 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! A reference-counted pointer.
> > +//!
> > +//! This module implements a way for users to create reference-counted objects and pointers to
> > +//! them. Such a pointer automatically increments and decrements the count, and drops the
> > +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> > +//! threads.
> > +//!
> > +//! It is different from the standard library's [`Arc`] in a few ways:
> > +//! 1. It is backed by the kernel's `refcount_t` type.
> > +//! 2. It does not support weak references, which allows it to be half the size.
> > +//! 3. It saturates the reference count instead of aborting when it goes over a threshold.
>
> This makes me worry, and the rest of the code confirms it. This is not a safe abstraction: what happens if the count saturates and then everything is dropped again? The count “goes negative” (which is to say, use-after-free).
Are you familiar with how refcount_t is implemented? Once the counter
saturates, it stays stuck in this saturated state. There is no
user-after-free.
> > +//! 4. It does not provide a `get_mut` method, so the ref counted object is pinned.
> > +//!
> > +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> > +
> > +use crate::{bindings, error::Result, types::Opaque};
> > +use alloc::boxed::Box;
> > +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> > +
> > +/// A reference-counted pointer to an instance of `T`.
> > +///
> > +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> > +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> > +///
> > +/// # Invariants
> > +///
> > +/// The reference count on an instance of [`Arc`] is always non-zero.
> > +/// The object pointed to by [`Arc`] is always pinned.
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::sync::Arc;
> > +///
> > +/// struct Example {
> > +/// a: u32,
> > +/// b: u32,
> > +/// }
> > +///
> > +/// // Create a ref-counted instance of `Example`.
> > +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> > +///
> > +/// // Get a new pointer to `obj` and increment the refcount.
> > +/// let cloned = obj.clone();
> > +///
> > +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> > +/// assert!(core::ptr::eq(&*obj, &*cloned));
> > +///
> > +/// // Destroy `obj` and decrement its refcount.
> > +/// drop(obj);
> > +///
> > +/// // Check that the values are still accessible through `cloned`.
> > +/// assert_eq!(cloned.a, 10);
> > +/// assert_eq!(cloned.b, 20);
> > +///
> > +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> > +/// ```
> > +pub struct Arc<T: ?Sized> {
> > + ptr: NonNull<ArcInner<T>>,
> > + _p: PhantomData<ArcInner<T>>,
> > +}
> > +
> > +#[repr(C)]
> > +struct ArcInner<T: ?Sized> {
> > + refcount: Opaque<bindings::refcount_t>,
> > + data: T,
> > +}
> > +
> > +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> > +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> > +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> > +// example, when the reference count reaches zero and `T` is dropped.
> > +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> > +
> > +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> > +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> > +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> > +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> > +
> > +impl<T> Arc<T> {
> > + /// Constructs a new reference counted instance of `T`.
> > + pub fn try_new(contents: T) -> Result<Self> {
> > + // INVARIANT: The refcount is initialised to a non-zero value.
> > + let value = ArcInner {
> > + // SAFETY: There are no safety requirements for this FFI call.
> > + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> > + data: contents,
> > + };
> > +
> > + let inner = Box::try_new(value)?;
> > +
> > + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> > + // `Arc` object.
> > + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Arc<T> {
> > + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> > + /// count, one of which will be owned by the new [`Arc`] instance.
> > + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> > + // INVARIANT: By the safety requirements, the invariants hold.
> > + Arc {
> > + ptr: inner,
> > + _p: PhantomData,
> > + }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Deref for Arc<T> {
> > + type Target = T;
> > +
> > + fn deref(&self) -> &Self::Target {
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> > + // safe to dereference it.
> > + unsafe { &self.ptr.as_ref().data }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Clone for Arc<T> {
> > + fn clone(&self) -> Self {
> > + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> > + // safe to increment the refcount.
> > + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
>
> This needs to be a fallible process; maybe provide a try_clone inherent method instead of the trait impl. It’s not worth the “convenience” to have something that can break safety (see above). There is a reason for the original one panicking here!
Thanks for your input, but I'm afraid your lack of familiarity with
refcount_t is clouding your judgement. May I suggest that you read the
comments at the top of refcount.h?
>
> > +
> > + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> > + unsafe { Self::from_inner(self.ptr) }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Drop for Arc<T> {
> > + fn drop(&mut self) {
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> > + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> > + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> > + // freed/invalid memory as long as it is never dereferenced.
> > + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> > +
> > + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> > + // this instance is being dropped, so the broken invariant is not observable.
> > + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> > + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> > + if is_zero {
> > + // The count reached zero, we must free the memory.
> > + //
> > + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> > + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> > + }
> > + }
> > +}
> > --
> > 2.34.1
> >
> >
>
^ permalink raw reply
* Re: [PATCH 7/7] rust: sync: add support for dispatching on Arc and ArcBorrow.
From: Laine Taffin Altman @ 2022-12-28 7:24 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel
In-Reply-To: <20221228060346.352362-7-wedsonaf@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3166 bytes --]
On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> Trait objects (`dyn T`) require trait `T` to be "object safe". One of
> the requirements for "object safety" is that the receiver have one of
> the allowed types. This commit adds `Arc<T>` and `ArcBorrow<'_, T>` to
> the list of allowed types.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync/arc.rs | 20 ++++++++++++++++++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 4bde65e7b06b..e0b0e953907d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -15,6 +15,7 @@
> #![feature(allocator_api)]
> #![feature(coerce_unsized)]
> #![feature(core_ffi_c)]
> +#![feature(dispatch_from_dyn)]
> #![feature(receiver_trait)]
> #![feature(unsize)]
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 832bafc74a90..ff73f9240ca1 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -92,9 +92,15 @@ use core::{
> /// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
> ///
> /// ```
> -/// use kernel::sync::Arc;
> +/// use kernel::sync::{Arc, ArcBorrow};
> +///
> +/// trait MyTrait {
> +/// // Trait has a function whose `self` type is `Arc<Self>`.
> +/// fn example1(self: Arc<Self>) {}
> ///
> -/// trait MyTrait {}
> +/// // Trait has a function whose `self` type is `ArcBorrow<'_, Self>`.
> +/// fn example2(self: ArcBorrow<'_, Self>) {}
> +/// }
> ///
> /// struct Example;
> /// impl MyTrait for Example {}
> @@ -123,6 +129,9 @@ impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
> // dynamically-sized type (DST) `U`.
> impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
>
> +// This is to allow `Arc<U>` to be dispatched on when `Arc<T>` can be coerced into `Arc<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> @@ -297,6 +306,13 @@ pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> // This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}
>
> +// This is to allow `ArcBorrow<U>` to be dispatched on when `ArcBorrow<T>` can be coerced into
> +// `ArcBorrow<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'_, U>>
> + for ArcBorrow<'_, T>
These lifetimes need to be tied together, I think.
`impl<'a, T: ?Sized + Unsize<U> + 'a, U: ?Sized + 'a> core::ops::DispatchFromDyn<ArcBorrow<'a, U>> for ArcBorrow<'a, T>`
> +{
> +}
> +
> impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> fn clone(&self) -> Self {
> *self
> --
> 2.34.1
>
>
— Laine Taffin Altman
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 6/7] rust: sync: introduce `UniqueArc`
From: Laine Taffin Altman @ 2022-12-28 7:19 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel
In-Reply-To: <20221228060346.352362-6-wedsonaf@gmail.com>
> On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> Since `Arc<T>` does not allow mutating `T` directly (i.e., without inner
> mutability), it is currently not possible to do some initialisation of
> `T` post construction but before being shared.
>
> `UniqueArc<T>` addresses this problem essentially being an `Arc<T>` that
> has a refcount of 1 and is therefore writable. Once initialisation is
> completed, it can be transitioned (without failure paths) into an
> `Arc<T>`.
>
> Suggested-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/sync.rs | 2 +-
> rust/kernel/sync/arc.rs | 152 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 151 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 5de03ea83ea1..33da23e3076d 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>
> mod arc;
>
> -pub use arc::{Arc, ArcBorrow};
> +pub use arc::{Arc, ArcBorrow, UniqueArc};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 84f31c85a513..832bafc74a90 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,8 +19,9 @@ use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> use core::{
> marker::{PhantomData, Unsize},
> - mem::ManuallyDrop,
> - ops::Deref,
> + mem::{ManuallyDrop, MaybeUninit},
> + ops::{Deref, DerefMut},
> + pin::Pin,
> ptr::NonNull,
> };
>
> @@ -222,6 +223,19 @@ impl<T: ?Sized> Drop for Arc<T> {
> }
> }
>
> +impl<T: ?Sized> From<UniqueArc<T>> for Arc<T> {
> + fn from(item: UniqueArc<T>) -> Self {
> + item.inner
> + }
> +}
> +
> +impl<T: ?Sized> From<Pin<UniqueArc<T>>> for Arc<T> {
> + fn from(item: Pin<UniqueArc<T>>) -> Self {
> + // SAFETY: The type invariants of `Arc` guarantee that the data is pinned.
> + unsafe { Pin::into_inner_unchecked(item).inner }
> + }
> +}
> +
> /// A borrowed reference to an [`Arc`] instance.
> ///
> /// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler
> @@ -328,3 +342,137 @@ impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> unsafe { &self.inner.as_ref().data }
> }
> }
> +
> +/// A refcounted object that is known to have a refcount of 1.
> +///
> +/// It is mutable and can be converted to an [`Arc`] so that it can be shared.
> +///
> +/// # Invariants
> +///
> +/// `inner` always has a reference count of 1.
> +///
> +/// # Examples
> +///
> +/// In the following example, we make changes to the inner object before turning it into an
> +/// `Arc<Test>` object (after which point, it cannot be mutated directly). Note that `x.into()`
> +/// cannot fail.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let mut x = UniqueArc::try_new(Example { a: 10, b: 20 })?;
> +/// x.a += 1;
> +/// x.b += 1;
> +/// Ok(x.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the following example we first allocate memory for a ref-counted `Example` but we don't
> +/// initialise it on allocation. We do initialise it later with a call to [`UniqueArc::write`],
> +/// followed by a conversion to `Arc<Example>`. This is particularly useful when allocation happens
> +/// in one context (e.g., sleepable) and initialisation in another (e.g., atomic):
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let x = UniqueArc::try_new_uninit()?;
> +/// Ok(x.write(Example { a: 10, b: 20 }).into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the last example below, the caller gets a pinned instance of `Example` while converting to
> +/// `Arc<Example>`; this is useful in scenarios where one needs a pinned reference during
> +/// initialisation, for example, when initialising fields that are wrapped in locks.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let mut pinned = Pin::from(UniqueArc::try_new(Example { a: 10, b: 20 })?);
> +/// // We can modify `pinned` because it is `Unpin`.
> +/// pinned.as_mut().a += 1;
> +/// Ok(pinned.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +pub struct UniqueArc<T: ?Sized> {
> + inner: Arc<T>,
> +}
> +
> +impl<T> UniqueArc<T> {
> + /// Tries to allocate a new [`UniqueArc`] instance.
> + pub fn try_new(value: T) -> Result<Self> {
> + Ok(Self {
> + // INVARIANT: The newly-created object has a ref-count of 1.
> + inner: Arc::try_new(value)?,
> + })
> + }
> +
> + /// Tries to allocate a new [`UniqueArc`] instance whose contents are not initialised yet.
> + pub fn try_new_uninit() -> Result<UniqueArc<MaybeUninit<T>>> {
> + Ok(UniqueArc::<MaybeUninit<T>> {
> + // INVARIANT: The newly-created object has a ref-count of 1.
> + inner: Arc::try_new(MaybeUninit::uninit())?,
> + })
> + }
> +}
> +
> +impl<T> UniqueArc<MaybeUninit<T>> {
> + /// Converts a `UniqueArc<MaybeUninit<T>>` into a `UniqueArc<T>` by writing a value into it.
> + pub fn write(mut self, value: T) -> UniqueArc<T> {
> + self.deref_mut().write(value);
> + let inner = ManuallyDrop::new(self).inner.ptr;
> + UniqueArc {
> + // SAFETY: The new `Arc` is taking over `ptr` from `self.inner` (which won't be
> + // dropped). The types are compatible because `MaybeUninit<T>` is compatible with `T`.
> + inner: unsafe { Arc::from_inner(inner.cast()) },
> + }
> + }
> +}
> +
> +impl<T: ?Sized> From<UniqueArc<T>> for Pin<UniqueArc<T>> {
> + fn from(obj: UniqueArc<T>) -> Self {
> + // SAFETY: It is not possible to move/replace `T` inside a `Pin<UniqueArc<T>>` (unless `T`
Minor nit: `Pin<UniqueArc<T>>` in this comment should just be `UniqueArc<T>`.
> + // is `Unpin`), so it is ok to convert it to `Pin<UniqueArc<T>>`.
> + unsafe { Pin::new_unchecked(obj) }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for UniqueArc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + self.inner.deref()
> + }
> +}
> +
> +impl<T: ?Sized> DerefMut for UniqueArc<T> {
> + fn deref_mut(&mut self) -> &mut Self::Target {
> + // SAFETY: By the `Arc` type invariant, there is necessarily a reference to the object, so
> + // it is safe to dereference it. Additionally, we know there is only one reference when
> + // it's inside a `UniqueArc`, so it is safe to get a mutable reference.
> + unsafe { &mut self.inner.ptr.as_mut().data }
> + }
> +}
> --
> 2.34.1
>
>
— Laine Taffin Altman
^ permalink raw reply
* Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`
From: Laine Taffin Altman @ 2022-12-28 7:15 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel
In-Reply-To: <20221228060346.352362-4-wedsonaf@gmail.com>
On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
> This allows us to create references to a ref-counted allocation without
> double-indirection and that still allow us to increment the refcount to
> a new `Arc<T>`.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/sync.rs | 2 +-
> rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 39b379dd548f..5de03ea83ea1 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>
> mod arc;
>
> -pub use arc::Arc;
> +pub use arc::{Arc, ArcBorrow};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index dbc7596cc3ce..f68bfc02c81a 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> use core::{
> marker::{PhantomData, Unsize},
> + mem::ManuallyDrop,
> ops::Deref,
> ptr::NonNull,
> };
> @@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
> _p: PhantomData,
> }
> }
> +
> + /// Returns an [`ArcBorrow`] from the given [`Arc`].
> + ///
> + /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
> + /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
> + #[inline]
> + pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
> + // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
> + // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
> + // reference can be created.
> + unsafe { ArcBorrow::new(self.ptr) }
> + }
> }
>
> impl<T: ?Sized> Deref for Arc<T> {
> @@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
> }
> }
> }
> +
> +/// A borrowed reference to an [`Arc`] instance.
> +///
> +/// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler
> +/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
> +///
> +/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
> +/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
> +/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
> +/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
> +/// needed.
> +///
> +/// # Invariants
> +///
> +/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
> +/// lifetime of the [`ArcBorrow`] instance.
> +///
> +/// # Example
> +///
> +/// ```
> +/// use crate::sync::{Arc, ArcBorrow};
> +///
> +/// struct Example;
> +///
> +/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
> +/// e.into()
> +/// }
> +///
> +/// let obj = Arc::try_new(Example)?;
> +/// let cloned = do_something(obj.as_arc_borrow());
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +/// ```
> +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> + inner: NonNull<ArcInner<T>>,
> + _p: PhantomData<&'a ()>,
> +}
> +
> +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> + fn clone(&self) -> Self {
> + *self
> + }
> +}
> +
> +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
> +
> +impl<T: ?Sized> ArcBorrow<'_, T> {
> + /// Creates a new [`ArcBorrow`] instance.
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance:
> + /// 1. That `inner` remains valid;
> + /// 2. That no mutable references to `inner` are created.
> + unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
> + // INVARIANT: The safety requirements guarantee the invariants.
> + Self {
> + inner,
> + _p: PhantomData,
> + }
> + }
> +}
> +
> +impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
> + fn from(b: ArcBorrow<'_, T>) -> Self {
> + // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
> + // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
> + // increment.
> + ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) })
> + .deref()
> + .clone()
The same worries about safety apply here too. You need to make this fallible—try_from is nice enough for that.
> + }
> +}
> +
> +impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + // SAFETY: By the type invariant, the underlying object is still alive with no mutable
> + // references to it, so it is safe to create a shared reference.
> + unsafe { &self.inner.as_ref().data }
> + }
> +}
> --
> 2.34.1
>
>
— Laine Taffin Altman
^ permalink raw reply
* Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations
From: Laine Taffin Altman @ 2022-12-28 7:09 UTC (permalink / raw)
To: Wedson Almeida Filho
Cc: rust-for-linux, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Will Deacon, Peter Zijlstra,
Mark Rutland
In-Reply-To: <20221228060346.352362-1-wedsonaf@gmail.com>
On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> allows Rust code to idiomatically allocate memory that is ref-counted.
>
> Cc: Will Deacon <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/bindings/lib.rs | 1 +
> rust/helpers.c | 19 ++++
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync.rs | 10 ++
> rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> 6 files changed, 189 insertions(+)
> create mode 100644 rust/kernel/sync.rs
> create mode 100644 rust/kernel/sync/arc.rs
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index c48bc284214a..75d85bd6c592 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/slab.h>
> +#include <linux/refcount.h>
>
> /* `bindgen` gets confused at certain things. */
> const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> index 6c50ee62c56b..7b246454e009 100644
> --- a/rust/bindings/lib.rs
> +++ b/rust/bindings/lib.rs
> @@ -41,6 +41,7 @@ mod bindings_raw {
> #[allow(dead_code)]
> mod bindings_helper {
> // Import the generated bindings for types.
> + use super::bindings_raw::*;
> include!(concat!(
> env!("OBJTREE"),
> "/rust/bindings/bindings_helpers_generated.rs"
> diff --git a/rust/helpers.c b/rust/helpers.c
> index b4f15eee2ffd..09a4d93f9d62 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -20,6 +20,7 @@
>
> #include <linux/bug.h>
> #include <linux/build_bug.h>
> +#include <linux/refcount.h>
>
> __noreturn void rust_helper_BUG(void)
> {
> @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> }
> EXPORT_SYMBOL_GPL(rust_helper_BUG);
>
> +refcount_t rust_helper_REFCOUNT_INIT(int n)
> +{
> + return (refcount_t)REFCOUNT_INIT(n);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> +
> +void rust_helper_refcount_inc(refcount_t *r)
> +{
> + refcount_inc(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> +
> +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> +{
> + return refcount_dec_and_test(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> +
> /*
> * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 53040fa9e897..ace064a3702a 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -31,6 +31,7 @@ mod static_assert;
> #[doc(hidden)]
> pub mod std_vendor;
> pub mod str;
> +pub mod sync;
> pub mod types;
>
> #[doc(hidden)]
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> new file mode 100644
> index 000000000000..39b379dd548f
> --- /dev/null
> +++ b/rust/kernel/sync.rs
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Synchronisation primitives.
> +//!
> +//! This module contains the kernel APIs related to synchronisation that have been ported or
> +//! wrapped for usage by Rust code in the kernel.
> +
> +mod arc;
> +
> +pub use arc::Arc;
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> new file mode 100644
> index 000000000000..22290eb5ab9b
> --- /dev/null
> +++ b/rust/kernel/sync/arc.rs
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A reference-counted pointer.
> +//!
> +//! This module implements a way for users to create reference-counted objects and pointers to
> +//! them. Such a pointer automatically increments and decrements the count, and drops the
> +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> +//! threads.
> +//!
> +//! It is different from the standard library's [`Arc`] in a few ways:
> +//! 1. It is backed by the kernel's `refcount_t` type.
> +//! 2. It does not support weak references, which allows it to be half the size.
> +//! 3. It saturates the reference count instead of aborting when it goes over a threshold.
This makes me worry, and the rest of the code confirms it. This is not a safe abstraction: what happens if the count saturates and then everything is dropped again? The count “goes negative” (which is to say, use-after-free).
> +//! 4. It does not provide a `get_mut` method, so the ref counted object is pinned.
> +//!
> +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> +
> +use crate::{bindings, error::Result, types::Opaque};
> +use alloc::boxed::Box;
> +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> +
> +/// A reference-counted pointer to an instance of `T`.
> +///
> +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> +///
> +/// # Invariants
> +///
> +/// The reference count on an instance of [`Arc`] is always non-zero.
> +/// The object pointed to by [`Arc`] is always pinned.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// // Create a ref-counted instance of `Example`.
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +///
> +/// // Get a new pointer to `obj` and increment the refcount.
> +/// let cloned = obj.clone();
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +///
> +/// // Destroy `obj` and decrement its refcount.
> +/// drop(obj);
> +///
> +/// // Check that the values are still accessible through `cloned`.
> +/// assert_eq!(cloned.a, 10);
> +/// assert_eq!(cloned.b, 20);
> +///
> +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> +/// ```
> +pub struct Arc<T: ?Sized> {
> + ptr: NonNull<ArcInner<T>>,
> + _p: PhantomData<ArcInner<T>>,
> +}
> +
> +#[repr(C)]
> +struct ArcInner<T: ?Sized> {
> + refcount: Opaque<bindings::refcount_t>,
> + data: T,
> +}
> +
> +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> +// example, when the reference count reaches zero and `T` is dropped.
> +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> +
> +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> +
> +impl<T> Arc<T> {
> + /// Constructs a new reference counted instance of `T`.
> + pub fn try_new(contents: T) -> Result<Self> {
> + // INVARIANT: The refcount is initialised to a non-zero value.
> + let value = ArcInner {
> + // SAFETY: There are no safety requirements for this FFI call.
> + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> + data: contents,
> + };
> +
> + let inner = Box::try_new(value)?;
> +
> + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> + // `Arc` object.
> + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> + }
> +}
> +
> +impl<T: ?Sized> Arc<T> {
> + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> + /// count, one of which will be owned by the new [`Arc`] instance.
> + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> + // INVARIANT: By the safety requirements, the invariants hold.
> + Arc {
> + ptr: inner,
> + _p: PhantomData,
> + }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for Arc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to dereference it.
> + unsafe { &self.ptr.as_ref().data }
> + }
> +}
> +
> +impl<T: ?Sized> Clone for Arc<T> {
> + fn clone(&self) -> Self {
> + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to increment the refcount.
> + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
This needs to be a fallible process; maybe provide a try_clone inherent method instead of the trait impl. It’s not worth the “convenience” to have something that can break safety (see above). There is a reason for the original one panicking here!
> +
> + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> + unsafe { Self::from_inner(self.ptr) }
> + }
> +}
> +
> +impl<T: ?Sized> Drop for Arc<T> {
> + fn drop(&mut self) {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> + // freed/invalid memory as long as it is never dereferenced.
> + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> +
> + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> + // this instance is being dropped, so the broken invariant is not observable.
> + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> + if is_zero {
> + // The count reached zero, we must free the memory.
> + //
> + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> + }
> + }
> +}
> --
> 2.34.1
>
>
^ permalink raw reply
* [PATCH 6/7] rust: sync: introduce `UniqueArc`
From: Wedson Almeida Filho @ 2022-12-28 6:03 UTC (permalink / raw)
To: rust-for-linux
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Wedson Almeida Filho
In-Reply-To: <20221228060346.352362-1-wedsonaf@gmail.com>
Since `Arc<T>` does not allow mutating `T` directly (i.e., without inner
mutability), it is currently not possible to do some initialisation of
`T` post construction but before being shared.
`UniqueArc<T>` addresses this problem essentially being an `Arc<T>` that
has a refcount of 1 and is therefore writable. Once initialisation is
completed, it can be transitioned (without failure paths) into an
`Arc<T>`.
Suggested-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
---
rust/kernel/sync.rs | 2 +-
rust/kernel/sync/arc.rs | 152 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 151 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 5de03ea83ea1..33da23e3076d 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -7,4 +7,4 @@
mod arc;
-pub use arc::{Arc, ArcBorrow};
+pub use arc::{Arc, ArcBorrow, UniqueArc};
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 84f31c85a513..832bafc74a90 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -19,8 +19,9 @@ use crate::{bindings, error::Result, types::Opaque};
use alloc::boxed::Box;
use core::{
marker::{PhantomData, Unsize},
- mem::ManuallyDrop,
- ops::Deref,
+ mem::{ManuallyDrop, MaybeUninit},
+ ops::{Deref, DerefMut},
+ pin::Pin,
ptr::NonNull,
};
@@ -222,6 +223,19 @@ impl<T: ?Sized> Drop for Arc<T> {
}
}
+impl<T: ?Sized> From<UniqueArc<T>> for Arc<T> {
+ fn from(item: UniqueArc<T>) -> Self {
+ item.inner
+ }
+}
+
+impl<T: ?Sized> From<Pin<UniqueArc<T>>> for Arc<T> {
+ fn from(item: Pin<UniqueArc<T>>) -> Self {
+ // SAFETY: The type invariants of `Arc` guarantee that the data is pinned.
+ unsafe { Pin::into_inner_unchecked(item).inner }
+ }
+}
+
/// A borrowed reference to an [`Arc`] instance.
///
/// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler
@@ -328,3 +342,137 @@ impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
unsafe { &self.inner.as_ref().data }
}
}
+
+/// A refcounted object that is known to have a refcount of 1.
+///
+/// It is mutable and can be converted to an [`Arc`] so that it can be shared.
+///
+/// # Invariants
+///
+/// `inner` always has a reference count of 1.
+///
+/// # Examples
+///
+/// In the following example, we make changes to the inner object before turning it into an
+/// `Arc<Test>` object (after which point, it cannot be mutated directly). Note that `x.into()`
+/// cannot fail.
+///
+/// ```
+/// use kernel::sync::{Arc, UniqueArc};
+///
+/// struct Example {
+/// a: u32,
+/// b: u32,
+/// }
+///
+/// fn test() -> Result<Arc<Example>> {
+/// let mut x = UniqueArc::try_new(Example { a: 10, b: 20 })?;
+/// x.a += 1;
+/// x.b += 1;
+/// Ok(x.into())
+/// }
+///
+/// # test().unwrap();
+/// ```
+///
+/// In the following example we first allocate memory for a ref-counted `Example` but we don't
+/// initialise it on allocation. We do initialise it later with a call to [`UniqueArc::write`],
+/// followed by a conversion to `Arc<Example>`. This is particularly useful when allocation happens
+/// in one context (e.g., sleepable) and initialisation in another (e.g., atomic):
+///
+/// ```
+/// use kernel::sync::{Arc, UniqueArc};
+///
+/// struct Example {
+/// a: u32,
+/// b: u32,
+/// }
+///
+/// fn test() -> Result<Arc<Example>> {
+/// let x = UniqueArc::try_new_uninit()?;
+/// Ok(x.write(Example { a: 10, b: 20 }).into())
+/// }
+///
+/// # test().unwrap();
+/// ```
+///
+/// In the last example below, the caller gets a pinned instance of `Example` while converting to
+/// `Arc<Example>`; this is useful in scenarios where one needs a pinned reference during
+/// initialisation, for example, when initialising fields that are wrapped in locks.
+///
+/// ```
+/// use kernel::sync::{Arc, UniqueArc};
+///
+/// struct Example {
+/// a: u32,
+/// b: u32,
+/// }
+///
+/// fn test() -> Result<Arc<Example>> {
+/// let mut pinned = Pin::from(UniqueArc::try_new(Example { a: 10, b: 20 })?);
+/// // We can modify `pinned` because it is `Unpin`.
+/// pinned.as_mut().a += 1;
+/// Ok(pinned.into())
+/// }
+///
+/// # test().unwrap();
+/// ```
+pub struct UniqueArc<T: ?Sized> {
+ inner: Arc<T>,
+}
+
+impl<T> UniqueArc<T> {
+ /// Tries to allocate a new [`UniqueArc`] instance.
+ pub fn try_new(value: T) -> Result<Self> {
+ Ok(Self {
+ // INVARIANT: The newly-created object has a ref-count of 1.
+ inner: Arc::try_new(value)?,
+ })
+ }
+
+ /// Tries to allocate a new [`UniqueArc`] instance whose contents are not initialised yet.
+ pub fn try_new_uninit() -> Result<UniqueArc<MaybeUninit<T>>> {
+ Ok(UniqueArc::<MaybeUninit<T>> {
+ // INVARIANT: The newly-created object has a ref-count of 1.
+ inner: Arc::try_new(MaybeUninit::uninit())?,
+ })
+ }
+}
+
+impl<T> UniqueArc<MaybeUninit<T>> {
+ /// Converts a `UniqueArc<MaybeUninit<T>>` into a `UniqueArc<T>` by writing a value into it.
+ pub fn write(mut self, value: T) -> UniqueArc<T> {
+ self.deref_mut().write(value);
+ let inner = ManuallyDrop::new(self).inner.ptr;
+ UniqueArc {
+ // SAFETY: The new `Arc` is taking over `ptr` from `self.inner` (which won't be
+ // dropped). The types are compatible because `MaybeUninit<T>` is compatible with `T`.
+ inner: unsafe { Arc::from_inner(inner.cast()) },
+ }
+ }
+}
+
+impl<T: ?Sized> From<UniqueArc<T>> for Pin<UniqueArc<T>> {
+ fn from(obj: UniqueArc<T>) -> Self {
+ // SAFETY: It is not possible to move/replace `T` inside a `Pin<UniqueArc<T>>` (unless `T`
+ // is `Unpin`), so it is ok to convert it to `Pin<UniqueArc<T>>`.
+ unsafe { Pin::new_unchecked(obj) }
+ }
+}
+
+impl<T: ?Sized> Deref for UniqueArc<T> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ self.inner.deref()
+ }
+}
+
+impl<T: ?Sized> DerefMut for UniqueArc<T> {
+ fn deref_mut(&mut self) -> &mut Self::Target {
+ // SAFETY: By the `Arc` type invariant, there is necessarily a reference to the object, so
+ // it is safe to dereference it. Additionally, we know there is only one reference when
+ // it's inside a `UniqueArc`, so it is safe to get a mutable reference.
+ unsafe { &mut self.inner.ptr.as_mut().data }
+ }
+}
--
2.34.1
^ permalink raw reply related
* [PATCH 7/7] rust: sync: add support for dispatching on Arc and ArcBorrow.
From: Wedson Almeida Filho @ 2022-12-28 6:03 UTC (permalink / raw)
To: rust-for-linux
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Wedson Almeida Filho
In-Reply-To: <20221228060346.352362-1-wedsonaf@gmail.com>
Trait objects (`dyn T`) require trait `T` to be "object safe". One of
the requirements for "object safety" is that the receiver have one of
the allowed types. This commit adds `Arc<T>` and `ArcBorrow<'_, T>` to
the list of allowed types.
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
---
rust/kernel/lib.rs | 1 +
rust/kernel/sync/arc.rs | 20 ++++++++++++++++++--
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 4bde65e7b06b..e0b0e953907d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -15,6 +15,7 @@
#![feature(allocator_api)]
#![feature(coerce_unsized)]
#![feature(core_ffi_c)]
+#![feature(dispatch_from_dyn)]
#![feature(receiver_trait)]
#![feature(unsize)]
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 832bafc74a90..ff73f9240ca1 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -92,9 +92,15 @@ use core::{
/// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
///
/// ```
-/// use kernel::sync::Arc;
+/// use kernel::sync::{Arc, ArcBorrow};
+///
+/// trait MyTrait {
+/// // Trait has a function whose `self` type is `Arc<Self>`.
+/// fn example1(self: Arc<Self>) {}
///
-/// trait MyTrait {}
+/// // Trait has a function whose `self` type is `ArcBorrow<'_, Self>`.
+/// fn example2(self: ArcBorrow<'_, Self>) {}
+/// }
///
/// struct Example;
/// impl MyTrait for Example {}
@@ -123,6 +129,9 @@ impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
// dynamically-sized type (DST) `U`.
impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
+// This is to allow `Arc<U>` to be dispatched on when `Arc<T>` can be coerced into `Arc<U>`.
+impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Arc<T> {}
+
// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
@@ -297,6 +306,13 @@ pub struct ArcBorrow<'a, T: ?Sized + 'a> {
// This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}
+// This is to allow `ArcBorrow<U>` to be dispatched on when `ArcBorrow<T>` can be coerced into
+// `ArcBorrow<U>`.
+impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'_, U>>
+ for ArcBorrow<'_, T>
+{
+}
+
impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
fn clone(&self) -> Self {
*self
--
2.34.1
^ permalink raw reply related
* [PATCH 5/7] rust: sync: allow type of `self` to be `ArcBorrow<T>`
From: Wedson Almeida Filho @ 2022-12-28 6:03 UTC (permalink / raw)
To: rust-for-linux
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Wedson Almeida Filho
In-Reply-To: <20221228060346.352362-1-wedsonaf@gmail.com>
This allows associated functions whose `self` argument has
`ArcBorrow<T>` as their type. This, in turn, allows callers to use the
dot syntax to make calls.
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
---
rust/kernel/sync/arc.rs | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index f68bfc02c81a..84f31c85a513 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -255,11 +255,34 @@ impl<T: ?Sized> Drop for Arc<T> {
/// // Assert that both `obj` and `cloned` point to the same underlying object.
/// assert!(core::ptr::eq(&*obj, &*cloned));
/// ```
+///
+/// Using `ArcBorrow<T>` as the type of `self`:
+///
+/// ```
+/// use crate::sync::{Arc, ArcBorrow};
+///
+/// struct Example {
+/// a: u32,
+/// b: u32,
+/// }
+///
+/// impl Example {
+/// fn use_reference(self: ArcBorrow<'_, Self>) {
+/// // ...
+/// }
+/// }
+///
+/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
+/// obj.as_arc_borrow().use_reference();
+/// ```
pub struct ArcBorrow<'a, T: ?Sized + 'a> {
inner: NonNull<ArcInner<T>>,
_p: PhantomData<&'a ()>,
}
+// This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
+impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}
+
impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
fn clone(&self) -> Self {
*self
--
2.34.1
^ permalink raw reply related
* [PATCH 4/7] rust: sync: introduce `ArcBorrow`
From: Wedson Almeida Filho @ 2022-12-28 6:03 UTC (permalink / raw)
To: rust-for-linux
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Wedson Almeida Filho
In-Reply-To: <20221228060346.352362-1-wedsonaf@gmail.com>
This allows us to create references to a ref-counted allocation without
double-indirection and that still allow us to increment the refcount to
a new `Arc<T>`.
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
---
rust/kernel/sync.rs | 2 +-
rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 39b379dd548f..5de03ea83ea1 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -7,4 +7,4 @@
mod arc;
-pub use arc::Arc;
+pub use arc::{Arc, ArcBorrow};
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index dbc7596cc3ce..f68bfc02c81a 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
use alloc::boxed::Box;
use core::{
marker::{PhantomData, Unsize},
+ mem::ManuallyDrop,
ops::Deref,
ptr::NonNull,
};
@@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
_p: PhantomData,
}
}
+
+ /// Returns an [`ArcBorrow`] from the given [`Arc`].
+ ///
+ /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
+ /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
+ #[inline]
+ pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
+ // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
+ // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
+ // reference can be created.
+ unsafe { ArcBorrow::new(self.ptr) }
+ }
}
impl<T: ?Sized> Deref for Arc<T> {
@@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
}
}
}
+
+/// A borrowed reference to an [`Arc`] instance.
+///
+/// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler
+/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
+///
+/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
+/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
+/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
+/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
+/// needed.
+///
+/// # Invariants
+///
+/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
+/// lifetime of the [`ArcBorrow`] instance.
+///
+/// # Example
+///
+/// ```
+/// use crate::sync::{Arc, ArcBorrow};
+///
+/// struct Example;
+///
+/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
+/// e.into()
+/// }
+///
+/// let obj = Arc::try_new(Example)?;
+/// let cloned = do_something(obj.as_arc_borrow());
+///
+/// // Assert that both `obj` and `cloned` point to the same underlying object.
+/// assert!(core::ptr::eq(&*obj, &*cloned));
+/// ```
+pub struct ArcBorrow<'a, T: ?Sized + 'a> {
+ inner: NonNull<ArcInner<T>>,
+ _p: PhantomData<&'a ()>,
+}
+
+impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
+ fn clone(&self) -> Self {
+ *self
+ }
+}
+
+impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
+
+impl<T: ?Sized> ArcBorrow<'_, T> {
+ /// Creates a new [`ArcBorrow`] instance.
+ ///
+ /// # Safety
+ ///
+ /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance:
+ /// 1. That `inner` remains valid;
+ /// 2. That no mutable references to `inner` are created.
+ unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
+ // INVARIANT: The safety requirements guarantee the invariants.
+ Self {
+ inner,
+ _p: PhantomData,
+ }
+ }
+}
+
+impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
+ fn from(b: ArcBorrow<'_, T>) -> Self {
+ // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
+ // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
+ // increment.
+ ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) })
+ .deref()
+ .clone()
+ }
+}
+
+impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: By the type invariant, the underlying object is still alive with no mutable
+ // references to it, so it is safe to create a shared reference.
+ unsafe { &self.inner.as_ref().data }
+ }
+}
--
2.34.1
^ permalink raw reply related
* [PATCH 3/7] rust: sync: allow coercion from `Arc<T>` to `Arc<U>`
From: Wedson Almeida Filho @ 2022-12-28 6:03 UTC (permalink / raw)
To: rust-for-linux
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, linux-kernel, Wedson Almeida Filho
In-Reply-To: <20221228060346.352362-1-wedsonaf@gmail.com>
The coercion is only allowed if `U` is a compatible dynamically-sized
type (DST). For example, if we have some type `X` that implements trait
`Y`, then this allows `Arc<X>` to be coerced into `Arc<dyn Y>`.
Suggested-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
---
rust/kernel/lib.rs | 2 ++
rust/kernel/sync/arc.rs | 27 ++++++++++++++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 1a10f7c0ddd9..4bde65e7b06b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -13,8 +13,10 @@
#![no_std]
#![feature(allocator_api)]
+#![feature(coerce_unsized)]
#![feature(core_ffi_c)]
#![feature(receiver_trait)]
+#![feature(unsize)]
// Ensure conditional compilation based on the kernel configuration works;
// otherwise we may silently break things like initcall handling.
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index e2eb0e67d483..dbc7596cc3ce 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -17,7 +17,11 @@
use crate::{bindings, error::Result, types::Opaque};
use alloc::boxed::Box;
-use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
+use core::{
+ marker::{PhantomData, Unsize},
+ ops::Deref,
+ ptr::NonNull,
+};
/// A reference-counted pointer to an instance of `T`.
///
@@ -82,6 +86,23 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
/// obj.use_reference();
/// obj.take_over();
/// ```
+///
+/// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
+///
+/// ```
+/// use kernel::sync::Arc;
+///
+/// trait MyTrait {}
+///
+/// struct Example;
+/// impl MyTrait for Example {}
+///
+/// // `obj` has type `Arc<Example>`.
+/// let obj: Arc<Example> = Arc::try_new(Example)?;
+///
+/// // `coerced` has type `Arc<dyn MyTrait>`.
+/// let coerced: Arc<dyn MyTrait> = obj;
+/// ```
pub struct Arc<T: ?Sized> {
ptr: NonNull<ArcInner<T>>,
_p: PhantomData<ArcInner<T>>,
@@ -96,6 +117,10 @@ struct ArcInner<T: ?Sized> {
// This is to allow [`Arc`] (and variants) to be used as the type of `self`.
impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
+// This is to allow coercion from `Arc<T>` to `Arc<U>` if `T` can be converted to the
+// dynamically-sized type (DST) `U`.
+impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
+
// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox