linux-kernel - Re: [PATCH] rust_binder: move BC_FREE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aQ5sYW7B9riNRNHI@google.com>
Date: Fri, 7 Nov 2025 22:02:09 +0000
From: Carlos Llamas <cmllamas@...gle.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Arve Hjønnevåg <arve@...roid.com>,
	Todd Kjos <tkjos@...roid.com>, Martijn Coenen <maco@...roid.com>,
	Joel Fernandes <joelagnelf@...dia.com>,
	Christian Brauner <brauner@...nel.org>,
	Suren Baghdasaryan <surenb@...gle.com>,
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH] rust_binder: move BC_FREE_BUFFER drop inside if statement

On Wed, Oct 29, 2025 at 11:50:58AM +0000, Alice Ryhl wrote:
> When looking at flamegraphs, there is a pretty large entry for the
> function call drop_in_place::<Option<Allocation>> which in turn calls
> drop_in_place::<Allocation>. Combined with the looper_need_return
> condition, this means that the generated code looks like this:
> 
> 	if let Some(buffer) = buffer {
> 	    if buffer.looper_need_return_on_free() {
> 	        self.inner.lock().looper_need_return = true;
> 	    }
> 	}
> 	drop_in_place::<Option<Allocation>>() { // not inlined
> 	    if let Some(buffer) = buffer {
> 	    	drop_in_place::<Allocation>(buffer);
> 	    }
> 	}
> 
> This kind of situation where you check X and then check X again is
> normally optimized into a single condition, but in this case due to the
> non-inlined function call to drop_in_place::<Option<Allocation>>, that
> optimization does not happen.
> 
> Furthermore, the drop_in_place::<Allocation> call is only two-thirds of
> the drop_in_place::<Option<Allocation>> call in the flamegraph. This
> indicates that this double condition is not performing well. Also, last
> time I looked at Binder perf, I remember finding that the destructor of
> Allocation was involved with many branch mispredictions.
> 
> Thus, change this code to look like this:
> 
> 	if let Some(buffer) = buffer {
> 	    if buffer.looper_need_return_on_free() {
> 	        self.inner.lock().looper_need_return = true;
> 	    }
> 	    drop_in_place::<Allocation>(buffer);
> 	}
> 
> by dropping the Allocation directly. Flamegraphs confirm that the
> drop_in_place::<Option<Allocation>> call disappears from this change.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> ---

LGTM,

Acked-by: Carlos Llamas <cmllamas@...gle.com>