Date:   Tue, 26 Oct 2021 23:40:03 +0800
From:   Huan Xie <xiehuan09@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Masami Hiramatsu <mhiramat@...nel.org>, mingo@...hat.com,
        chenhuacai@...nel.org, linux-kernel@...r.kernel.org,
        Tom Zanussi <zanussi@...nel.org>
Subject: Re: [RFC PATCH v2] trace: Add trace any kernel object

On Tue, Oct 26, 2021 at 10:47 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Tue, 26 Oct 2021 16:50:46 +0800
> Huan Xie <xiehuan09@...il.com> wrote:
>
> > > > +static void submit_trace_object(unsigned long ip, unsigned long parent_ip,
> > > > +                              unsigned long object)
> > > > +{
> > > > +
> > > > +     struct trace_buffer *buffer;
> > > > +     struct ring_buffer_event *event;
> > > > +     struct trace_object_entry *entry;
> > > > +     int pc;
> > > > +
> > > > +     pc = preempt_count();
> > > > +     event = trace_event_buffer_lock_reserve(&buffer, &event_trace_file,
> > > > +                     TRACE_OBJECT, sizeof(*entry), pc);
> > > > +     if (!event)
> > > > +             return;
> > > > +     entry   = ring_buffer_event_data(event);
> > > > +     entry->ip                       = ip;
> > > > +     entry->parent_ip                = parent_ip;
> > > > +     entry->object                   = object;
> > >
> > > So here we are just recording the value we saw at the kprobe (not very
> > > interesting).
> > >
> > > I think we want the content of the object:
> > >
> > >         long val;
> > >
> > >         ret = copy_from_kernel_nofault(&val, object, sizeof(val));
> > >         if (ret)
> > >                 val = 0;
> >
> > This place is the only thing I don't understand, don't know  why and
> > where to use the copy_from_kernel_nofault.
>
>
> If we have the address of the symbol, we want to read what's at that
> address, right?
>
> >
> > we can only get the struct pt_regs from the  __kprobe_trace_fun() ,
> > and use it on the  trace_object_trigger() ,
> > so need to save the pt_regs using a struct:
> >
> > struct object_trigger_param {
> >         struct pt_regs *regs;
> >         int param;
> > };
> >
> > /* Kprobe handler */
> > static nokprobe_inline void __kprobe_trace_func(struct trace_kprobe
> > *tk, struct pt_regs *regs,
> >                     struct trace_event_file *trace_file)
> >
> >
> > static void trace_object_trigger(struct event_trigger_data *data,
> > struct trace_buffer *buffer,  void *rec,
> >                    struct ring_buffer_event *event)
>
>
> OK, so let me ask this question. What is it that you want to see?

Thanks, I got your point now,  my original idea was to just track the
flow of objects.

> If we have (using your example):
>
> int bio_add_page(struct bio *bio, struct page *page,
>                                 unsigned int len, unsigned int offset)
>
> And we want to trace "bio" right?
>
> Doing:
>
>   echo 'p bio_add_page arg1=$arg1' > kprobe_events
>
> Will make "arg1" be assigned the pointer that was passed in.
>
>   0xffff888102a4b900
>
> Which is a local variable that holds an address to some structure bio.
>
> Your current example just keeps showing us that same pointer address and
> not the content of bio, and will never change until the bio_add_page
> function is called again, in which case, you will now be tracing the
> next address of the structure that was passed into the function. There's
> nothing more to learn from this over just tracing that function and giving
> us the address passed in.
>
> Now if I look at struct bio, I see:
>
> struct bio {
>         [..]
>         atomic_t                __bi_cnt;       /* pin count */
>         [..]
> };
>
> And let's say I want to monitor that __bi_cnt while functions are being
> traced. What would be *really cool*, is to mark that value!

This is really cool to  get the change of the value. And is a good
feature enhancement.

>
> // find the offset of __bi_cnt in struct bio:
> $ gdb vmlinux
> (gdb) p &((struct bio *)0)->__bi_cnt
> $1 = (atomic_t *) 0x64
>
>  # echo 'objfilter:0x64(arg1) if comm == "cat"' > ./trigger
>
> Which would then read that arg1=0xffff888102a4b900 and offset it by 0x64,
> and give me the value at that location:
>
>   *(0xffff888102a4b900 + 0x64)
>
> at every function. Then I could watch the __bi_cnt change over time. But to
> dereference memory safely, we need to use copy_from_kernel_nofault()
> because that address "0xffff888102a4b900 + 0x64" could point to nothing
> and fault / crash the kernel.
>
>         obj = arg1 + 0x64
>         if (copy_from_kernel_nofault(&val, arg1 + 0x64, sizeof(val)))
>                 // faulted
>                 return;
>
> Now val has the content of __bi_cnt and we can print that!
>
> -- Steve
>
>
>
> >
> > > Then we can see what changed during this time.

Powered by blists - more mailing lists