linux-kernel - Re: [PATCH 2/4] preemptirq_delay_test: Add the burst feature and a sysfs trigger

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] [day] [month] [year] [list]

Message-ID: <6b0bf71d9c4cf1f16f70d0aeb18a3b3448010025.camel@bmw.de>
Date:   Wed, 23 Jan 2019 09:22:09 +0000
From:   <Viktor.Rosendahl@....de>
To:     <joel@...lfernandes.org>
CC:     <linux-kernel@...r.kernel.org>, <mingo@...hat.com>,
        <rostedt@...dmis.org>
Subject: Re: [PATCH 2/4] preemptirq_delay_test: Add the burst feature and a
 sysfs trigger

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