Message-ID: <916bb20962e4c0e480539d6ec80a25d67c30a4ae.camel@kernel.org>
Date: Mon, 15 Sep 2025 10:09:35 -0500
From: Tom Zanussi <zanussi@...nel.org>
To: Bagas Sanjaya <bagasdotme@...il.com>, Linux Kernel Mailing List
 <linux-kernel@...r.kernel.org>, Linux Documentation
 <linux-doc@...r.kernel.org>,  Linux Kernel Tracing
 <linux-trace-kernel@...r.kernel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu
 <mhiramat@...nel.org>,  Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH 2/5] Documentation: trace: histogram-design: Trim
 trailing vertices in diagram explanation text

On Thu, 2025-09-11 at 11:25 +0700, Bagas Sanjaya wrote:
> Diagram explanation text is supposed to be interleaved commentary
> between diagram parts that are spread out, but it outputs ugly in
> htmldocs due to trailing vertices as if both the explanation and the
> diagram are in the same literal code block.
> 
> Trim trailing vertices.

Yes, this is much better, and the lines are still followable in the
text version. Thanks,

Reviewed-by: Tom Zanussi <zanussi@...nel.org>

> 
> Signed-off-by: Bagas Sanjaya <bagasdotme@...il.com>
> ---
>  Documentation/trace/histogram-design.rst | 138 +++++++++++----------
> --
>  1 file changed, 69 insertions(+), 69 deletions(-)
> 
> diff --git a/Documentation/trace/histogram-design.rst
> b/Documentation/trace/histogram-design.rst
> index 5765eb3e9efa78..231a12bd7d461c 100644
> --- a/Documentation/trace/histogram-design.rst
> +++ b/Documentation/trace/histogram-design.rst
> @@ -142,30 +142,30 @@ elements for a couple hypothetical keys and
> values.::
>                               +--------------
> +                            |  |
>                                              n_keys = n_fields -
> n_vals   |  |
>  
> -The hist_data n_vals and n_fields delineate the extent of the
> fields[]   |  |
> -array and separate keys from values for the rest of the
> code.            |  |
> +The hist_data n_vals and n_fields delineate the extent of the
> fields[]
> +array and separate keys from values for the rest of the code.
>  
> -Below is a run-time representation of the tracing_map part of
> the        |  |
> -histogram, with pointers from various parts of the fields[]
> array        |  |
> -to corresponding parts of the
> tracing_map.                               |  |
> +Below is a run-time representation of the tracing_map part of the
> +histogram, with pointers from various parts of the fields[] array
> +to corresponding parts of the tracing_map.
>  
> -The tracing_map consists of an array of tracing_map_entrys and a
> set     |  |
> -of preallocated tracing_map_elts (abbreviated below as map_entry
> and     |  |
> -map_elt).  The total number of map_entrys in the hist_data.map array
> =   |  |
> -map->max_elts (actually map->map_size but only max_elts of those
> are     |  |
> -used.  This is a property required by the map_insert()
> algorithm).       |  |
> +The tracing_map consists of an array of tracing_map_entrys and a set
> +of preallocated tracing_map_elts (abbreviated below as map_entry and
> +map_elt).  The total number of map_entrys in the hist_data.map array
> =
> +map->max_elts (actually map->map_size but only max_elts of those are
> +used.  This is a property required by the map_insert() algorithm).
>  
> -If a map_entry is unused, meaning no key has yet hashed into it,
> its     |  |
> -.key value is 0 and its .val pointer is NULL.  Once a map_entry
> has      |  |
> -been claimed, the .key value contains the key's hash value and
> the       |  |
> -.val member points to a map_elt containing the full key and an
> entry     |  |
> -for each key or value in the map_elt.fields[] array.  There is
> an        |  |
> -entry in the map_elt.fields[] array corresponding to each
> hist_field     |  |
> -in the histogram, and this is where the continually aggregated
> sums      |  |
> -corresponding to each histogram value are
> kept.                          |  |
> +If a map_entry is unused, meaning no key has yet hashed into it, its
> +.key value is 0 and its .val pointer is NULL.  Once a map_entry has
> +been claimed, the .key value contains the key's hash value and the
> +.val member points to a map_elt containing the full key and an entry
> +for each key or value in the map_elt.fields[] array.  There is an
> +entry in the map_elt.fields[] array corresponding to each hist_field
> +in the histogram, and this is where the continually aggregated sums
> +corresponding to each histogram value are kept.
>  
> -The diagram attempts to show the relationship between
> the                |  |
> -hist_data.fields[] and the map_elt.fields[] with the links
> drawn         |  |
> +The diagram attempts to show the relationship between the
> +hist_data.fields[] and the map_elt.fields[] with the links drawn
>  between diagrams::
>  
>    +-----------
> +		                                                 |  |
> @@ -440,31 +440,31 @@ sched_waking histogram
>                                               n_keys = n_fields -
> n_vals   | | |
>                                                                      
>       | | |
>  
> -This is very similar to the basic case.  In the above diagram, we
> can     | | |
> -see a new .flags member has been added to the struct
> hist_field           | | |
> -struct, and a new entry added to hist_data.fields representing the
> ts0    | | |
> -variable.  For a normal val hist_field, .flags is just 0
> (modulo          | | |
> -modifier flags), but if the value is defined as a variable, the
> .flags    | | |
> -contains a set FL_VAR
> bit.                                                | | |
> +This is very similar to the basic case.  In the above diagram, we
> can
> +see a new .flags member has been added to the struct hist_field
> +struct, and a new entry added to hist_data.fields representing the
> ts0
> +variable.  For a normal val hist_field, .flags is just 0 (modulo
> +modifier flags), but if the value is defined as a variable, the
> .flags
> +contains a set FL_VAR bit.
>  
> -As you can see, the ts0 entry's .var.idx member contains the
> index        | | |
> -into the tracing_map_elts' .vars[] array containing variable
> values.      | | |
> -This idx is used whenever the value of the variable is set or
> read.       | | |
> -The map_elt.vars idx assigned to the given variable is assigned
> and       | | |
> -saved in .var.idx by create_tracing_map_fields() after it
> calls           | | |
> -
> tracing_map_add_var().                                                
>     | | |
> +As you can see, the ts0 entry's .var.idx member contains the index
> +into the tracing_map_elts' .vars[] array containing variable values.
> +This idx is used whenever the value of the variable is set or read.
> +The map_elt.vars idx assigned to the given variable is assigned and
> +saved in .var.idx by create_tracing_map_fields() after it calls
> +tracing_map_add_var().
>  
> -Below is a representation of the histogram at run-time,
> which             | | |
> -populates the map, along with correspondence to the above hist_data
> and   | | |
> -hist_field data
> structures.                                               | | |
> +Below is a representation of the histogram at run-time, which
> +populates the map, along with correspondence to the above hist_data
> and
> +hist_field data structures.
>  
> -The diagram attempts to show the relationship between
> the                 | | |
> -hist_data.fields[] and the map_elt.fields[] and map_elt.vars[]
> with       | | |
> -the links drawn between diagrams.  For each of the map_elts, you
> can      | | |
> -see that the .fields[] members point to the .sum or .offset of a
> key      | | |
> -or val and the .vars[] members point to the value of a variable. 
> The     | | |
> -arrows between the two diagrams show the linkages between
> those           | | |
> -tracing_map members and the field definitions in the
> corresponding        | | |
> +The diagram attempts to show the relationship between the
> +hist_data.fields[] and the map_elt.fields[] and map_elt.vars[] with
> +the links drawn between diagrams.  For each of the map_elts, you can
> +see that the .fields[] members point to the .sum or .offset of a key
> +or val and the .vars[] members point to the value of a variable. 
> The
> +arrows between the two diagrams show the linkages between those
> +tracing_map members and the field definitions in the corresponding
>  hist_data fields[] members.::
>  
>    +-----------
> +		                                                  | | |
> @@ -565,40 +565,40 @@ hist_data fields[] members.::
>                                                       
> |               |     | |
>                                                        +-------------
> --+     | |
>  
> -For each used map entry, there's a map_elt pointing to an array
> of          | |
> -.vars containing the current value of the variables associated
> with         | |
> -that histogram entry.  So in the above, the timestamp associated
> with       | |
> -pid 999 is 113345679876, and the timestamp variable in the
> same             | |
> -.var.idx for pid 4444 is
> 213499240729.                                      | |
> +For each used map entry, there's a map_elt pointing to an array of
> +.vars containing the current value of the variables associated with
> +that histogram entry.  So in the above, the timestamp associated
> with
> +pid 999 is 113345679876, and the timestamp variable in the same
> +.var.idx for pid 4444 is 213499240729.
>  
> -sched_switch
> histogram                                                      | |
> -----------------------
>                                                       | |
> +sched_switch histogram
> +----------------------
>  
> -The sched_switch histogram paired with the above
> sched_waking               | |
> -histogram is shown below.  The most important aspect of
> the                 | |
> -sched_switch histogram is that it references a variable on
> the              | |
> -sched_waking histogram
> above.                                               | |
> +The sched_switch histogram paired with the above sched_waking
> +histogram is shown below.  The most important aspect of the
> +sched_switch histogram is that it references a variable on the
> +sched_waking histogram above.
>  
> -The histogram diagram is very similar to the others so far
> displayed,       | |
> -but it adds variable references.  You can see the normal hitcount
> and       | |
> -key fields along with a new wakeup_lat variable implemented in
> the          | |
> -same way as the sched_waking ts0 variable, but in addition there's
> an       | |
> -entry with the new FL_VAR_REF (short for HIST_FIELD_FL_VAR_REF)
> flag.       | |
> +The histogram diagram is very similar to the others so far
> displayed,
> +but it adds variable references.  You can see the normal hitcount
> and
> +key fields along with a new wakeup_lat variable implemented in the
> +same way as the sched_waking ts0 variable, but in addition there's
> an
> +entry with the new FL_VAR_REF (short for HIST_FIELD_FL_VAR_REF)
> flag.
>  
> -Associated with the new var ref field are a couple of new
> hist_field        | |
> -members, var.hist_data and var_ref_idx.  For a variable reference,
> the      | |
> -var.hist_data goes with the var.idx, which together uniquely
> identify       | |
> -a particular variable on a particular histogram.  The var_ref_idx
> is        | |
> -just the index into the var_ref_vals[] array that caches the values
> of      | |
> -each variable whenever a hist trigger is updated.  Those
> resulting          | |
> -values are then finally accessed by other code such as trace
> action         | |
> -code that uses the var_ref_idx values to assign param
> values.               | |
> +Associated with the new var ref field are a couple of new hist_field
> +members, var.hist_data and var_ref_idx.  For a variable reference,
> the
> +var.hist_data goes with the var.idx, which together uniquely
> identify
> +a particular variable on a particular histogram.  The var_ref_idx is
> +just the index into the var_ref_vals[] array that caches the values
> of
> +each variable whenever a hist trigger is updated.  Those resulting
> +values are then finally accessed by other code such as trace action
> +code that uses the var_ref_idx values to assign param values.
>  
> -The diagram below describes the situation for the
> sched_switch              | |
> +The diagram below describes the situation for the sched_switch
>  histogram referred to before::
>  
> -  # echo 'hist:keys=next_pid:wakeup_lat=common_timestamp.usecs-$ts0'
> >>     | |
> -         
> events/sched/sched_switch/trigger                                 | |
> +  # echo 'hist:keys=next_pid:wakeup_lat=common_timestamp.usecs-$ts0'
> >>
> +          events/sched/sched_switch/trigger
>                                                                      
>         | |
>    +------------------
> +                                                      | |
>    | hist_data       
> |                                                      | |

Powered by blists - more mailing lists