diff mbox series

[v2] kselftest/alsa: Increase kselftest timeout

Message ID 20221214130353.1989075-1-nfraprado@collabora.com (mailing list archive)
State New
Headers show
Series [v2] kselftest/alsa: Increase kselftest timeout | expand

Commit Message

Nícolas F. R. A. Prado Dec. 14, 2022, 1:03 p.m. UTC
The default timeout for kselftests is 45 seconds, but that isn't enough
time to run pcm-test when there are many PCMs on the device, nor for
mixer-test when slower control buses and fancier CODECs are present.

As data points, running pcm-test on mt8192-asurada-spherion takes about
1m15s, and mixer-test on rk3399-gru-kevin takes about 2m.

Set the timeout to 4 minutes to allow both pcm-test and mixer-test to
run to completion with some slack.

Reviewed-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---

Changes in v2:
- Reduced timeout from 10 to 4 minutes
- Tweaked commit message to also mention mixer-test and run time for
  mixer-test on rk3399-gru-kevin

 tools/testing/selftests/alsa/settings | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 tools/testing/selftests/alsa/settings

Comments

Shuah Khan Dec. 14, 2022, 4:40 p.m. UTC | #1
On 12/14/22 06:03, Nícolas F. R. A. Prado wrote:
> The default timeout for kselftests is 45 seconds, but that isn't enough
> time to run pcm-test when there are many PCMs on the device, nor for
> mixer-test when slower control buses and fancier CODECs are present.
> 
> As data points, running pcm-test on mt8192-asurada-spherion takes about
> 1m15s, and mixer-test on rk3399-gru-kevin takes about 2m.
> 
> Set the timeout to 4 minutes to allow both pcm-test and mixer-test to
> run to completion with some slack.
> 
> Reviewed-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> ---
> 
> Changes in v2:
> - Reduced timeout from 10 to 4 minutes
> - Tweaked commit message to also mention mixer-test and run time for
>    mixer-test on rk3399-gru-kevin
> 

What I have in mind is that the default run can be limited scope.
Run it on a few controllers and in the report mention that a full
test can be run as needed.

There are a couple of examples of default vs. full test runs - cpu
and memory hot-lug tests.

thanks,
-- Shuah
Mark Brown Dec. 14, 2022, 6:15 p.m. UTC | #2
On Wed, Dec 14, 2022 at 09:40:02AM -0700, Shuah Khan wrote:
> On 12/14/22 06:03, Nícolas F. R. A. Prado wrote:

> > The default timeout for kselftests is 45 seconds, but that isn't enough
> > time to run pcm-test when there are many PCMs on the device, nor for
> > mixer-test when slower control buses and fancier CODECs are present.
> > 
> > As data points, running pcm-test on mt8192-asurada-spherion takes about
> > 1m15s, and mixer-test on rk3399-gru-kevin takes about 2m.
> > 
> > Set the timeout to 4 minutes to allow both pcm-test and mixer-test to
> > run to completion with some slack.

> What I have in mind is that the default run can be limited scope.
> Run it on a few controllers and in the report mention that a full
> test can be run as needed.

> There are a couple of examples of default vs. full test runs - cpu
> and memory hot-lug tests.

For pcm-test it's probably more sensible to refactor things to run
multiple PCMs (or at least cards, though that's less relevant in an
embedded context) in parallel rather than cut down the test coverage,
it's already very limited coverage as things stand.  There is some risk
there could be false positives from cross talk between the PCMs but it's
probably worth it.

With mixer-test if it's actually taking a long time to run generally
this is just identifying that the driver could use some work,
implementing runtime power management and a register cache will probably
resolve most issues.
Nícolas F. R. A. Prado April 3, 2023, 9:30 p.m. UTC | #3
On Wed, Dec 14, 2022 at 06:15:22PM +0000, Mark Brown wrote:
> On Wed, Dec 14, 2022 at 09:40:02AM -0700, Shuah Khan wrote:
> > On 12/14/22 06:03, Nícolas F. R. A. Prado wrote:
> 
> > > The default timeout for kselftests is 45 seconds, but that isn't enough
> > > time to run pcm-test when there are many PCMs on the device, nor for
> > > mixer-test when slower control buses and fancier CODECs are present.
> > > 
> > > As data points, running pcm-test on mt8192-asurada-spherion takes about
> > > 1m15s, and mixer-test on rk3399-gru-kevin takes about 2m.
> > > 
> > > Set the timeout to 4 minutes to allow both pcm-test and mixer-test to
> > > run to completion with some slack.
> 
> > What I have in mind is that the default run can be limited scope.
> > Run it on a few controllers and in the report mention that a full
> > test can be run as needed.
> 
> > There are a couple of examples of default vs. full test runs - cpu
> > and memory hot-lug tests.
> 
> For pcm-test it's probably more sensible to refactor things to run
> multiple PCMs (or at least cards, though that's less relevant in an
> embedded context) in parallel rather than cut down the test coverage,
> it's already very limited coverage as things stand.  There is some risk
> there could be false positives from cross talk between the PCMs but it's
> probably worth it.
> 
> With mixer-test if it's actually taking a long time to run generally
> this is just identifying that the driver could use some work,
> implementing runtime power management and a register cache will probably
> resolve most issues.

Hi Shuah and Mark,

sorry for the delay, but I'd still like to move forward with this.

Shuah, I've checked the tests you mentioned that have limited scope by default,
and we could do the same for the alsa kselftest, but I'm not sure I understand
how this solves the problem here. The fact is that the current timeout is too
short for a full run of the alsa kselftest on some machines, so we need to
increase the timeout in any case regardless of there being a limited scope run
by default or not. Mark implemented the parallelization he mentioned in the
meantime, but it doesn't help every hardware. The only other option I see is
reducing the time the PCM is tested for (currently 4 seconds). But I assume that
number was chosen for a reason.

I'd also like to better understand why we have an arbitrary (45 seconds) default
timeout. If there are users who want to limit the test duration to an arbitrary
time even if that means not getting full test coverage, shouldn't such arbitrary
time be supplied by the users themselves?

And is there any guidance on what are the acceptable exceptions to having a
longer timeout? Because there seem to be many kselftests which override the
default timeout with a longer one, even ones that disable it altogether.

I can see the value of having a timeout as the worst case scenario of how long
the test takes to run, to avoid hanging indefinitely, which is what the tests
with overriden timeout setting do. For the alsa kselftests that's
hardware-dependent (number of kcontrols for mixer-test; and number of PCMs and
working configurations for pcm-test), so we'd either need to guess a high enough
value for the timeout that all known hardware fits, or allow the timeout to be
set dynamically during runtime.

Thanks,
Nícolas
diff mbox series

Patch

diff --git a/tools/testing/selftests/alsa/settings b/tools/testing/selftests/alsa/settings
new file mode 100644
index 000000000000..b478e684846a
--- /dev/null
+++ b/tools/testing/selftests/alsa/settings
@@ -0,0 +1 @@ 
+timeout=240