[patch v4 08/13] remote, btrace: add branch trace remote ops

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Wednesday, November 28, 2012 8:23 PM

Thanks for your review.

[...]

> >   Qbtrace:on:<ptid>  enable branch tracing for one thread
> >                      returns "OK" or "Enn"
> >
> >   Qbtrace:off:<ptid>  disable branch tracing for one thread
> >                       returns "OK" or "Enn"
> 
> Nit,  I'd put <ptid> first:

In the context of your comment regarding qXfer:btrace:read, should I rather omit the ptid here, as well, and use set_general_thread?


> >   Qbtrace:<ptid>:on  enable branch tracing for one thread
> >                      returns "OK" or "Enn"
> >
> >   Qbtrace:<ptid>:off  disable branch tracing for one thread
> >                       returns "OK" or "Enn"
> 
> 
> How does "btrace enable all" for new threads work with remote targets?
> GDB isn't notified of new threads until they stop for some reason.

I ran into this problem, as well. At the moment, btrace is configured after the first stop. This is not what I would want, but all that I could get.

Is this some conscious design decision? Is gdbserver supposed to handle this automatic enabling? Or could we change this and have gdbserver notify gdb about new threads? This would also address the update_therad_list problem, where commands might work on an outdated thread list until the user says "info threads".

[...]

> > +struct btrace_target_info
> > +{
> > +  /* The ptid of the traced thread.  */
> > +  ptid_t ptid;
> > +};
> 
> Hmm, why would you need to store this?  To get at a struct btrace_target_info,
> you need to start from a struct thread_info, so why not just pass that
> down, or pass the ptid down?

This struct is supposed to contain all the information gdb needs to implement branch tracing for one thread. In the native case, it holds the perf event ring buffer and the perf event configuration. In the remote case, it holds the ptid for communication with gdbserver. The perf event specific data is stored in gdbserver.

If I passed a struct thread_info, btrace related functions would work directly on the thread info. I would still need some mechanism to store different data for different implementations. And I also use the btrace_target_info pointer to indicate whether btrace has been enabled for this thread.


[...]

Thanks,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052