@@ -87,45 +87,31 @@ static int close_sysfs(int fd_sysfs_dev)
return local_close(fd_sysfs_dev, "Device sysfs node close failed");
}
-static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
+static void prepare(struct hotunplug *priv)
{
- int len;
+ const char *filter = igt_device_filter_get(0), *sysfs_path;
- igt_assert(buflen);
+ igt_assert(filter);
- priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
- O_DIRECTORY);
- igt_assert_fd(priv->fd.sysfs_drv);
-
- len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
- buf[len] = '\0';
- priv->dev_bus_addr = strrchr(buf, '/');
+ priv->dev_bus_addr = strrchr(filter, '/');
igt_assert(priv->dev_bus_addr++);
- /* sysfs_dev no longer needed */
- priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
- igt_assert_eq(priv->fd.sysfs_dev, -1);
-}
+ sysfs_path = strchr(filter, ':');
+ igt_assert(sysfs_path++);
-static void prepare(struct hotunplug *priv, char *buf, int buflen)
-{
- /* assert device file descriptors closed cleanly on subtest start */
- igt_assert_eq(priv->fd.drm, -1);
igt_assert_eq(priv->fd.sysfs_dev, -1);
+ priv->fd.sysfs_dev = open(sysfs_path, O_DIRECTORY);
+ igt_assert_fd(priv->fd.sysfs_dev);
- priv->fd.drm = local_drm_open_driver("", " for subtest");
+ priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "driver", O_DIRECTORY);
+ igt_assert_fd(priv->fd.sysfs_drv);
- priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
- igt_assert_fd(priv->fd.sysfs_dev);
+ priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev, "subsystem/devices",
+ O_DIRECTORY);
+ igt_assert_fd(priv->fd.sysfs_bus);
- if (buf) {
- prepare_for_unbind(priv, buf, buflen);
- } else {
- /* prepare for bus rescan */
- priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev,
- "device/subsystem", O_DIRECTORY);
- igt_assert_fd(priv->fd.sysfs_bus);
- }
+ priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev);
+ igt_assert_eq(priv->fd.sysfs_dev, -1);
}
/* Unbind the driver from the device */
@@ -138,8 +124,6 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix)
igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
igt_reset_timeout();
priv->failure = NULL;
-
- /* don't close fd.sysfs_drv, it will be used for driver rebinding */
}
/* Re-bind the driver to the device */
@@ -152,18 +136,21 @@ static void driver_bind(struct hotunplug *priv)
igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
igt_reset_timeout();
priv->failure = NULL;
-
- close(priv->fd.sysfs_drv);
}
/* Remove (virtually unplug) the device from its bus */
static void device_unplug(struct hotunplug *priv, const char *prefix)
{
+ igt_assert_eq(priv->fd.sysfs_dev, -1);
+ priv->fd.sysfs_dev = openat(priv->fd.sysfs_bus, priv->dev_bus_addr,
+ O_DIRECTORY);
+ igt_assert_fd(priv->fd.sysfs_dev);
+
igt_debug("%sunplugging the device\n", prefix);
priv->failure = "Device unplug timeout!";
igt_set_timeout(60, priv->failure);
- igt_sysfs_set(priv->fd.sysfs_dev, "device/remove", "1");
+ igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1");
igt_reset_timeout();
priv->failure = NULL;
@@ -177,11 +164,9 @@ static void bus_rescan(struct hotunplug *priv)
priv->failure = "Bus rescan timeout!";
igt_set_timeout(60, priv->failure);
- igt_sysfs_set(priv->fd.sysfs_bus, "rescan", "1");
+ igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1");
igt_reset_timeout();
priv->failure = NULL;
-
- close(priv->fd.sysfs_bus);
}
static void healthcheck(struct hotunplug *priv)
@@ -237,11 +222,6 @@ static void set_filter_from_device(int fd)
static void unbind_rebind(struct hotunplug *priv)
{
- char buf[PATH_MAX];
-
- prepare(priv, buf, sizeof(buf));
-
- priv->fd.drm = close_device(priv->fd.drm, "", "exercised ");
igt_assert_eq(priv->fd.drm, -1);
driver_unbind(priv, "");
@@ -253,9 +233,6 @@ static void unbind_rebind(struct hotunplug *priv)
static void unplug_rescan(struct hotunplug *priv)
{
- prepare(priv, NULL, 0);
-
- priv->fd.drm = close_device(priv->fd.drm, "", "exercised ");
igt_assert_eq(priv->fd.drm, -1);
device_unplug(priv, "");
@@ -267,9 +244,8 @@ static void unplug_rescan(struct hotunplug *priv)
static void hotunbind_lateclose(struct hotunplug *priv)
{
- char buf[PATH_MAX];
-
- prepare(priv, buf, sizeof(buf));
+ igt_assert_eq(priv->fd.drm, -1);
+ priv->fd.drm = local_drm_open_driver("", " for hot unbind");
driver_unbind(priv, "hot ");
@@ -282,7 +258,8 @@ static void hotunbind_lateclose(struct hotunplug *priv)
static void hotunplug_lateclose(struct hotunplug *priv)
{
- prepare(priv, NULL, 0);
+ igt_assert_eq(priv->fd.drm, -1);
+ priv->fd.drm = local_drm_open_driver("", " for hot unplug");
device_unplug(priv, "hot ");
@@ -317,6 +294,8 @@ igt_main
set_filter_from_device(fd_drm);
igt_assert_eq(close_device(fd_drm, "", "selected "), -1);
+
+ prepare(&priv);
}
igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
@@ -344,6 +323,10 @@ igt_main
igt_subtest("hotunplug-lateclose")
hotunplug_lateclose(&priv);
- igt_fixture
+ igt_fixture {
post_healthcheck(&priv);
+
+ close(priv.fd.sysfs_bus);
+ close(priv.fd.sysfs_drv);
+ }
}
Each subtest now calls a prepare() helper which opens a couple of files required by that subtest. Those files are then closed after use, either directly from the subtest body, or indirectly from inside one of helper functions called during the subtest execution. That approach not only makes life cycle of individual file descriptors difficult to follow but also prevents us from re-running health checks on subtest failures from follow up igt_fixture sections since we may need to retry bus rescan or driver rebind operations. Two of those files - device bus and driver sysfs nodes - are not affected nor interfere with driver unbind / device unplug operations performed by subtests. Then, there is not much sense in closing and reopening those nodes. Open them once at the beginning of a test run, then close them as late as on test completion. The prepare() helper also populates a device bus address string used by driver unbind / rebind operations. Since the bus address of an exercised device never changes, also prepare that string only once at the beginning of a test run. Note that it is the same as the last component of a device filter string which is already resolved and installed from an initial igt_fixture section of the test. Then, initialize the device bus address field of a hotunplug structure instance with a pointer to the respective substring of that filter rather than resolving it again from the device sysfs node pathname. There is one more sysfs node - a DRM device node - now opened by the prepare() helper for subtests which perform device remove operations. That node can't be opened only once at the beginning of a test run because its open file descriptor is no longer usable as soon as a driver unbind operation is performed. On the other hand, it can't be opened easily from inside a device_remove() helper since some subtests just don't open the device so its file descriptor used by igt_sysfs_open() may just not be available. However, note that only a PCI sysfs node of the device, not necessarily the DRM one, is actually required for a successful device remove operation, and that node can be opened easily from a bus file descriptor using a device bus address string, both already available. Then, change the semantics of a .fd.sysfs_dev field of the hotunplug structure from DRM to PCI device sysfs file descriptor, then let the device_remove() helper open the device PCI node by itself and store its file descriptor in that field. Also, for still more easy access to the device PCI node, use a 'subsystem/devices' sub-node of the PCI device as its bus sysfs location instead of just 'subsystem', then adjust a relative path to the bus 'rescan' function accordingly. A side benefit of using the PCI device sysfs node, not the DRM one, while removing the device is that a future subtest may now easily perform both driver unbind and device remove operations in a row. v2: Rebase only. v3: Refresh. v4: Refresh, still assert a device dile descriptor closed cleanly on subtest start, a device sysfs file descriptor still before open. Suggested-by: Michał Winiarski <michal.winiarski@intel.com> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> # v1 --- tests/core_hotunplug.c | 83 +++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 50 deletions(-)