* [PATCH] rust_binder: use Atomic::add() in BinderStats
@ 2026-10-31 3:08 Jenhao Chen
2026-10-31 4:00 ` Boqun Feng
0 siblings, 1 reply; 2+ messages in thread
From: Jenhao Chen @ 2026-10-31 3:08 UTC (permalink / raw)
To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, Christian Brauner, Carlos Llamas,
Alice Ryhl
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
FUJITA Tomonori, Qais Yousef, Ben Wei, rust-for-linux,
linux-kernel, Jenhao Chen
BinderStats::inc_bc() and BinderStats::inc_br() increment command and
return statistics counters on every Binder transaction and reply using
Atomic::fetch_add(1, Relaxed), discarding the returned previous value.
Because the return value of fetch_add() is unused in Rust but must still
be produced by the underlying value-returning atomic RMW helper
(atomic_fetch_add_relaxed()), the compiler is forced to emit a
value-returning atomic operation (LDADD on arm64 with LSE, or LOCK XADD
on x86) instead of a non-value-returning atomic add (STADD on arm64, or
LOCK ADD on x86). On arm64 CPUs with LSE, value-returning LDADD requires
a round-trip response from the L2/interconnect to populate the
destination register, whereas non-value-returning STADD (encoded as
LDADD with WZR as the destination register) may be executed as a far
atomic without waiting for the old value to return to the core.
Switch BinderStats::inc_bc() and BinderStats::inc_br() from
fetch_add(1, Relaxed) to add(1, Relaxed) so they lower to atomic_add()
and emit non-value-returning atomic increments. This also matches the C
Binder driver, which uses atomic_inc() for these statistics counters in
binder_thread_write() and binder_stat_br().
No functional change intended.
Signed-off-by: Jenhao Chen <jenhaochen@google.com>
---
drivers/android/binder/stats.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/android/binder/stats.rs b/drivers/android/binder/stats.rs
index ec81dc7747db..68cb0cf48865 100644
--- a/drivers/android/binder/stats.rs
+++ b/drivers/android/binder/stats.rs
@@ -32,14 +32,14 @@ impl BinderStats {
pub(crate) fn inc_bc(&self, bc: u32) {
let idx = _IOC_NR(bc) as usize;
if let Some(bc_ref) = self.bc.get(idx) {
- bc_ref.fetch_add(1, Relaxed);
+ bc_ref.add(1, Relaxed);
}
}
pub(crate) fn inc_br(&self, br: u32) {
let idx = _IOC_NR(br) as usize;
if let Some(br_ref) = self.br.get(idx) {
- br_ref.fetch_add(1, Relaxed);
+ br_ref.add(1, Relaxed);
}
}
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] rust_binder: use Atomic::add() in BinderStats
2026-10-31 3:08 [PATCH] rust_binder: use Atomic::add() in BinderStats Jenhao Chen
@ 2026-10-31 4:00 ` Boqun Feng
0 siblings, 0 replies; 2+ messages in thread
From: Boqun Feng @ 2026-10-31 4:00 UTC (permalink / raw)
To: Jenhao Chen
Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, Christian Brauner, Carlos Llamas,
Alice Ryhl, Miguel Ojeda, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
FUJITA Tomonori, Qais Yousef, Ben Wei, rust-for-linux,
linux-kernel
On Sat, Oct 31, 2026 at 03:08:01AM +0000, Jenhao Chen wrote:
> BinderStats::inc_bc() and BinderStats::inc_br() increment command and
> return statistics counters on every Binder transaction and reply using
> Atomic::fetch_add(1, Relaxed), discarding the returned previous value.
>
> Because the return value of fetch_add() is unused in Rust but must still
> be produced by the underlying value-returning atomic RMW helper
> (atomic_fetch_add_relaxed()), the compiler is forced to emit a
> value-returning atomic operation (LDADD on arm64 with LSE, or LOCK XADD
> on x86) instead of a non-value-returning atomic add (STADD on arm64, or
> LOCK ADD on x86). On arm64 CPUs with LSE, value-returning LDADD requires
> a round-trip response from the L2/interconnect to populate the
> destination register, whereas non-value-returning STADD (encoded as
> LDADD with WZR as the destination register) may be executed as a far
> atomic without waiting for the old value to return to the core.
>
> Switch BinderStats::inc_bc() and BinderStats::inc_br() from
> fetch_add(1, Relaxed) to add(1, Relaxed) so they lower to atomic_add()
> and emit non-value-returning atomic increments. This also matches the C
> Binder driver, which uses atomic_inc() for these statistics counters in
> binder_thread_write() and binder_stat_br().
>
Make sense, thank you!
> No functional change intended.
>
However I would remove the "No functional change intended", because a
code gen change is still somewhat a functional change to me. Anyway,
with or without it:
Reviewed-by: Boqun Feng <boqun@kernel.org>
Regards,
Boqun
> Signed-off-by: Jenhao Chen <jenhaochen@google.com>
> ---
> drivers/android/binder/stats.rs | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/android/binder/stats.rs b/drivers/android/binder/stats.rs
> index ec81dc7747db..68cb0cf48865 100644
> --- a/drivers/android/binder/stats.rs
> +++ b/drivers/android/binder/stats.rs
> @@ -32,14 +32,14 @@ impl BinderStats {
> pub(crate) fn inc_bc(&self, bc: u32) {
> let idx = _IOC_NR(bc) as usize;
> if let Some(bc_ref) = self.bc.get(idx) {
> - bc_ref.fetch_add(1, Relaxed);
> + bc_ref.add(1, Relaxed);
> }
> }
>
> pub(crate) fn inc_br(&self, br: u32) {
> let idx = _IOC_NR(br) as usize;
> if let Some(br_ref) = self.br.get(idx) {
> - br_ref.fetch_add(1, Relaxed);
> + br_ref.add(1, Relaxed);
> }
> }
>
> --
> 2.56.0.317.g454f8302e1-goog
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-31 4:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-31 3:08 [PATCH] rust_binder: use Atomic::add() in BinderStats Jenhao Chen
2026-10-31 4:00 ` Boqun Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®