Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Kwok Cheung Yeung <kcy@codesourcery.com>]

On 05/05/2011 19:51, Eli Zaretskii wrote:
>> Date: Thu, 05 May 2011 18:27:11 +0100
>> From: Kwok Cheung Yeung <kcy@codesourcery.com>
>>
>> +                     If a trace experiment is started with no enabled
>> +  tracepoints, then a warning will be printed but the experiment will
>> +  proceed anyway.
> 
> You mean, if the experiment started with no enabled tracepoints, and
> the user says "disable", right?  Otherwise, why the warning?
>

Originally, it was an error to try to start an experiment with no enabled
tracepoints because the resulting experiment would never do anything. However,
with the patch, tracepoints can be re-enabled after the experiment starts, so I
downgraded the error to a warning. I suppose we could do away with it
altogether, but then a user who inadvertently disabled all tracepoints might sit
and wonder why nothing seems to be happening.

>> +* New remote packets
>> +
>> +QTEnable
>> +
>> +  Dynamically enable a tracepoint in a started trace experiment.
>> +
>> +QTDisable
>> +
>> +  Dynamically disable a tracepoint in a started trace experiment.
> 
> Can't the existing packets do the job?
> 

As far as I can see, there are no packets that allow a tracepoint that is
already created on the target to be modified. It might be possible to extend
QTDP: to do so, but that would seem to needlessly complicate its definition
(which is to create new tracepoints, not modify existing ones).

>>  Disable tracepoint @var{num}, or all tracepoints if no argument
>> -@var{num} is given.  A disabled tracepoint will have no effect during
>> -the next trace experiment, but it is not forgotten.  You can re-enable
>> -a disabled tracepoint using the @code{enable tracepoint} command.
>> +@var{num} is given.  The change is effective immediately.
> 
> Err... that's inaccurate, isn't it?  The code patch clearly shows that
> sometimes it won't work:
> 
>> +  sprintf_vma (addr_buf, location->address);
>> +  sprintf (rs->buf, "QTEnable:%x:%s", location->owner->number, addr_buf);
>> +  putpkt (rs->buf);
>> +  remote_get_noisy_reply (&rs->buf, &rs->buf_size);
>> +  if (*rs->buf == '\0')
>> +    error (_("Target does not support enabling tracepoints while a trace run is ongoing."));
>> +  if (strcmp (rs->buf, "OK") != 0)
>> +    error (_("Error on target while enabling tracepoint."));
>

It won't work if the target does not have support for it (i.e. the target was
not compiled with this patch) or if an unexpected error occurs (in which case
all bets are off). Would something like this be better?

 Disable tracepoint @var{num}, or all tracepoints if no argument
 @var{num} is given.  A disabled tracepoint will have no effect during
-the next trace experiment, but it is not forgotten.  You can re-enable
+a trace experiment, but it is not forgotten.  You can re-enable
 a disabled tracepoint using the @code{enable tracepoint} command.
+If the command is issued during a trace experiment and the debug target
+has support for disabling tracepoints during a trace experiment, then the
+change will be effective immediately.  Otherwise, it will be applied to the
+next trace experiment.

-Enable tracepoint @var{num}, or all tracepoints.  The enabled
-tracepoints will become effective the next time a trace experiment is
-run.
+Enable tracepoint @var{num}, or all tracepoints.  If this command is
+issued during a trace experiment and the debug target supports enabling
+tracepoints during a trace experiment, then the enabled tracepoints will
+become effective immediately.  Otherwise, they will become effective the
+next time a trace experiment is run.

Kwok