On Fri, 13 May 2022 02:31:28 -0700, Tvrtko Ursulin wrote:
>
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> Leave some sysfs files open and attempt to read from them after hotunplug.
A few minor comments below but otherwise LGTM:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
> ---
> tests/core_hotunplug.c | 62 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 02eae19e1e16..c3831541202d 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -602,6 +602,59 @@ static void hotreplug_lateclose(struct hotunplug *priv)
> igt_assert_f(healthcheck(priv, false), "%s\n", priv->failure);
> }
>
> +static void hotunplug_sysfs(struct hotunplug *priv)
> +{
> + int i915, sysfs;
> + struct {
> + int fd;
> + const char *path;
> + char buf[256];
> + } *item, items[] = {
> + { .fd = -1, .path = "error" },
> + { .fd = -1, .path = "gt_act_freq_mhz" },
> + { .fd = -1, .path = "gt/gt0/rps_act_freq_mhz" },
> + { .fd = -1, .path = "gt/gt0/rc6_residency_ms" },
> + { .fd = -1, .path = "engine/rcs0/name" },
> + { .fd = -1, .path = "engine/bcs0/name" },
> + { .path = NULL },
> + };
> + unsigned int cnt = 0;
> +
> + pre_check(priv);
> +
> + i915 = local_drm_open_driver(false, "", " for hot unplug");
> + sysfs = igt_sysfs_open(i915);
> + igt_require(sysfs >= 0);
> + for (item = &items[0]; item->path; item++) {
> + item->fd = openat(sysfs, item->path, O_RDONLY);
> + cnt += item->fd >= 0;
> + }
> + close(sysfs);
> + close_device(i915, " ", "sysfs open ");
> +
> + igt_require(cnt >= 3);
igt_require(cnt) ?
> +
> + device_unplug(priv, "hot ", 60);
Not sure what happens when there are child devices etc. Anyway it's being
used elsewhere too so probably ok.
Also maybe unbind (instead of remove) using driver_unbind() is sufficient?
> +
> + sleep(2);
Device remove via sysfs:
echo 1 > /sys/bus/pci/devices/0000\:03\:00.0/remove
is most likely synchronous but anyway...
> +
> + for (item = &items[0]; item->path; item++) {
> + ssize_t len;
> + int err;
> +
> + if (item->fd < 0)
> + continue;
> +
> + len = read(item->fd, item->buf, sizeof(item->buf));
> + err = len < 0 ? -errno : 0;
> + close(item->fd);
> + igt_debug("%s='%s' (%d)\n", item->path, item->buf, err);
> + igt_assert_eq(err, -ENODEV);
> + }
> +
> + igt_assert_f(healthcheck(priv, false), "%s\n", priv->failure);
> +}
> +
> /* Main */
>
> igt_main
> @@ -735,6 +788,15 @@ igt_main
> recover(&priv);
> }
>
> + igt_subtest_group {
> + igt_describe("Check if sysfs files left open can be safely accessed after hotunplug.");
> + igt_subtest("hotunplug-sysfs")
> + hotunplug_sysfs(&priv);
> +
> + igt_fixture
> + recover(&priv);
> + }
> +
> igt_fixture {
> post_healthcheck(&priv);
>
> --
> 2.32.0
>
