From patchwork Fri Aug 28 07:59:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Janusz Krzysztofik X-Patchwork-Id: 11742317 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 3A3F9913 for ; Fri, 28 Aug 2020 08:00:13 +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 2157220C56 for ; Fri, 28 Aug 2020 08:00:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2157220C56 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 AB4A76EB9E; Fri, 28 Aug 2020 08:00:10 +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 656396EBAC; Fri, 28 Aug 2020 08:00:03 +0000 (UTC) IronPort-SDR: +UBaKWYiwzcXS7QmDBXciD6Mmkl2m4TPgfopPOMRbPTXJqdQJkszm65wvPK0Ox3cHpeNK6GNGR ITbihjuirbrg== X-IronPort-AV: E=McAfee;i="6000,8403,9726"; a="241440837" X-IronPort-AV: E=Sophos;i="5.76,363,1592895600"; d="scan'208";a="241440837" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Aug 2020 01:00:02 -0700 IronPort-SDR: N+cdN1/gm8R7aZoHDfzXYCXDPPpxX1gBA55EwJ+0M+Y7n3gNnMnxK8WHaHQ4Yeca+Fb7a1mWJ5 4KaKknwBA2ww== X-IronPort-AV: E=Sophos;i="5.76,362,1592895600"; d="scan'208";a="444755920" Received: from jkrzyszt-desk.igk.intel.com ([172.22.244.18]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Aug 2020 01:00:00 -0700 From: Janusz Krzysztofik To: igt-dev@lists.freedesktop.org Date: Fri, 28 Aug 2020 09:59:14 +0200 Message-Id: <20200828075927.17061-9-janusz.krzysztofik@linux.intel.com> X-Mailer: git-send-email 2.21.1 In-Reply-To: <20200828075927.17061-1-janusz.krzysztofik@linux.intel.com> References: <20200828075927.17061-1-janusz.krzysztofik@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH i-g-t v5 08/21] tests/core_hotunplug: Handle device close errors 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" The test now ignores device close errors. Those errors are believed to have no influence on device health so there is no need to process them the same way as we mostly do on errors, i.e., notify CI about a problem via igt_abort. However, those errors may indicate issues with the test itself. Moreover, impact of those errors on operations performed by subtests, like driver unbind or device remove, should be perceived as undefined. Then, we should fail as soon as a device or device sysfs node close error occurs in a subtest and also skip subsequent subtests. However, once a driver unbind or device unplug operation has been attempted by a subtest, we would still like to check the device health. When in a subtest, store results of device close operations for future reference. Reuse file descriptor fields of the hotunplug structure for that. Unless in between of a driver remove or device unplug operation and a successful device health check completion, fail current test section right after a device close error occurs, warn otherwise. If still running, examine device file descriptor fields in subsequent igt_fixture sections and skip on errors. v2: Fix a typo in post_healthcheck function name. v3: Don't fail on close error after successful health check, warn only, - move duplicated messages to helpers. v4: On start of each subtest assert device file descriptors closed cleanly. Signed-off-by: Janusz Krzysztofik Reviewed-by: Michał Winiarski # v1 --- tests/core_hotunplug.c | 68 +++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 14 deletions(-) diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c index 4f7e89c95..2884c3f77 100644 --- a/tests/core_hotunplug.c +++ b/tests/core_hotunplug.c @@ -43,7 +43,7 @@ struct hotunplug { int sysfs_dev; int sysfs_bus; int sysfs_drv; - } fd; + } fd; /* >= 0: valid fd, == -1: closed, < -1: close failed */ const char *dev_bus_addr; const char *failure; }; @@ -67,6 +67,25 @@ static int local_drm_open_driver(const char *prefix, const char *suffix) return fd_drm; } +static int local_close(int fd, const char *message) +{ + errno = 0; + if (igt_warn_on_f(close(fd), "%s\n", message)) + return -errno; /* (never -1) */ + + return -1; /* success - return 'closed' */ +} + +static int close_device(int fd_drm) +{ + return local_close(fd_drm, "Device close failed"); +} + +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) { int len; @@ -83,11 +102,16 @@ static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen) igt_assert(priv->dev_bus_addr++); /* sysfs_dev no longer needed */ - close(priv->fd.sysfs_dev); + 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) { + /* 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.drm = local_drm_open_driver("", " for subtest"); priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm); @@ -142,7 +166,7 @@ static void device_unplug(struct hotunplug *priv, const char *prefix) igt_reset_timeout(); priv->failure = NULL; - close(priv->fd.sysfs_dev); + priv->fd.sysfs_dev = close_sysfs(priv->fd.sysfs_dev); } /* Re-discover the device by rescanning its bus */ @@ -161,6 +185,7 @@ static void bus_rescan(struct hotunplug *priv) static void healthcheck(struct hotunplug *priv) { + /* preserve error code potentially stored before in priv->fd.drm */ int fd_drm; /* device name may have changed, rebuild IGT device list */ @@ -176,7 +201,17 @@ static void healthcheck(struct hotunplug *priv) priv->failure = NULL; } - close(fd_drm); + fd_drm = close_device(fd_drm); + if (priv->fd.drm == -1) /* store result if no error code to preserve */ + priv->fd.drm = fd_drm; +} + +static void post_healthcheck(struct hotunplug *priv) +{ + igt_abort_on_f(priv->failure, "%s\n", priv->failure); + + igt_require(priv->fd.drm == -1); + igt_require(priv->fd.sysfs_dev == -1); } static void set_filter_from_device(int fd) @@ -203,7 +238,8 @@ static void unbind_rebind(struct hotunplug *priv) prepare(priv, buf, sizeof(buf)); igt_debug("closing the device\n"); - close(priv->fd.drm); + priv->fd.drm = close_device(priv->fd.drm); + igt_assert_eq(priv->fd.drm, -1); driver_unbind(priv, ""); @@ -217,7 +253,8 @@ static void unplug_rescan(struct hotunplug *priv) prepare(priv, NULL, 0); igt_debug("closing the device\n"); - close(priv->fd.drm); + priv->fd.drm = close_device(priv->fd.drm); + igt_assert_eq(priv->fd.drm, -1); device_unplug(priv, ""); @@ -237,7 +274,7 @@ static void hotunbind_lateclose(struct hotunplug *priv) driver_bind(priv); igt_debug("late closing the unbound device instance\n"); - close(priv->fd.drm); + priv->fd.drm = close_device(priv->fd.drm); healthcheck(priv); } @@ -251,7 +288,7 @@ static void hotunplug_lateclose(struct hotunplug *priv) bus_rescan(priv); igt_debug("late closing the removed device instance\n"); - close(priv->fd.drm); + priv->fd.drm = close_device(priv->fd.drm); healthcheck(priv); } @@ -260,7 +297,10 @@ static void hotunplug_lateclose(struct hotunplug *priv) igt_main { - struct hotunplug priv = { .failure = NULL, }; + struct hotunplug priv = { + .fd = { .drm = -1, .sysfs_dev = -1, }, + .failure = NULL, + }; igt_fixture { int fd_drm; @@ -276,7 +316,7 @@ igt_main /* Make sure subtests always reopen the same device */ set_filter_from_device(fd_drm); - close(fd_drm); + igt_assert_eq(close_device(fd_drm), -1); } igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed"); @@ -284,26 +324,26 @@ igt_main unbind_rebind(&priv); igt_fixture - igt_abort_on_f(priv.failure, "%s\n", priv.failure); + post_healthcheck(&priv); igt_describe("Check if a device believed to be closed can be cleanly unplugged"); igt_subtest("unplug-rescan") unplug_rescan(&priv); igt_fixture - igt_abort_on_f(priv.failure, "%s\n", priv.failure); + post_healthcheck(&priv); igt_describe("Check if the driver can be cleanly unbound from a still open device, then released"); igt_subtest("hotunbind-lateclose") hotunbind_lateclose(&priv); igt_fixture - igt_abort_on_f(priv.failure, "%s\n", priv.failure); + post_healthcheck(&priv); igt_describe("Check if a still open device can be cleanly unplugged, then released"); igt_subtest("hotunplug-lateclose") hotunplug_lateclose(&priv); igt_fixture - igt_abort_on_f(priv.failure, "%s\n", priv.failure); + post_healthcheck(&priv); }