lkml.org 
[lkml]   [2023]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2023-04-13 19:06    [W:0.340 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site