diff mbox series

[1/2] selftests/kmod: increase the kmod timeout from 45 to 165

Message ID 20230206234344.2433950-2-mcgrof@kernel.org (mailing list archive)
State New, archived
Headers show
Series selftests: bump timeout for firmware and kmod | expand

Commit Message

Luis Chamberlain Feb. 6, 2023, 11:43 p.m. UTC
The default sefltests timeout is 45 seconds. If you run the kmod
selftests on your own with say:

./tools/testings/selftests/kmod.sh

Then the default timeout won't be in effect.

I've never ran kmod selftests using the generic make wrapper
(./tools/testing/selftests/run_kselftest.sh -s) util now
that I have support for it on kdevops [0]. And with that the
test is limitted to the default timeout which we quickly run
into. Bump this up to what I see is required on 8GiB / 8 vcpu
libvirt q35 guest as can be easily created now with kdevops.

To run selftests with kdevops:

make menuconfig # enable dedicated selftests and kmod test
make
make bringup
make linux
make selftests-kmod

This ends up taking about 280 seconds now, give or take add
50 seconds more more and we end up with 350. Document the
rationale.

[0] https://github.com/linux-kdevops/kdevops
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 tools/testing/selftests/kmod/settings | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 tools/testing/selftests/kmod/settings

Comments

Shuah Khan Feb. 27, 2023, 10:32 p.m. UTC | #1
On 2/6/23 16:43, Luis Chamberlain wrote:
> The default sefltests timeout is 45 seconds. If you run the kmod
> selftests on your own with say:
> 
> ./tools/testings/selftests/kmod.sh
> 
> Then the default timeout won't be in effect.
> 
> I've never ran kmod selftests using the generic make wrapper
> (./tools/testing/selftests/run_kselftest.sh -s) util now
> that I have support for it on kdevops [0]. And with that the
> test is limitted to the default timeout which we quickly run
> into. Bump this up to what I see is required on 8GiB / 8 vcpu
> libvirt q35 guest as can be easily created now with kdevops.
> 
> To run selftests with kdevops:
> 
> make menuconfig # enable dedicated selftests and kmod test
> make
> make bringup
> make linux
> make selftests-kmod
> 
> This ends up taking about 280 seconds now, give or take add
> 50 seconds more more and we end up with 350. Document the
> rationale.
> 
> [0] https://github.com/linux-kdevops/kdevops
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>   tools/testing/selftests/kmod/settings | 4 ++++
>   1 file changed, 4 insertions(+)
>   create mode 100644 tools/testing/selftests/kmod/settings
> 
> diff --git a/tools/testing/selftests/kmod/settings b/tools/testing/selftests/kmod/settings
> new file mode 100644
> index 000000000000..6fca0f1a4594
> --- /dev/null
> +++ b/tools/testing/selftests/kmod/settings
> @@ -0,0 +1,4 @@
> +# measured from a manual run:
> +# time ./tools/testing/selftests/kmod/kmod.sh
> +# Then add ~50 seconds more gracetime.
> +timeout=350

Adding timeouts like this for individual tests increases the overall kselftest
run-time. I am not in favor of adding timeouts.

We have to find a better way to do this.

thanks,
-- Shuah
Luis Chamberlain Feb. 27, 2023, 10:42 p.m. UTC | #2
On Mon, Feb 27, 2023 at 03:32:50PM -0700, Shuah Khan wrote:
> On 2/6/23 16:43, Luis Chamberlain wrote:
> > The default sefltests timeout is 45 seconds. If you run the kmod
> > selftests on your own with say:
> > 
> > ./tools/testings/selftests/kmod.sh
> > 
> > Then the default timeout won't be in effect.
> > 
> > I've never ran kmod selftests using the generic make wrapper
> > (./tools/testing/selftests/run_kselftest.sh -s) util now
> > that I have support for it on kdevops [0]. And with that the
> > test is limitted to the default timeout which we quickly run
> > into. Bump this up to what I see is required on 8GiB / 8 vcpu
> > libvirt q35 guest as can be easily created now with kdevops.
> > 
> > To run selftests with kdevops:
> > 
> > make menuconfig # enable dedicated selftests and kmod test
> > make
> > make bringup
> > make linux
> > make selftests-kmod
> > 
> > This ends up taking about 280 seconds now, give or take add
> > 50 seconds more more and we end up with 350. Document the
> > rationale.
> > 
> > [0] https://github.com/linux-kdevops/kdevops
> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > ---
> >   tools/testing/selftests/kmod/settings | 4 ++++
> >   1 file changed, 4 insertions(+)
> >   create mode 100644 tools/testing/selftests/kmod/settings
> > 
> > diff --git a/tools/testing/selftests/kmod/settings b/tools/testing/selftests/kmod/settings
> > new file mode 100644
> > index 000000000000..6fca0f1a4594
> > --- /dev/null
> > +++ b/tools/testing/selftests/kmod/settings
> > @@ -0,0 +1,4 @@
> > +# measured from a manual run:
> > +# time ./tools/testing/selftests/kmod/kmod.sh
> > +# Then add ~50 seconds more gracetime.
> > +timeout=350
> 
> Adding timeouts like this for individual tests increases the overall kselftest
> run-time. I am not in favor of adding timeouts.
> 
> We have to find a better way to do this.

Well if folks don't have this the test will fail, and so a false
positive. If the goal is to have a low time timeout for "do not run
tests past this time and do not fail if we stopped the test" then
that seems to be likely one way to go and each test may need to be
modified to not fail fatally in case of a special signal.

  Luis
Shuah Khan March 3, 2023, 8:35 p.m. UTC | #3
On 2/27/23 15:42, Luis Chamberlain wrote:
> On Mon, Feb 27, 2023 at 03:32:50PM -0700, Shuah Khan wrote:
>> On 2/6/23 16:43, Luis Chamberlain wrote:
>>> The default sefltests timeout is 45 seconds. If you run the kmod
>>> selftests on your own with say:
>>>
>>> ./tools/testings/selftests/kmod.sh
>>>
>>> Then the default timeout won't be in effect.
>>>
>>> I've never ran kmod selftests using the generic make wrapper
>>> (./tools/testing/selftests/run_kselftest.sh -s) util now
>>> that I have support for it on kdevops [0]. And with that the
>>> test is limitted to the default timeout which we quickly run
>>> into. Bump this up to what I see is required on 8GiB / 8 vcpu
>>> libvirt q35 guest as can be easily created now with kdevops.
>>>
>>> To run selftests with kdevops:
>>>
>>> make menuconfig # enable dedicated selftests and kmod test
>>> make
>>> make bringup
>>> make linux
>>> make selftests-kmod
>>>
>>> This ends up taking about 280 seconds now, give or take add
>>> 50 seconds more more and we end up with 350. Document the
>>> rationale.
>>>
>>> [0] https://github.com/linux-kdevops/kdevops
>>> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
>>> ---
>>>    tools/testing/selftests/kmod/settings | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>    create mode 100644 tools/testing/selftests/kmod/settings
>>>
>>> diff --git a/tools/testing/selftests/kmod/settings b/tools/testing/selftests/kmod/settings
>>> new file mode 100644
>>> index 000000000000..6fca0f1a4594
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/kmod/settings
>>> @@ -0,0 +1,4 @@
>>> +# measured from a manual run:
>>> +# time ./tools/testing/selftests/kmod/kmod.sh
>>> +# Then add ~50 seconds more gracetime.
>>> +timeout=350
>>
>> Adding timeouts like this for individual tests increases the overall kselftest
>> run-time. I am not in favor of adding timeouts.
>>
>> We have to find a better way to do this.
> 
> Well if folks don't have this the test will fail, and so a false
> positive. If the goal is to have a low time timeout for "do not run
> tests past this time and do not fail if we stopped the test" then
> that seems to be likely one way to go and each test may need to be
> modified to not fail fatally in case of a special signal.
> 

We are finding more and more that timeout values are requiring
tweaks. I am in favor of coming up a way to exit the test with
a timeout condition.

thanks,
-- Shuah
Luis Chamberlain March 3, 2023, 9:48 p.m. UTC | #4
On Fri, Mar 03, 2023 at 01:35:10PM -0700, Shuah Khan wrote:
> On 2/27/23 15:42, Luis Chamberlain wrote:
> > On Mon, Feb 27, 2023 at 03:32:50PM -0700, Shuah Khan wrote:
> > > On 2/6/23 16:43, Luis Chamberlain wrote:
> > > > The default sefltests timeout is 45 seconds. If you run the kmod
> > > > selftests on your own with say:
> > > > 
> > > > ./tools/testings/selftests/kmod.sh
> > > > 
> > > > Then the default timeout won't be in effect.
> > > > 
> > > > I've never ran kmod selftests using the generic make wrapper
> > > > (./tools/testing/selftests/run_kselftest.sh -s) util now
> > > > that I have support for it on kdevops [0]. And with that the
> > > > test is limitted to the default timeout which we quickly run
> > > > into. Bump this up to what I see is required on 8GiB / 8 vcpu
> > > > libvirt q35 guest as can be easily created now with kdevops.
> > > > 
> > > > To run selftests with kdevops:
> > > > 
> > > > make menuconfig # enable dedicated selftests and kmod test
> > > > make
> > > > make bringup
> > > > make linux
> > > > make selftests-kmod
> > > > 
> > > > This ends up taking about 280 seconds now, give or take add
> > > > 50 seconds more more and we end up with 350. Document the
> > > > rationale.
> > > > 
> > > > [0] https://github.com/linux-kdevops/kdevops
> > > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > > > ---
> > > >    tools/testing/selftests/kmod/settings | 4 ++++
> > > >    1 file changed, 4 insertions(+)
> > > >    create mode 100644 tools/testing/selftests/kmod/settings
> > > > 
> > > > diff --git a/tools/testing/selftests/kmod/settings b/tools/testing/selftests/kmod/settings
> > > > new file mode 100644
> > > > index 000000000000..6fca0f1a4594
> > > > --- /dev/null
> > > > +++ b/tools/testing/selftests/kmod/settings
> > > > @@ -0,0 +1,4 @@
> > > > +# measured from a manual run:
> > > > +# time ./tools/testing/selftests/kmod/kmod.sh
> > > > +# Then add ~50 seconds more gracetime.
> > > > +timeout=350
> > > 
> > > Adding timeouts like this for individual tests increases the overall kselftest
> > > run-time. I am not in favor of adding timeouts.
> > > 
> > > We have to find a better way to do this.
> > 
> > Well if folks don't have this the test will fail, and so a false
> > positive. If the goal is to have a low time timeout for "do not run
> > tests past this time and do not fail if we stopped the test" then
> > that seems to be likely one way to go and each test may need to be
> > modified to not fail fatally in case of a special signal.
> > 
> 
> We are finding more and more that timeout values are requiring
> tweaks. I am in favor of coming up a way to exit the test with
> a timeout condition.

OK so do we use the existing timeout as a "optional, I don't want my
test to take longer than this" or "if this test takes longer than
this amount this is a fatal issue"?

I ask because right now we can't override it even with an environment
variable. If we had such support we can let test runners (like kdevops)
use selftests with its own set of qualified / verified timeouts for the
VMs it uses.

For instance, Iw ant to soon start asking 0day to enable my kdevops
0-day tests for the subsystems I maintain, but I can't do that yet as
the timeout is not correct.

  Luis
Shuah Khan March 6, 2023, 4:06 p.m. UTC | #5
On 3/3/23 14:48, Luis Chamberlain wrote:
> On Fri, Mar 03, 2023 at 01:35:10PM -0700, Shuah Khan wrote:
>> On 2/27/23 15:42, Luis Chamberlain wrote:
>>> On Mon, Feb 27, 2023 at 03:32:50PM -0700, Shuah Khan wrote:
>>>> On 2/6/23 16:43, Luis Chamberlain wrote:
>>>>> The default sefltests timeout is 45 seconds. If you run the kmod
>>>>> selftests on your own with say:
>>>>>
>>>>> ./tools/testings/selftests/kmod.sh
>>>>>
>>>>> Then the default timeout won't be in effect.
>>>>>
>>>>> I've never ran kmod selftests using the generic make wrapper
>>>>> (./tools/testing/selftests/run_kselftest.sh -s) util now
>>>>> that I have support for it on kdevops [0]. And with that the
>>>>> test is limitted to the default timeout which we quickly run
>>>>> into. Bump this up to what I see is required on 8GiB / 8 vcpu
>>>>> libvirt q35 guest as can be easily created now with kdevops.
>>>>>
>>>>> To run selftests with kdevops:
>>>>>
>>>>> make menuconfig # enable dedicated selftests and kmod test
>>>>> make
>>>>> make bringup
>>>>> make linux
>>>>> make selftests-kmod
>>>>>
>>>>> This ends up taking about 280 seconds now, give or take add
>>>>> 50 seconds more more and we end up with 350. Document the
>>>>> rationale.
>>>>>
>>>>> [0] https://github.com/linux-kdevops/kdevops
>>>>> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
>>>>> ---
>>>>>     tools/testing/selftests/kmod/settings | 4 ++++
>>>>>     1 file changed, 4 insertions(+)
>>>>>     create mode 100644 tools/testing/selftests/kmod/settings
>>>>>
>>>>> diff --git a/tools/testing/selftests/kmod/settings b/tools/testing/selftests/kmod/settings
>>>>> new file mode 100644
>>>>> index 000000000000..6fca0f1a4594
>>>>> --- /dev/null
>>>>> +++ b/tools/testing/selftests/kmod/settings
>>>>> @@ -0,0 +1,4 @@
>>>>> +# measured from a manual run:
>>>>> +# time ./tools/testing/selftests/kmod/kmod.sh
>>>>> +# Then add ~50 seconds more gracetime.
>>>>> +timeout=350
>>>>
>>>> Adding timeouts like this for individual tests increases the overall kselftest
>>>> run-time. I am not in favor of adding timeouts.
>>>>
>>>> We have to find a better way to do this.
>>>
>>> Well if folks don't have this the test will fail, and so a false
>>> positive. If the goal is to have a low time timeout for "do not run
>>> tests past this time and do not fail if we stopped the test" then
>>> that seems to be likely one way to go and each test may need to be
>>> modified to not fail fatally in case of a special signal.
>>>
>>
>> We are finding more and more that timeout values are requiring
>> tweaks. I am in favor of coming up a way to exit the test with
>> a timeout condition.
> 
> OK so do we use the existing timeout as a "optional, I don't want my
> test to take longer than this" or "if this test takes longer than
> this amount this is a fatal issue"?

It isn't a fatal issue. So I wouldn't call it one. I would add a
message saying test timed out.

One way to handle this is:
- Add a test run-time option and have user tune it as needed.

Make the timeout an option so users can set it based on their
environments.

> 
> I ask because right now we can't override it even with an environment
> variable. If we had such support we can let test runners (like kdevops)
> use selftests with its own set of qualified / verified timeouts for the
> VMs it uses.
> 
> For instance, Iw ant to soon start asking 0day to enable my kdevops
> 0-day tests for the subsystems I maintain, but I can't do that yet as
> the timeout is not correct.

This test isn't part of the default run, so day has to run this as a
special case and it would make prefect sense to provide a tunable
timeout option.

thanks,
-- Shuah
> 
>    Luis
Luis Chamberlain March 8, 2023, 8:29 p.m. UTC | #6
On Mon, Mar 06, 2023 at 09:06:41AM -0700, Shuah Khan wrote:
> On 3/3/23 14:48, Luis Chamberlain wrote:
> > On Fri, Mar 03, 2023 at 01:35:10PM -0700, Shuah Khan wrote:
> > > On 2/27/23 15:42, Luis Chamberlain wrote:
> > > > On Mon, Feb 27, 2023 at 03:32:50PM -0700, Shuah Khan wrote:
> > > > > On 2/6/23 16:43, Luis Chamberlain wrote:
> > > > > > The default sefltests timeout is 45 seconds. If you run the kmod
> > > > > > selftests on your own with say:
> > > > > > 
> > > > > > ./tools/testings/selftests/kmod.sh
> > > > > > 
> > > > > > Then the default timeout won't be in effect.
> > > > > > 
> > > > > > I've never ran kmod selftests using the generic make wrapper
> > > > > > (./tools/testing/selftests/run_kselftest.sh -s) util now
> > > > > > that I have support for it on kdevops [0]. And with that the
> > > > > > test is limitted to the default timeout which we quickly run
> > > > > > into. Bump this up to what I see is required on 8GiB / 8 vcpu
> > > > > > libvirt q35 guest as can be easily created now with kdevops.
> > > > > > 
> > > > > > To run selftests with kdevops:
> > > > > > 
> > > > > > make menuconfig # enable dedicated selftests and kmod test
> > > > > > make
> > > > > > make bringup
> > > > > > make linux
> > > > > > make selftests-kmod
> > > > > > 
> > > > > > This ends up taking about 280 seconds now, give or take add
> > > > > > 50 seconds more more and we end up with 350. Document the
> > > > > > rationale.
> > > > > > 
> > > > > > [0] https://github.com/linux-kdevops/kdevops
> > > > > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > > > > > ---
> > > > > >     tools/testing/selftests/kmod/settings | 4 ++++
> > > > > >     1 file changed, 4 insertions(+)
> > > > > >     create mode 100644 tools/testing/selftests/kmod/settings
> > > > > > 
> > > > > > diff --git a/tools/testing/selftests/kmod/settings b/tools/testing/selftests/kmod/settings
> > > > > > new file mode 100644
> > > > > > index 000000000000..6fca0f1a4594
> > > > > > --- /dev/null
> > > > > > +++ b/tools/testing/selftests/kmod/settings
> > > > > > @@ -0,0 +1,4 @@
> > > > > > +# measured from a manual run:
> > > > > > +# time ./tools/testing/selftests/kmod/kmod.sh
> > > > > > +# Then add ~50 seconds more gracetime.
> > > > > > +timeout=350
> > > > > 
> > > > > Adding timeouts like this for individual tests increases the overall kselftest
> > > > > run-time. I am not in favor of adding timeouts.
> > > > > 
> > > > > We have to find a better way to do this.
> > > > 
> > > > Well if folks don't have this the test will fail, and so a false
> > > > positive. If the goal is to have a low time timeout for "do not run
> > > > tests past this time and do not fail if we stopped the test" then
> > > > that seems to be likely one way to go and each test may need to be
> > > > modified to not fail fatally in case of a special signal.
> > > > 
> > > 
> > > We are finding more and more that timeout values are requiring
> > > tweaks. I am in favor of coming up a way to exit the test with
> > > a timeout condition.
> > 
> > OK so do we use the existing timeout as a "optional, I don't want my
> > test to take longer than this" or "if this test takes longer than
> > this amount this is a fatal issue"?
> 
> It isn't a fatal issue. So I wouldn't call it one. I would add a
> message saying test timed out.
> 
> One way to handle this is:
> - Add a test run-time option and have user tune it as needed.
> 
> Make the timeout an option so users can set it based on their
> environments.
> 
> > 
> > I ask because right now we can't override it even with an environment
> > variable. If we had such support we can let test runners (like kdevops)
> > use selftests with its own set of qualified / verified timeouts for the
> > VMs it uses.
> > 
> > For instance, Iw ant to soon start asking 0day to enable my kdevops
> > 0-day tests for the subsystems I maintain, but I can't do that yet as
> > the timeout is not correct.
> 
> This test isn't part of the default run, so day has to run this as a
> special case and it would make prefect sense to provide a tunable
> timeout option.

That's the thing, I *want* it to be part of *my runs* for my git trees
on git.kernel.org for modules-testing and modules-next. That allows me
to have 0day run whatever things I need. Long term it will be through
kdevops with just:

make linux
make selftests-kmod
make kmod
make kmod-check

Vincenzo expressed interest to help, I think he may be interested in
helping with this configurable timeout for selftests as some initial
low hanging fruit.

Then on the kdevops front we'd set what we know is right for a typical
libvirt use case for our current default VM target.

  Luis
diff mbox series

Patch

diff --git a/tools/testing/selftests/kmod/settings b/tools/testing/selftests/kmod/settings
new file mode 100644
index 000000000000..6fca0f1a4594
--- /dev/null
+++ b/tools/testing/selftests/kmod/settings
@@ -0,0 +1,4 @@ 
+# measured from a manual run:
+# time ./tools/testing/selftests/kmod/kmod.sh
+# Then add ~50 seconds more gracetime.
+timeout=350