[PATCH] rust_binder: use Atomic::add() in BinderStats

[PATCH] rust_binder: use Atomic::add() in BinderStats

[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

Re: [PATCH] rust_binder: use Atomic::add() in BinderStats

[Boqun Feng]

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
> 

Re: [PATCH] rust_binder: use Atomic::add() in BinderStats

[FUJITA Tomonori]

On Sat, 31 Oct 2026 03:08:01 +0000
Jenhao Chen <jenhaochen@google.com> 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().
> 
> No functional change intended.
> 
> Signed-off-by: Jenhao Chen <jenhaochen@google.com>

Reviewed-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

[PATCH v2] rust_binder: use Atomic::add() in BinderStats

[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().

Reviewed-by: Boqun Feng <boqun@kernel.org>
Reviewed-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
Signed-off-by: Jenhao Chen <jenhaochen@google.com>
---
Changes in v2:
- Drop "No functional change intended." from commit description (Boqun Feng).
- Collect Reviewed-by tags from Boqun Feng and FUJITA Tomonori.
- Link to v1: https://lore.kernel.org/r/20261031030801.4100693-1-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

Re: [PATCH v2] rust_binder: use Atomic::add() in BinderStats

[Alice Ryhl]

On Sat, Oct 31, 2026 at 4:09 PM Jenhao Chen <jenhaochen@google.com> 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().
>
> Reviewed-by: Boqun Feng <boqun@kernel.org>
> Reviewed-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> Signed-off-by: Jenhao Chen <jenhaochen@google.com>

Acked-by: Alice Ryhl <aliceryhl@google.com>

Greg, please pick this up, thanks!