From: <Viktor.Rosendahl@bmw.de>
To: <joel@joelfernandes.org>
Cc: <linux-kernel@vger.kernel.org>, <mingo@redhat.com>,
<rostedt@goodmis.org>
Subject: Re: [PATCH 2/4] preemptirq_delay_test: Add the burst feature and a sysfs trigger
Date: Wed, 23 Jan 2019 09:22:09 +0000 [thread overview]
Message-ID: <6b0bf71d9c4cf1f16f70d0aeb18a3b3448010025.camel@bmw.de> (raw)
In-Reply-To: <20190122215311.GA200493@google.com>
Hi Joel,
On Tue, 2019-01-22 at 16:53 -0500, Joel Fernandes wrote:
> Could you CC me on the other patches as well, next time? I am quite
> interested and recently have worked on the latency tracer.
>
Sure, I will.
>
> > +MODULE_PARM_DESC(burst_size, "The size of a burst (default 1)");
>
> Where are we bounds checking the burst_size here? It seems like a high
> burst_size can overflow your array of functions.
>
I don't think so. I use "i % NR_TEST_FUNCS" as index when I call functions in
the array.
Basically, if the user specifies a burst size larger than 10, then we will begin
to reuse the test functions.
> >
> > +DECLARE_TESTFN(9)
>
> You really only need 2 functions here, since the odd and even suffixed
> functions are identical.
>
This would indeed make the code more neat and compact.
However, it would no longer be a good test for the latency-collector, which I
posted here:
https://lkml.org/lkml/2019/1/21/572
If we want to test the latency-collector properly, we need backtraces that look
different from each other, otherwise, there is no way of knowing whether the
latency-collector captured the first latency, the last latency or one somewhere
in the middle.
With my code we see backtraces like this:
=> preemptirqtest_4
=> preemptirq_delay_run
=> kthread
=> ret_from_fork
This tells us that the latency-collector captured the 5th latency in a burst. I
want to be able to see that, yes the latency-collector can fish out the 5th
latency in a burst of 10.
Having 10 testfunctions and then reusing them with the modulo game is an attempt
at a compromise between having an inifinte number of testfunctions and compact
code. An alternative would be to check that the burst_size parameter is not
greater than the number of functions.
> >
>
> I honestly feel a sysfs trigger file is pointless. Why can't the module be
> reloaded?
It is just a convenenince for lazy people who manually want to repeat the same
test without having to write a shell script with modprobe & rmmod. Or perhaps
for those who are worried about spending CPU cycles on module loading :)
> Note also that module parameters can be changed after the module
> has been loaded. Perhaps that can be used as a trigger?
I was not aware of this possibility. I can try to make it so if it's desired.
> So if the test_mode
> is changed, then the test is re-run.
>
> However, if Steve prefers the sysfs trigger file, then I am Ok with that.
>
>
I would still prefer to keep the sysfs trigger but I don't insist on it.
When testing the latency-collector it's often desired to repeat the exact same
test many times.
Thanks for the comments.
best regards,
Viktor
next prev parent reply other threads:[~2019-01-23 9:32 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-21 12:35 [PATCH 0/4] Some new features for the preempt/irqsoff tracers Viktor Rosendahl
2019-01-21 12:35 ` [PATCH 1/4] ftrace: Implement fs notification for " Viktor Rosendahl
2019-01-21 12:35 ` [PATCH 2/4] preemptirq_delay_test: Add the burst feature and a sysfs trigger Viktor Rosendahl
2019-01-22 21:53 ` Joel Fernandes
2019-01-23 9:22 ` Viktor.Rosendahl [this message]
2019-01-21 12:35 ` [PATCH 3/4] Add the latency-collector to tools Viktor Rosendahl
2019-01-21 12:35 ` [PATCH 4/4] ftrace: Add an option for tracing console latencies Viktor Rosendahl
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=6b0bf71d9c4cf1f16f70d0aeb18a3b3448010025.camel@bmw.de \
--to=viktor.rosendahl@bmw.de \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--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).