Re: [PATCH 1/2] test_firmware: add suspend support to test buggy drivers

Lukas Middendorf
2021-04-21T22:23:25Z

On 17/04/2021 01:58, Luis Chamberlain wrote:
> Lukas Middendorf reported a situation where a driver's
> request_firmware() call on resume caused a stall. Upon
> inspection the issue was that the driver in question was
> calling request_firmware() only on resume, and since we
> currently do not have a generic kernel VFS freeze / thaw
> solution in place we are allowing races for filesystems
> to race against the disappearance of a block device, and
> this is presently an issue which can lead to a stall.
>
> It is difficult to reproduce this unless you have hardware
> which mimics this setup. So to test this setup, let's just
> implement support for doing these wacky things. This lets
> us test that corner case easily as follows.
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

The resume test functionality added to test_firmware works as intended
in reproducing the stall bug I reported.

However, the fw_test_resume.sh seems to be incomplete.

> +usage()
> +{
> + echo "Usage: $0 [ -v ] | [ -h | --help]"
> + echo ""
> + echo " --check-resume-test Verify resume test"
> + echo " -h|--help Help"
> + echo
> + echo "Without any arguments this will enable the resume firmware test"
> + echo "after suspend. To verify that the test went well, run with -v".
> + echo
> + exit 1
> +}

The "-v" is not implemented as an alias to "--check-resume-test"

> +
> +verify_resume_test()
> +{
> + trap "test_finish" EXIT
> +}

Does not seem to do anything regarding checking, just some cleanup.

> +
> +parse_args()
> +{
> + if [ $# -eq 0 ]; then
> + config_enable_resume_test
> + else
> + if [[ "$1" = "--check-resume-test" ]]; then
> + config_disable_resume_test
> + verify_resume_test
> + else
> + usage
> + fi
> + fi
> +}
> +

there should likely be a call

parse_args "$@"

> +exit 0