Hi Alejandro,
Sorry for the late reply.
On Thu, Jan 19, 2023 at 5:41 AM Alejandro Colomar
<alx.manpages@xxxxxxxxx> wrote:
>
> Hi Namhyung,
>
> On 1/17/23 09:08, Namhyung Kim wrote:
> > From: Namhyung Kim <namhyung@xxxxxxxxxx>
> >
> > Add missing perf_event_attr fields, new event codes and sample type.
> > Also add descriptions for PERF_FORMAT_LOST.
> >
> > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
>
> Thanks! Just a minor question.
Sure.
>
> > ---
> > man2/perf_event_open.2 | 201 +++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 193 insertions(+), 8 deletions(-)
> >
> > diff --git a/man2/perf_event_open.2 b/man2/perf_event_open.2
> > index 8e94fb4ac..05f06cf68 100644
> > --- a/man2/perf_event_open.2
> > +++ b/man2/perf_event_open.2
> [...]
>
> > @@ -2704,6 +2881,14 @@ is the inode number.
> > .I ino_generation
> > is the inode generation.
> > .TP
> > +.I build_id_size
> > +is the actual size of
> > +.I build_id
> > +field (up to 20).
>
> Do we want to document this limit? Is it some contract that we want to offer to
> users? Or is it just an implementation detail that might change?
It's highly unlikely to change the max build-id size due to the
format of the event record. So I think it's good to document
the limit.
>
> > +.TP
> > +.I build_id
> > +is a raw data to identify a binary.
> > +.TP
> > .I prot
> > is the protection information.
> > .TP
>
> Apart from that, I'll apply the following cosmetic changes to your patch, if you
> agree.
Please go ahead.
Thanks,
Namhyung
> ---
>
> diff --git a/man2/perf_event_open.2 b/man2/perf_event_open.2
> index 05f06cf68..abfa59efa 100644
> --- a/man2/perf_event_open.2
> +++ b/man2/perf_event_open.2
> @@ -918,21 +918,22 @@ .SS Arguments
> Records hardware provided weight values like in
> .BR PERF_SAMPLE_WEIGHT ,
> but it can represent multiple values in a struct.
> -This shares the same space as the
> +This shares the same space as
> .BR PERF_SAMPLE_WEIGHT ,
> -so users can apply either of those, not the both.
> -When used, it would have the following format and
> -the meaning of each fields is dependent to the
> -hardware implementation.
> +so users can apply either of those,
> +not both.
> +It has the following format and
> +the meaning of each fields is
> +dependent on the hardware implementation.
> .PP
> .in +4n
> .EX
> union perf_sample_weight {
> - u64 full; /* PERF_SAMPLE_WEIGHT */
> + u64 full; /* PERF_SAMPLE_WEIGHT */
> struct { /* PERF_SAMPLE_WEIGHT_STRUCT */
> - u32 var1_dw;
> - u16 var2_w;
> - u16 var3_w;
> + u32 var1_dw;
> + u16 var2_w;
> + u16 var3_w;
> };
> };
> .EE
> @@ -1337,15 +1338,15 @@ .SS Arguments
> .IR inherit_thread " (since Linux 5.13)"
> .\" commit 2b26f0aa004995f49f7b6f4100dd0e4c39a9ed5f
> This disables the inheritance of the event to a child process.
> -Only new threads in the same process (which is cloned with
> +Only new threads in the same process
> +(which is cloned with
> .BR CLONE_THREAD )
> will inherit the event.
> .TP
> .IR remove_on_exec " (since Linux 5.13)"
> .\" commit 2e498d0a74e5b88a6689ae1b811f247f91ff188e
> This closes the event when it starts a new process image by
> -.BR execve (2)
> -system call.
> +.BR execve (2).
> .TP
> .IR sigtrap " (since Linux 5.13)"
> .\" commit 97ba62b278674293762c3d91f724f1bb922f04e0
> @@ -1568,12 +1569,14 @@ .SS Arguments
> .\" commit a4faf00d994c40e64f656805ac375c65e324eefb
> When
> .B PERF_SAMPLE_AUX
> -flag is set, specify the desired size of AUX data.
> +flag is set,
> +specify the desired size of AUX data.
> Note that it can get smaller data than the specified size.
> .TP
> .IR sig_data " (since Linux 5.13)"
> .\" commit 97ba62b278674293762c3d91f724f1bb922f04e0
> -This data will be copied to user's signal handler (through
> +This data will be copied to user's signal handler
> +(through
> .I si_perf
> in the
> .IR siginfo_t )
> @@ -1662,7 +1665,8 @@ .SS Reading results
> .IR read_format .
> .TP
> .I lost
> -The number of lost samples of this event; only present if
> +The number of lost samples of this event;
> +only present if
> .B PERF_FORMAT_LOST
> was specified in
> .IR read_format .
> @@ -2123,8 +2127,8 @@ .SS MMAP layout
> .B PERF_RECORD_SWITCH
> or
> .B PERF_RECORD_SWITCH_CPU_WIDE
> -record is generated, this indicates the context switch
> -was a preemption.
> +record is generated,
> +this indicates the context switch was a preemption.
> .TP
> .BR PERF_RECORD_MISC_MMAP_BUILD_ID " (since Linux 5.12)"
> .\" commit 88a16a1309333e43d328621ece3e9fa37027e8eb
> @@ -2804,10 +2808,11 @@ .SS MMAP layout
> .TP
> .I size
> .TQ
> -.IR data [size]
> +.IR data [ size ]
> If
> .B PERF_SAMPLE_AUX
> -is enabled, then a snapshot of the aux buffer is recorded.
> +is enabled,
> +a snapshot of the aux buffer is recorded.
> .RE
> .TP
> .B PERF_RECORD_MMAP2
> @@ -2820,7 +2825,8 @@ .SS MMAP layout
> shared mappings.
> Depending on the
> .B PERF_RECORD_MISC_MMAP_BUILD_ID
> -bit in the header, the extra values have different layout and meanings.
> +bit in the header,
> +the extra values have different layout and meanings.
> .IP
> .in +4n
> .EX
>
>
> --
> <http://www.alejandro-colomar.es/>
