From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f74.google.com (mail-ej1-f74.google.com [209.85.218.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 85B792472A2 for ; Sat, 4 Jul 2026 21:03:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.74 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783199005; cv=none; b=khCYEgPokCFc02Oberj9OcsIZF4pZ878v/sFZhgStuOGdpykbaaZshfNg+1lg7ZtaervNrMWbmop+IUG6MFSkF4y/ETNIqTFKb2qEYQPFzniqxTv4XFJv7nI2rlTtdhNrxW1NjhLPxCT7natWmticTG6jbp+fyElJsxLp9E3lXw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783199005; c=relaxed/simple; bh=Ck4JJaZmgkoCl/I+djI8TbJbk2wJ29zl60Y1stgczUA=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=L2ybxSL4M6YCh6KZOhQRvfqs+4HgDZBW8RJO/mHSB7UnbB7JzgP+3Ij2x6bLLv1LbNmbLVG9mMWSXGdYmME4lvH91zZF1OCdAclyP7RaAgyqdIcVKfhjcyrFqzVB/lAi9X8hzMb9ozY1zZGdw9t7rLi9OOaBFe4RZegEr+4RYhQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=vU6DJ9Ug; arc=none smtp.client-ip=209.85.218.74 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="vU6DJ9Ug" Received: by mail-ej1-f74.google.com with SMTP id a640c23a62f3a-c1288e572c9so153252566b.0 for ; Sat, 04 Jul 2026 14:03:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1783199001; x=1783803801; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ug4GAW6hRxP0V0DrpHgPLZXGFh1mzzQejcgwIy96G7s=; b=vU6DJ9Ug18ETh0nOMy9EYiHEtqsG9HWS//Q70llrG9vFSWApsKZVEhO5iQPvhtR32r R5vkFh9mS7IUo/rc8nZffdFEzpjU953LxYM2K+Dojmd3jgjp7Es28+22lHCVW26l4QKC YQsX0DndmI4Y85Dse++KvXxACqUxUO9WgWXXtxh3dW1C2KdrM65+jlvQzmhI0gDsFZxP iQR6D39NFRWrNMq24xX2rWmlQd23gb7V0NHCo/yuy8A3z21UbmB44HP0XMwdX3SbZrqC nIS0+PyYte82bJ4HILWbmNeP5jHDx9IvN57AQGxGz1TNFID1sz8Q/1aaTfMLderjrlvX iKbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783199001; x=1783803801; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ug4GAW6hRxP0V0DrpHgPLZXGFh1mzzQejcgwIy96G7s=; b=M0ureJbjdxLK5FThsW9fu0n2NOFpW9Pw8Xuh4jnK5tZbaUNU7u5hJYXg464+WzlvXw wP04n+GTf3VkUyNkWlzOMLhDoEMooXdmqdjMFYBr9wHAu23l3X4ymtlRTe6f8io6uDM9 uLD8ZApqTxtwQAIuIdZhfyOk5zIGPiLQ+BYCZtQ8EbJXfGgDSmChvEGhLrzEXWtYYLkF Y6FXt+Q2d+GTNb7kJxbHEDFWO1MNAVXYx9lOIPJEemtwFnxpStGRurR7ozvUgx8hY2hE 9sL2SS8OuGUm65Uf8jw9pmfwO48H0aDZR5wC9JZYs9xHAAr3T8avtjf2ogbiEquttbDm 8aAA== X-Forwarded-Encrypted: i=1; AHgh+RrMJ+Abfa5WCdzm/n107e7lkRnIscsdrWxdBw8sk4iTTedKDrqxOsKCsK+6+ANK/LECjbUoSq7GIy2YJso=@vger.kernel.org X-Gm-Message-State: AOJu0YyhG9bfz8PHpgmzcEzFhPft2XhS1ZJLwKv7G0QahV5TQLn3N2F9 AeVv2H2eF1Nuq3ZjvuoO+rXHJF/x+o8Hw61fycZJVVSZQ52XZBO9PmOKriMPSmdOeRnLEp2qehp FKp5s7y638Cm/oeN2Rg== X-Received: from ejju21.prod.google.com ([2002:a17:906:4095:b0:c12:4bf3:86c1]) (user=aliceryhl job=prod-delivery.src-stubby-dispatcher) by 2002:a17:906:ba8c:b0:c12:8798:7bf9 with SMTP id a640c23a62f3a-c12e6c1ec1dmr126199966b.46.1783199000441; Sat, 04 Jul 2026 14:03:20 -0700 (PDT) Date: Sat, 4 Jul 2026 21:03:19 +0000 In-Reply-To: <20260703-rust_binder_debug_mask-v1-1-9bdf12b5325c@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260703-rust_binder_debug_mask-v1-0-9bdf12b5325c@google.com> <20260703-rust_binder_debug_mask-v1-1-9bdf12b5325c@google.com> Message-ID: Subject: Re: [PATCH 1/7] rust_binder: Add dynamic debug logging mask From: Alice Ryhl To: Jahnavi MN Cc: Greg Kroah-Hartman , "Arve =?utf-8?B?SGrDuG5uZXbDpWc=?=" , Todd Kjos , Christian Brauner , Carlos Llamas , Miguel Ojeda , Boqun Feng , Gary Guo , "=?utf-8?B?QmrDtnJu?= Roy Baron" , Benno Lossin , Andreas Hindborg , Trevor Gross , Danilo Krummrich , Daniel Almeida , Tamir Duberstein , Alexandre Courbot , "Onur =?utf-8?B?w5Z6a2Fu?=" , linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org Content-Type: text/plain; charset="utf-8" On Fri, Jul 03, 2026 at 03:29:22PM +0000, Jahnavi MN wrote: > Implement a dynamic debug logging mask (`debug_mask`) for the > `rust_binder` module to allow dynamic runtime configuration of log > levels. This enables parity with the legacy C driver's debug mask. > > Since the Rust `module!` macro in the current kernel build does not yet > support declaring module parameters directly in Rust, we define the > `debug_mask` variable in a C companion file to expose it to the kernel > runtime and import it using FFI with volatile reads. > > To verify the setup, instrument process lifecycle events (open, flush, > and release) in `process.rs` under the new `BINDER_DEBUG_OPEN_CLOSE` > logging mask. These entry-point events are chosen for initial validation > because they represent the start of the Binder lifecycle and occur > at low frequency, allowing simple runtime verification of the dynamic > toggle without log noise. > > Signed-off-by: Jahnavi MN > diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs > index 96b8440ceac6..c13a04a3afcb 100644 > --- a/drivers/android/binder/process.rs > +++ b/drivers/android/binder/process.rs > @@ -898,7 +898,6 @@ pub(crate) fn insert_or_update_handle( > } > > pub(crate) fn get_transaction_node(&self, handle: u32) -> BinderResult { > - // When handle is zero, try to get the context manager. > if handle == 0 { > Ok(self.ctx.get_manager_node(true)?) > } else { Removing this comment appears to be a spurious change. > @@ -1314,6 +1313,11 @@ pub(crate) fn lock_with_nodes(&self) -> WithNodes<'_> { > } > > fn deferred_flush(&self) { > + binder_debug!( > + crate::debug::BINDER_DEBUG_OPEN_CLOSE, > + "binder_flush: process {}", > + self.pid_in_current_ns() > + ); > let inner = self.inner.lock(); > for thread in inner.threads.values() { > thread.exit_looper(); > @@ -1321,6 +1325,11 @@ fn deferred_flush(&self) { > } > > fn deferred_release(self: Arc) { > + binder_debug!( > + crate::debug::BINDER_DEBUG_OPEN_CLOSE, > + "binder_deferred_release: process {}", > + self.pid_in_current_ns() > + ); Since you included the pid in the binder_debug! macro itself, you can remove it from these two println statements. Also, I think it'd be nicer to word these as "flushing process" and "releasing process". > diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs > index 97d5f31e8fe3..c908dde5796a 100644 > --- a/drivers/android/binder/thread.rs > +++ b/drivers/android/binder/thread.rs > @@ -1222,6 +1222,7 @@ fn read_transaction_info( > info.buffers_size = td.buffers_size as usize; > // SAFETY: Above `read` call initializes all bytes, so this union read is ok. > info.target_handle = unsafe { td.transaction_data.target.handle }; > + info.debug_id = super::next_debug_id(); > Ok(()) > } > > diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs > index 1d9b66920a21..f9285eb93fc0 100644 > --- a/drivers/android/binder/transaction.rs > +++ b/drivers/android/binder/transaction.rs > @@ -42,6 +42,7 @@ pub(crate) struct TransactionInfo { > pub(crate) reply: u32, > pub(crate) oneway_spam_suspect: bool, > pub(crate) is_reply: bool, > + pub(crate) debug_id: usize, > } > > impl TransactionInfo { > @@ -93,7 +94,7 @@ pub(crate) fn new( > from: &Arc, > info: &mut TransactionInfo, > ) -> BinderResult> { > - let debug_id = super::next_debug_id(); > + let debug_id = info.debug_id; > let allow_fds = node_ref.node.flags & FLAT_BINDER_FLAG_ACCEPTS_FDS != 0; > let txn_security_ctx = node_ref.node.flags & FLAT_BINDER_FLAG_TXN_SECURITY_CTX != 0; > let mut txn_security_ctx_off = if txn_security_ctx { Some(0) } else { None }; > @@ -152,7 +153,7 @@ pub(crate) fn new_reply( > info: &mut TransactionInfo, > allow_fds: bool, > ) -> BinderResult> { > - let debug_id = super::next_debug_id(); > + let debug_id = info.debug_id; It looks like this change to move debug_id so that it's present in TransactionInfo is happening in the wrong patch. Ideally this change would be part of the patch that actually needs to read debug_id from TransactionInfo. Alice