On Fri, 13 May 2022 02:31:28 -0700, Tvrtko Ursulin wrote: > > From: Tvrtko Ursulin <[email protected]> > > 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 <[email protected]> > Signed-off-by: Tvrtko Ursulin <[email protected]> > Cc: Ashutosh Dixit <[email protected]> > --- > 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 >
