On Wed, Mar 12, 2025 at 11:49:02AM -0700, Li Li wrote:
> > > + mutex_lock(&binder_procs_lock);
> > > + hlist_for_each_entry(proc, &binder_procs, proc_node) {
> > > + if (proc->pid == pid)
> > > + break;
> >
> > Wait... can't there be multiple binder_proc instances matching the same
> > pid? I know that binder_proc is a bit of a misnomer but what should you
> > do in such case? Shouldn't you set the flags in _all_ matching pids?
> >
> > Furthermore, there could be a single task talking on multiple contexts,
> > so you could be returning the 'proc' that doesn't match the context that
> > you are looking for right?
> >
>
> You're right. I should update this logic to search the process within a
> certain binder_context only.
Also, note the comment about multiple 'struct binder_proc' matching the
same desired pid.
> > > +static void binder_netlink_report(struct binder_context *context, u32 err,
> > > + u32 pid, u32 tid, u32 to_pid, u32 to_tid,
> >
> > Instead of all these parameters, is there a way to pass the transaction
> > itself? Isn't this info already populated there? I think it even holds
> > the info you are looking for from the 'binder_transaction_data' below.
> >
>
> The binder_transaction_data doesn't include all of pid, tid, to_pid and to_tid.
I'm not referring to binder_transaction_data, I mean 'struct
binder_transaction'. I _think_ this should have all you need?
> > > + ret = genlmsg_multicast(&binder_nl_family, skb, 0, BINDER_NLGRP_REPORT, GFP_KERNEL);
> >
> > Thanks for switching to multicast. On this topic, we can only have a
> > single global configuration at a time correct? e.g. context vs per-proc.
> > So all listeners would ahve to work with the same setup?
> >
>
> We only have a single global configuration, which can include both
> context and proc setup.
> Yes, all listeners work with the same setup as we have only one
> multicast group defined.
> The user space code can demux it by checking the context field of the
> netlink messages.
Ack. I understand the demux solution. I was wondering if we'll need to
OR the different configurations (per-proc and flags) from each listener
in that case.
> > > +TRACE_EVENT(binder_netlink_report,
> > > + TP_PROTO(const char *name, u32 err, u32 pid, u32 tid, u32 to_pid,
> > > + u32 to_tid, u32 reply, struct binder_transaction_data *tr),
> >
> > Similarly here I think you could get away with passing 'struct
> > binder_transaction' instead of all the individual fields.
> >
>
> Same as above, the pid/tid fields are not in the struct
> binder_transaction (or redacted for oneway txns).
There is something off here. You have t->from_{pid|tid} and also
t->to_{proc|thead} that you can use. Isn't this what you are looking
for?
--
Carlos Llamas
