linux-kernel - Re: [PATCH v2] perf sched timehist: Fixed timestamp error when unable to confirm event sched

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM9d7cjvx2jozQNiSj+Anf1+hsyL7DT17XJWv8oDJUr_4Ud=hg@mail.gmail.com>
Date: Mon, 19 Aug 2024 13:08:57 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Yang Jihong <yangjihong@...edance.com>
Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org, 
	mark.rutland@....com, alexander.shishkin@...ux.intel.com, jolsa@...nel.org, 
	irogers@...gle.com, adrian.hunter@...el.com, kan.liang@...ux.intel.com, 
	james.clark@....com, linux-perf-users@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] perf sched timehist: Fixed timestamp error when unable
 to confirm event sched_in time

Hello,

On Sun, Aug 18, 2024 at 7:47 PM Yang Jihong <yangjihong@...edance.com> wrote:
>
> If sched_in event for current task is not recorded, sched_in timestamp
> will be set to end_time of time window interest, causing an error in
> timestamp show. In this case, we choose to ignore this event.
>
> Test scenario:
>   perf[1229608] does not record the first sched_in event, run time and sch delay are both 0
>
>   # perf sched timehist
>   Samples of sched_switch event do not have callchains.
>              time    cpu  task name                       wait time  sch delay   run time
>                           [tid/pid]                          (msec)     (msec)     (msec)
>   --------------- ------  ------------------------------  ---------  ---------  ---------
>    2090450.763231 [0000]  perf[1229608]                       0.000      0.000      0.000
>    2090450.763235 [0000]  migration/0[15]                     0.000      0.001      0.003
>    2090450.763263 [0001]  perf[1229608]                       0.000      0.000      0.000
>    2090450.763268 [0001]  migration/1[21]                     0.000      0.001      0.004
>    2090450.763302 [0002]  perf[1229608]                       0.000      0.000      0.000
>    2090450.763309 [0002]  migration/2[27]                     0.000      0.001      0.007
>    2090450.763338 [0003]  perf[1229608]                       0.000      0.000      0.000
>    2090450.763343 [0003]  migration/3[33]                     0.000      0.001      0.004
>
> Before:
>   arbitrarily specify a time window of interest, timestamp will be set to an incorrect value
>
>   # perf sched timehist --time 100,200
>   Samples of sched_switch event do not have callchains.
>              time    cpu  task name                       wait time  sch delay   run time
>                           [tid/pid]                          (msec)     (msec)     (msec)
>   --------------- ------  ------------------------------  ---------  ---------  ---------
>        200.000000 [0000]  perf[1229608]                       0.000      0.000      0.000
>        200.000000 [0001]  perf[1229608]                       0.000      0.000      0.000
>        200.000000 [0002]  perf[1229608]                       0.000      0.000      0.000
>        200.000000 [0003]  perf[1229608]                       0.000      0.000      0.000
>        200.000000 [0004]  perf[1229608]                       0.000      0.000      0.000
>        200.000000 [0005]  perf[1229608]                       0.000      0.000      0.000
>        200.000000 [0006]  perf[1229608]                       0.000      0.000      0.000
>        200.000000 [0007]  perf[1229608]                       0.000      0.000      0.000
>
>  After:
>
>   # perf sched timehist --time 100,200
>   Samples of sched_switch event do not have callchains.
>              time    cpu  task name                       wait time  sch delay   run time
>                           [tid/pid]                          (msec)     (msec)     (msec)
>   --------------- ------  ------------------------------  ---------  ---------  ---------
>
> Fixes: 853b74071110 ("perf sched timehist: Add option to specify time window of interest")
> Signed-off-by: Yang Jihong <yangjihong@...edance.com>

Acked-by: Namhyung Kim <namhyung@...nel.org>

Thanks,
Namhyung

> ---
>
> Changes since v1:
>  - Simplify conditional judgment. (suggested-by Namhyung)
>
>  tools/perf/builtin-sched.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 9c1276dc8ef6..1c386ebe4b98 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2683,9 +2683,12 @@ static int timehist_sched_change_event(const struct perf_tool *tool,
>          * - previous sched event is out of window - we are done
>          * - sample time is beyond window user cares about - reset it
>          *   to close out stats for time window interest
> +        * - If tprev is 0, that is, sched_in event for current task is
> +        *   not recorded, cannot determine whether sched_in event is
> +        *   within time window interest - ignore it
>          */
>         if (ptime->end) {
> -               if (tprev > ptime->end)
> +               if (!tprev || tprev > ptime->end)
>                         goto out;
>
>                 if (t > ptime->end)
> --
> 2.25.1
>