linux-kernel - Re: [PATCH] cyclictest: Set tracing

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: <520f0cf11001270506ge76cca1ha259539a101522c2@mail.gmail.com>
Date:	Wed, 27 Jan 2010 14:06:51 +0100
From:	John Kacur <jkacur@...hat.com>
To:	yi li <liyi.dev@...il.com>
Cc:	williams@...hat.com, tglx@...utronix.de,
	LKML <linux-kernel@...r.kernel.org>,
	linux-rt-users@...r.kernel.org
Subject: Re: [PATCH] cyclictest: Set tracing_thresh optionally

On Wed, Jan 27, 2010 at 12:18 PM, yi li <liyi.dev@...il.com> wrote:
> Hi,
>
> In cyclictest, by default "tracing_thresh" will be set to "tracelimit"
> (given to cyclictest with option "-b tracelimit").
> If the maximum latency of the tracer currently used is less than
> "tracelimit", no trace will be recorded.

So, can't you just set the tracing_thresh to your desired value with the
-b tracelimit option?

Explain to me how cyclictest will behave if tracelimit and tracing_thresh
are set to different values.

If you showed us two sets of outputs (with and without your option)
to show what setting tracing_thresh separately would achieve that
would be helpful.

>
> e.g:
> ./cyclictest -p 80 -t 1 -n -l 10000 -i 10000 -b 8000 -I -f
> If the "irqsoff" maximum latency is less than 8000,
> "/sys/kernel/debug/tracing/trace" will no contain valid "irqsoff"
> trace.
>
> This is not expected sometimes if e.g, the maximum latency of
> cyclictest is caused by wakeup, but I want to see the irqsoff latency.
> So I think it may be better to allow user set "tracing_thresh"
> optionally, by adding a "--tracing_thresh=THRESH" option.
>
> Here is a simple patch to show the idea:
>
> Signed-off-by: yi.li@...log.com
>
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index ce4d911..c2f1c07 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -144,6 +144,7 @@ struct thread_stat {
>
>  static int shutdown;
>  static int tracelimit = 0;
> +static int tracing_thresh = 0;
>  static int ftrace = 0;
>  static int kernelversion;
>  static int verbose = 0;
> @@ -384,9 +385,11 @@ static void setup_tracer(void)
>        if (kernelversion == KV_26_CURR) {
>                char buffer[32];
>                int ret;
> -
> -               sprintf(buffer, "%d", tracelimit);
> -               setkernvar("tracing_thresh", buffer);
> +
> +               if (tracing_thresh > 0 && tracing_thresh <= tracelimit) {
> +                       sprintf(buffer, "%d", tracing_thresh);
> +                       setkernvar("tracing_thresh", buffer);
> +               }

Do you need an else part that sets tracing_thresh to tracelimit?


>
>                /* ftrace_enabled is a sysctl variable */
>                fileprefix = procfileprefix;
> @@ -766,6 +769,7 @@ static void display_help(int error)
>               "                           without -t default = 1\n"
>               "-T TRACE --tracer=TRACER   set tracing function\n"
>               "    configured tracers: %s\n"
> +              "--tracing_thresh=THRESH    set tracing_thresh of current
> tracer to THRESH\n"

If you run your patch through checkpatch you'll get

ERROR: patch seems to be corrupt (line wrapped?)
#64: FILE: src/cyclictest/cyclictest.c:772:
tracer to THRESH\n"

And because of the above, I am unable to apply your patch
with git am

We don't follow all the kernel style recommendations in cyclictest
but we try to follow most of them. So please run your patch
through checkpatch and fix-up the warnings.

>               "-u       --unbuffered      force unbuffered output for live
> processing\n"
>               "-v       --verbose         output values on stdout for statistics\n"
>               "                           format: n:c:v n=tasknum c=count
> v=value in us\n"
> @@ -896,6 +900,7 @@ static void process_options (int argc, char *argv[])
>                        {"help", no_argument, NULL, '?'},
>                        {"tracer", required_argument, NULL, 'T'},
>                        {"traceopt", required_argument, NULL, 'O'},
> +                       {"tracing_thresh", required_argument, NULL, 1},
>                        {"smp", no_argument, NULL, 'S'},
>                        {NULL, 0, NULL, 0}
>                };
> @@ -972,6 +977,7 @@ static void process_options (int argc, char *argv[])
>                        use_nanosleep = MODE_CLOCK_NANOSLEEP;
>                        break;
>                case '?': display_help(0); break;
> +               case 1 : tracing_thresh = atoi(optarg); break;
>                }
>        }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/