Messages in this thread | | | Date | Thu, 13 Apr 2023 14:17:43 +0200 | From | Greg KH <> | Subject | Re: [PATCH] Binder: Add timestamp and async from pid/tid to transaction record |
| |
On Thu, Apr 13, 2023 at 07:33:26PM +0800, Chuang Zhang wrote: > On Thu, Apr 13, 2023 at 6:51 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > spin_lock(&t->lock); > > > to_proc = t->to_proc; > > > seq_printf(m, > > > - "%s %d: %pK from %d:%d to %d:%d code %x flags %x pri > > %ld r%d", > > > + "%s %d: %pK from %d:%d to %d:%d code %x elapsed %lldms > > flags %x pri %ld r%d", > > > > Why not add this at the end of the line instead of in the middle? That > > way any existing code that looks at this line isn't as broken as it > > might be with your change :) > > > >> [chuang] Ok, so move it after t->need_reply or buffer->user_data?
After need_reply?
> > > --- a/drivers/android/binder_internal.h > > > +++ b/drivers/android/binder_internal.h > > > @@ -528,6 +528,9 @@ struct binder_transaction { > > > long priority; > > > long saved_priority; > > > kuid_t sender_euid; > > > + int async_from_pid; > > > + int async_from_tid; > > > > As you are just using 0/1 for these variables, why not add them to the > > bitfield location in this structure instead of wasting a whole int for > > both of them? > > > >> [chuang] They are used to store the caller's process ID and thread ID, > not just 1 or 0, so I understand that integers are still needed to save > them. thanks!
Ah, I missed that these really were the pid itself. Then an int will not work at all, it has to be pid_t, right?
And how are pid namespaces being handled here? I know it's just debugging data but showing a pid from the wrong namespace probably would not be a good idea as it might get confusing quickly.
thanks,
greg k-h
| |