Re: [PATCH v2 3/3] rust_binder: add in the new tracepoint calls

[Alice Ryhl <aliceryhl@google.com>]

On Sun, Mar 08, 2026 at 04:24:36AM +0300, Mohamad Alsadhan wrote:
> Wire the new Rust Binder tracepoints into live execution paths.
> 
> Hook trace calls into:
> 
> - ioctl entry/exit (`binder_ioctl`, `binder_ioctl_done`)
> - command parsing and return writes (`binder_command`,
>   `binder_return`)
> - read/write completion (`binder_read_done`, `binder_write_done`)
> - wait (`binder_wait_for_work`)
> - transaction (`binder_transaction_received`)
> - fd translation (`binder_transaction_fd_{send,recv}`)
> - buffer/page lifecycle (`binder_alloc_*`, `binder_free_*`,
>   `binder_unmap_*`)
> 
> Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc>

Thanks for your series.

Instead of having one commit declaring *all* the tracepoints, and
another commit calling *all* the tracepoints, this needs to be one
commit per tracepoint category. Declaring the tracepoint and adding its
caller can happen in the same commit.

Also, I'm not sure we need the buffer/page lifecycle ones, so I think we
can just drop those.

Alice

> ---
>  drivers/android/binder/allocation.rs       |  1 +
>  drivers/android/binder/page_range.rs       | 17 +++++++++++++++++
>  drivers/android/binder/process.rs          |  7 +++++--
>  drivers/android/binder/rust_binder_main.rs |  1 +
>  drivers/android/binder/thread.rs           | 15 +++++++++++++--
>  drivers/android/binder/transaction.rs      |  2 ++
>  6 files changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/android/binder/allocation.rs b/drivers/android/binder/allocation.rs
> index 7f65a9c3a..8c0f94463 100644
> --- a/drivers/android/binder/allocation.rs
> +++ b/drivers/android/binder/allocation.rs
> @@ -208,6 +208,7 @@ pub(crate) fn translate_fds(&mut self) -> Result<TranslatedFds> {
>              let res = FileDescriptorReservation::get_unused_fd_flags(bindings::O_CLOEXEC)?;
>              let fd = res.reserved_fd();
>              self.write::<u32>(file_info.buffer_offset, &fd)?;
> +            crate::trace::trace_transaction_fd_recv(self.debug_id, fd, file_info.buffer_offset);
>  
>              reservations.push(
>                  Reservation {
> diff --git a/drivers/android/binder/page_range.rs b/drivers/android/binder/page_range.rs
> index fdd97112e..43fe23907 100644
> --- a/drivers/android/binder/page_range.rs
> +++ b/drivers/android/binder/page_range.rs
> @@ -327,6 +327,8 @@ pub(crate) fn use_range(&self, start: usize, end: usize) -> Result<()> {
>  
>              // SAFETY: The pointer is valid, and we hold the lock so reading from the page is okay.
>              if let Some(page) = unsafe { PageInfo::get_page(page_info) } {
> +                crate::trace::trace_alloc_lru_start(self.pid, i);
> +
>                  // Since we're going to use the page, we should remove it from the lru list so that
>                  // the shrinker will not free it.
>                  //
> @@ -335,9 +337,12 @@ pub(crate) fn use_range(&self, start: usize, end: usize) -> Result<()> {
>                  // The shrinker can't free the page between the check and this call to
>                  // `list_lru_del` because we hold the lock.
>                  unsafe { PageInfo::list_lru_del(page_info, page.nid(), self.shrinker) };
> +
> +                crate::trace::trace_alloc_lru_end(self.pid, i);
>              } else {
>                  // We have to allocate a new page. Use the slow path.
>                  drop(inner);
> +                crate::trace::trace_alloc_page_start(self.pid, i);
>                  // SAFETY: `i < end <= inner.size` so `i` is in bounds.
>                  match unsafe { self.use_page_slow(i) } {
>                      Ok(()) => {}
> @@ -346,6 +351,7 @@ pub(crate) fn use_range(&self, start: usize, end: usize) -> Result<()> {
>                          return Err(err);
>                      }
>                  }
> +                crate::trace::trace_alloc_page_end(self.pid, i);
>                  inner = self.lock.lock();
>              }
>          }
> @@ -448,8 +454,12 @@ pub(crate) fn stop_using_range(&self, start: usize, end: usize) {
>  
>              // SAFETY: Okay for reading since we have the lock.
>              if let Some(page) = unsafe { PageInfo::get_page(page_info) } {
> +                crate::trace::trace_free_lru_start(self.pid, i);
> +
>                  // SAFETY: The pointer is valid, and it's the right shrinker.
>                  unsafe { PageInfo::list_lru_add(page_info, page.nid(), self.shrinker) };
> +
> +                crate::trace::trace_free_lru_end(self.pid, i);
>              }
>          }
>      }
> @@ -667,6 +677,7 @@ fn drop(self: Pin<&mut Self>) {
>      let mmap_read;
>      let mm_mutex;
>      let vma_addr;
> +    let pid;
>  
>      {
>          // CAST: The `list_head` field is first in `PageInfo`.
> @@ -700,7 +711,9 @@ fn drop(self: Pin<&mut Self>) {
>  
>          // SAFETY: Both pointers are in bounds of the same allocation.
>          page_index = unsafe { info.offset_from(inner.pages) } as usize;
> +        pid = range.pid;
>  
> +        crate::trace::trace_unmap_kernel_start(pid, page_index);
>          // SAFETY: We hold the spinlock, so we can take the page.
>          //
>          // This sets the page pointer to zero before we unmap it from the vma. However, we call
> @@ -709,6 +722,8 @@ fn drop(self: Pin<&mut Self>) {
>          page = unsafe { PageInfo::take_page(info) };
>          vma_addr = inner.vma_addr;
>  
> +        crate::trace::trace_unmap_kernel_end(pid, page_index);
> +
>          // From this point on, we don't access this PageInfo or ShrinkablePageRange again, because
>          // they can be freed at any point after we unlock `lru_lock`. This is with the exception of
>          // `mm_mutex` which is kept alive by holding the lock.
> @@ -719,7 +734,9 @@ fn drop(self: Pin<&mut Self>) {
>  
>      if let Some(vma) = mmap_read.vma_lookup(vma_addr) {
>          let user_page_addr = vma_addr + (page_index << PAGE_SHIFT);
> +        crate::trace::trace_unmap_user_start(pid, page_index);
>          vma.zap_page_range_single(user_page_addr, PAGE_SIZE);
> +        crate::trace::trace_unmap_user_end(pid, page_index);
>      }
>  
>      drop(mmap_read);
> diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
> index 41de55931..4d7b165e7 100644
> --- a/drivers/android/binder/process.rs
> +++ b/drivers/android/binder/process.rs
> @@ -1656,11 +1656,14 @@ pub(crate) fn ioctl(this: ArcBorrow<'_, Process>, file: &File, cmd: u32, arg: us
>  
>          const _IOC_READ_WRITE: u32 = _IOC_READ | _IOC_WRITE;
>  
> -        match _IOC_DIR(cmd) {
> +        let res = match _IOC_DIR(cmd) {
>              _IOC_WRITE => Self::ioctl_write_only(this, file, cmd, &mut user_slice.reader()),
>              _IOC_READ_WRITE => Self::ioctl_write_read(this, file, cmd, user_slice),
>              _ => Err(EINVAL),
> -        }
> +        };
> +
> +        crate::trace::trace_ioctl_done(res);
> +        res
>      }
>  
>      pub(crate) fn mmap(
> diff --git a/drivers/android/binder/rust_binder_main.rs b/drivers/android/binder/rust_binder_main.rs
> index aa5f2a75a..1028e0a8a 100644
> --- a/drivers/android/binder/rust_binder_main.rs
> +++ b/drivers/android/binder/rust_binder_main.rs
> @@ -116,6 +116,7 @@ fn new(writer: UserSliceWriter, thread: &'a Thread) -> Self {
>      /// Write a return code back to user space.
>      /// Should be a `BR_` constant from [`defs`] e.g. [`defs::BR_TRANSACTION_COMPLETE`].
>      fn write_code(&mut self, code: u32) -> Result {
> +        crate::trace::trace_return(code);
>          stats::GLOBAL_STATS.inc_br(code);
>          self.thread.process.stats.inc_br(code);
>          self.writer.write(&code)
> diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs
> index 0b62d24b2..ef7fba700 100644
> --- a/drivers/android/binder/thread.rs
> +++ b/drivers/android/binder/thread.rs
> @@ -706,6 +706,7 @@ fn translate_object(
>                      core::mem::offset_of!(uapi::binder_fd_object, __bindgen_anon_1.fd);
>  
>                  let field_offset = offset + FD_FIELD_OFFSET;
> +                crate::trace::trace_transaction_fd_send(view.alloc.debug_id, fd, field_offset);
>  
>                  view.alloc.info_add_fd(file, field_offset, false)?;
>              }
> @@ -1323,6 +1324,7 @@ fn write(self: &Arc<Self>, req: &mut BinderWriteRead) -> Result {
>          while reader.len() >= size_of::<u32>() && self.inner.lock().return_work.is_unused() {
>              let before = reader.len();
>              let cmd = reader.read::<u32>()?;
> +            crate::trace::trace_command(cmd);
>              GLOBAL_STATS.inc_bc(cmd);
>              self.process.stats.inc_bc(cmd);
>              match cmd {
> @@ -1412,11 +1414,18 @@ fn read(self: &Arc<Self>, req: &mut BinderWriteRead, wait: bool) -> Result {
>              UserSlice::new(UserPtr::from_addr(read_start as _), read_len as _).writer(),
>              self,
>          );
> -        let (in_pool, use_proc_queue) = {
> +        let (in_pool, has_transaction, thread_todo, use_proc_queue) = {
>              let inner = self.inner.lock();
> -            (inner.is_looper(), inner.should_use_process_work_queue())
> +            (
> +                inner.is_looper(),
> +                inner.current_transaction.is_some(),
> +                !inner.work_list.is_empty(),
> +                inner.should_use_process_work_queue(),
> +            )
>          };
>  
> +        crate::trace::trace_wait_for_work(use_proc_queue, has_transaction, thread_todo);
> +
>          let getter = if use_proc_queue {
>              Self::get_work
>          } else {
> @@ -1482,6 +1491,7 @@ pub(crate) fn write_read(self: &Arc<Self>, data: UserSlice, wait: bool) -> Resul
>          let mut ret = Ok(());
>          if req.write_size > 0 {
>              ret = self.write(&mut req);
> +            crate::trace::trace_write_done(ret);
>              if let Err(err) = ret {
>                  pr_warn!(
>                      "Write failure {:?} in pid:{}",
> @@ -1498,6 +1508,7 @@ pub(crate) fn write_read(self: &Arc<Self>, data: UserSlice, wait: bool) -> Resul
>          // Go through the work queue.
>          if req.read_size > 0 {
>              ret = self.read(&mut req, wait);
> +            crate::trace::trace_read_done(ret);
>              if ret.is_err() && ret != Err(EINTR) {
>                  pr_warn!(
>                      "Read failure {:?} in pid:{}",
> diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs
> index 75e6f5fba..c43846bb7 100644
> --- a/drivers/android/binder/transaction.rs
> +++ b/drivers/android/binder/transaction.rs
> @@ -430,6 +430,8 @@ fn do_work(
>  
>          self.drop_outstanding_txn();
>  
> +        crate::trace::trace_transaction_received(&self);
> +
>          // When this is not a reply and not a oneway transaction, update `current_transaction`. If
>          // it's a reply, `current_transaction` has already been updated appropriately.
>          if self.target_node.is_some() && tr_sec.transaction_data.flags & TF_ONE_WAY == 0 {
> 
> -- 
> 2.52.0
>