Re: [PATCH 05/12] record-btrace: add bts buffer size configuration option

[Eli Zaretskii <eliz at gnu dot org>]

> From: Markus Metzger <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org, Eli Zaretskii <eliz@gnu.org>
> Date: Mon, 14 Jul 2014 15:56:29 +0200
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -9,11 +9,27 @@ record btrace bts
>  record bts
>    Start branch trace recording using Intel(R) Branch Trace Store format.
>  
> +* New options
> +
> +set|show record btrace bts buffer-size
> +  Set and show the size of the ring buffer used for branch tracing in
> +  BTS format.
> +  The obtained size may differ from the requested size.  Use "info
> +  record" to see the obtained buffer size.

This hints that the "show" command only shows the requested size, is
that right?  If so, I suggest to tell that explcitly, or maybe delete
the last 2 sentences from NEWS and leave them for the manual.

> +@kindex set record btrace bts
> +@item set record btrace bts buffer-size @var{size}
> +@itemx set record btrace bts buffer-size unlimited
> +Set the requested ring buffer size for branch tracing in BTS format.
> +Default is 64KB.

If SIZE is optional, it should be in brackets [...].

> +If @var{size} is a positive number, then @value{GDBN} will try to
> +allocate a buffer of at least @var{size} bytes for each new thread
> +that uses the btrace recording method and the BTS format.  The actually

By "thread" you mean threads in the inferior, is that right?  If so,
"uses" is not really accurate, since it's not the thread that is
recording the branch trace, is it?

> +If @var{limit} is @code{unlimited} or zero, @value{GDBN} will try to
> +allocate a buffer of 4MB.

I wonder whether we should advertise the size of "unlimited" here.
What if we would like to change it, or if some architecture requires a
different maximum?

I also wonder what kind of "unlimited" is it.  In general, when I ask
for "unlimited", I expect the value to be limited only by the amount
of available virtual memory; 4MB sounds too few to fit my intuitive
notion of "unlimited".  What are the reason for such a small upper
bound?

> +@item Qbtrace-conf:bts:size=@var{value}
> +Set the requested ring buffer size for new threads that use the
> +btrace recording method in BTS format.
> +
> +Reply:
> +@table @samp
> +@item OK
> +The ring buffer size has been set.
> +@item E.errtext

"errtext" is not literal text, it stands for some meaningful error
message, right?  If so, please use @var{errtext}.

> @@ -38688,7 +38747,16 @@ configuration using the @samp{qXfer:btrace-conf:read}
>  (@pxref{qXfer btrace-conf read}) packet.
>  
>  The configuration describes the branch trace format and configuration
> -settings for that format.
> +settings for that format.  The following information is described:
> +
> +@table @code
> +@item bts
> +This thread uses the Branch Trace Store (BTS) format.

Please add a cross-reference here to the section which describes BTS.

The documentation parts are OK with these fixed.