Re: [igt-dev] [PATCH i-g-t 2/2] tests/core_hotunplug: Always check device health after late close

[Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>]

On Wed, 12 Oct 2022 13:02:03 +0200
Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote:

> Subtests hotrebind and hotreplug perform post-operation device healthcheck
> while keeping an old instance of the device still open.  If that
> healthcheck succeeds, the subtest deliberately returns success, without
> closing that old device instance first, so potential issues when closing
> it don't contribute to those subtests' results (we have dedicated subtests
> -- hotrebind-lateclose and hotreplug-lateclose -- focused on that step).
> Next, recover() function called from a follow-up igt_fixture section only
> closes the old device, without re-checking whether the new device is still
> healthy thereafter.  As a consequence, next tasks may be executed in an
> insane environment.
> 
> Teach recover() to perform additional healthckech right after device close
> if preceding subtest succeeded but left the device open.  Since next
> recovery steps can recognize potential failure of that healthcheck by
> non-NULL error message passed back, safely ignore the healtheck return
> value.
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>

LGTM.

Acked-by: Mauro Carvalho Chehab <mchehab@kernel.org>

> ---
>  tests/core_hotunplug.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 68fd92238c..ebb646b50f 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -463,8 +463,13 @@ static void pre_check(struct hotunplug *priv)
>  
>  static void recover(struct hotunplug *priv)
>  {
> +	bool late_close = priv->fd.drm >= 0;
> +
>  	cleanup(priv);
>  
> +	if (!priv->failure && late_close)
> +		igt_ignore_warn(healthcheck(priv, false));
> +
>  	/* unbind the driver from a possibly hot rebound unhealthy device */
>  	if (!faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr, F_OK, 0) &&
>  	    priv->fd.drm == -1 && priv->fd.drm_hc == -1 && priv->failure)