From patchwork Thu Aug 20 14:52:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Janusz Krzysztofik X-Patchwork-Id: 11726397 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 33469739 for ; Thu, 20 Aug 2020 14:53:06 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1C6222078B for ; Thu, 20 Aug 2020 14:53:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1C6222078B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 94D066E54C; Thu, 20 Aug 2020 14:53:05 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3E4AF6E94D; Thu, 20 Aug 2020 14:53:04 +0000 (UTC) IronPort-SDR: wTJ9WWiZpwJmjsugoPX/Kv0LIN92UrZhC2yItBwyUzuUGHDuQMkJ04y8dbp6jl4LEVoS1EIqZ3 ScM2wS0rnXRQ== X-IronPort-AV: E=McAfee;i="6000,8403,9718"; a="240136191" X-IronPort-AV: E=Sophos;i="5.76,333,1592895600"; d="scan'208";a="240136191" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Aug 2020 07:53:03 -0700 IronPort-SDR: fubk1n1G0ZOjYDDwXXRaDkRRdCm8fMzFSO+YyWVm2gNCL/zo1skbtDub87rX0totATf1nLAY2o eF3QfHutRGiw== X-IronPort-AV: E=Sophos;i="5.76,333,1592895600"; d="scan'208";a="472679724" Received: from jkrzyszt-desk.igk.intel.com ([172.22.244.18]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Aug 2020 07:53:01 -0700 From: Janusz Krzysztofik To: igt-dev@lists.freedesktop.org Date: Thu, 20 Aug 2020 16:52:05 +0200 Message-Id: <20200820145215.13238-10-janusz.krzysztofik@linux.intel.com> X-Mailer: git-send-email 2.21.1 In-Reply-To: <20200820145215.13238-1-janusz.krzysztofik@linux.intel.com> References: <20200820145215.13238-1-janusz.krzysztofik@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH i-g-t v3 09/19] tests/core_hotunplug: Prepare invariant data once per test run X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?utf-8?q?Micha=C5=82_Winiarski?= , intel-gfx@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" 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 lifecycle 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' subnode 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. v3: Refresh. Suggested-by: Michał Winiarski Signed-off-by: Janusz Krzysztofik Reviewed-by: Michał Winiarski --- tests/core_hotunplug.c | 85 +++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 54 deletions(-) diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c index f7a54010b..849a774ff 100644 --- a/tests/core_hotunplug.c +++ b/tests/core_hotunplug.c @@ -86,41 +86,30 @@ 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); -} - -static void prepare(struct hotunplug *priv, char *buf, int buflen) -{ - priv->fd.drm = local_drm_open_driver("", " for subtest"); + sysfs_path = strchr(filter, ':'); + igt_assert(sysfs_path++); - priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm); + priv->fd.sysfs_dev = open(sysfs_path, O_DIRECTORY); igt_assert_fd(priv->fd.sysfs_dev); - 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_drv = openat(priv->fd.sysfs_dev, "driver", O_DIRECTORY); + igt_assert_fd(priv->fd.sysfs_drv); + + priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev, "subsystem/devices", + 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 */ @@ -133,8 +122,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 */ @@ -147,18 +134,20 @@ 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) { + 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; @@ -172,11 +161,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) @@ -229,14 +216,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)); - - igt_debug("closing the device\n"); - priv->fd.drm = close_device(priv->fd.drm); - igt_assert_eq(priv->fd.drm, -1); - driver_unbind(priv, ""); driver_bind(priv); @@ -246,12 +225,6 @@ static void unbind_rebind(struct hotunplug *priv) static void unplug_rescan(struct hotunplug *priv) { - prepare(priv, NULL, 0); - - igt_debug("closing the device\n"); - priv->fd.drm = close_device(priv->fd.drm); - igt_assert_eq(priv->fd.drm, -1); - device_unplug(priv, ""); bus_rescan(priv); @@ -261,9 +234,7 @@ static void unplug_rescan(struct hotunplug *priv) static void hotunbind_lateclose(struct hotunplug *priv) { - char buf[PATH_MAX]; - - prepare(priv, buf, sizeof(buf)); + priv->fd.drm = local_drm_open_driver("", " for hotunbind"); driver_unbind(priv, "hot "); @@ -277,7 +248,7 @@ static void hotunbind_lateclose(struct hotunplug *priv) static void hotunplug_lateclose(struct hotunplug *priv) { - prepare(priv, NULL, 0); + priv->fd.drm = local_drm_open_driver("", " for hotunplug"); device_unplug(priv, "hot "); @@ -313,6 +284,8 @@ igt_main set_filter_from_device(fd_drm); igt_assert_eq(close_device(fd_drm), -1); + + prepare(&priv); } igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed"); @@ -340,6 +313,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); + } }