linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tom Zanussi <zanussi@kernel.org>
To: Bagas Sanjaya <bagasdotme@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Documentation <linux-doc@vger.kernel.org>,
	 Linux Kernel Tracing <linux-trace-kernel@vger.kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Jonathan Corbet <corbet@lwn.net>
Subject: Re: [PATCH 2/5] Documentation: trace: histogram-design: Trim trailing vertices in diagram explanation text
Date: Mon, 15 Sep 2025 10:09:35 -0500	[thread overview]
Message-ID: <916bb20962e4c0e480539d6ec80a25d67c30a4ae.camel@kernel.org> (raw)
In-Reply-To: <20250911042527.22573-3-bagasdotme@gmail.com>

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@kernel.org>

> 
> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.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       
> |                                                      | |


  parent reply	other threads:[~2025-09-15 15:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-11  4:25 [PATCH 0/5] histogram docs formatting cleanup Bagas Sanjaya
2025-09-11  4:25 ` [PATCH 1/5] Documentation: trace: histogram: Fix histogram trigger subsection number order Bagas Sanjaya
2025-09-12  0:59   ` Masami Hiramatsu
2025-09-15 15:07   ` Tom Zanussi
2025-09-11  4:25 ` [PATCH 2/5] Documentation: trace: histogram-design: Trim trailing vertices in diagram explanation text Bagas Sanjaya
2025-09-12  1:01   ` Masami Hiramatsu
2025-09-15 15:09   ` Tom Zanussi [this message]
2025-09-11  4:25 ` [PATCH 3/5] Documentation: trace: historgram-design: Separate sched_waking histogram section heading and the following diagram Bagas Sanjaya
2025-09-12  1:03   ` Masami Hiramatsu
2025-09-15 15:10   ` Tom Zanussi
2025-09-11  4:25 ` [PATCH 4/5] Documentation: trace: histogram-design: Wrap introductory note in note:: directive Bagas Sanjaya
2025-09-12  1:03   ` Masami Hiramatsu
2025-09-15 15:11   ` Tom Zanussi
2025-09-11  4:25 ` [PATCH 5/5] Documentation: trace: histogram: Link to ftrace docs Bagas Sanjaya
2025-09-12  0:58   ` Masami Hiramatsu
2025-09-12  2:55     ` Bagas Sanjaya
2025-09-15 15:11   ` Tom Zanussi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=916bb20962e4c0e480539d6ec80a25d67c30a4ae.camel@kernel.org \
    --to=zanussi@kernel.org \
    --cc=bagasdotme@gmail.com \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).