[igt-dev] [PATCH v3 1/2] tests/kms_lease: Terminate lease fd before subtests and remove lease_t from all sub-tests

Mohammed Thasleem mohammed.thasleem at intel.com
Tue Oct 3 19:02:21 UTC 2023


Failures happen at drmModeSetCursor and prepare_crtc_master or
prepare_crtc_lease which exit tests without termination of lease fd
which resulting in resource busy for sub-sequence execution, when
new lessor request for lease.

Terminate the lease fd when any assert happen with out termination
of lease fd and on any drmModeSetCrtc or drmModeGetCrtc exit.
Terminate lease fd on every display dependent subtests execution.

Remove lease_t from all sub-tests and move it to data_t and
send data struct instead lease struct and avoid dupicating
of prepare_crtc.

v2: -Separated prepare crtc for master and lease.
    -Replace close with terminate_lease.
    -Removed lease_t from all sub-tests. (Ankit)
v3: Updated commit message. (Ankit)
v4: Avoid duplicating prepare_crtc. (Ankit)
v5: Send fd to close instead structure address.

Signed-off-by: Mohammed Thasleem <mohammed.thasleem at intel.com>
---
 tests/kms_lease.c | 129 ++++++++++++++++++++++------------------------
 1 file changed, 61 insertions(+), 68 deletions(-)

diff --git a/tests/kms_lease.c b/tests/kms_lease.c
index 343cdb812..ad739677f 100644
--- a/tests/kms_lease.c
+++ b/tests/kms_lease.c
@@ -211,6 +211,7 @@ typedef struct {
 } lease_t;
 
 typedef struct {
+	lease_t lease;
 	lease_t master;
 	enum pipe pipe;
 	uint32_t crtc_id;
@@ -226,9 +227,10 @@ static igt_output_t *connector_id_to_output(igt_display_t *display, uint32_t con
 	return igt_output_from_connector(display, &connector);
 }
 
-static int prepare_crtc(lease_t *lease, data_t *data)
+static int prepare_crtc(data_t *data, bool is_master)
 {
 	drmModeModeInfo *mode;
+	lease_t *lease = is_master ? &data->master : &data->lease;
 	igt_display_t *display = &lease->display;
 	igt_output_t *output = connector_id_to_output(display, data->connector_id);
 	enum pipe pipe = display->pipes[data->pipe].pipe;
@@ -248,7 +250,6 @@ static int prepare_crtc(lease_t *lease, data_t *data)
 			    DRM_FORMAT_MOD_LINEAR,
 			    0.0, 0.0, 0.0,
 			    &lease->primary_fb);
-
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_plane_set_fb(primary, &lease->primary_fb);
 
@@ -314,7 +315,7 @@ static int get_lease(int fd, struct drm_mode_get_lease *mgl)
 	return err;
 }
 
-static int make_lease(data_t *data, lease_t *lease)
+static int make_lease(data_t *data)
 {
 	uint32_t object_ids[3];
 	struct drm_mode_create_lease mcl;
@@ -330,18 +331,18 @@ static int make_lease(data_t *data, lease_t *lease)
 	object_ids[mcl.object_count++] = data->plane_id;
 
 	ret = create_lease(data->master.fd, &mcl);
+	data->lease.fd = mcl.fd;
 
 	if (ret)
 		return ret;
 
-	lease->fd = mcl.fd;
-	lease->lessee_id = mcl.lessee_id;
+	data->lease.lessee_id = mcl.lessee_id;
 	return 0;
 }
 
-static void terminate_lease(lease_t *lease)
+static void terminate_lease(int lease_fd)
 {
-	close(lease->fd);
+	close(lease_fd);
 }
 
 static int paint_fb(int drm_fd, struct igt_fb *fb, const char *test_name,
@@ -370,24 +371,23 @@ static void simple_lease(data_t *data)
 {
 	enum pipe pipe = data->pipe;
 
-	lease_t lease;
-
 	/* Create a valid lease */
-	igt_assert_eq(make_lease(data, &lease), 0);
+	igt_assert_eq(make_lease(data), 0);
 
-	igt_display_require(&lease.display, lease.fd);
+	igt_display_require(&data->lease.display, data->lease.fd);
 
 	/* Set a mode on the leased output */
-	igt_assert_eq(0, prepare_crtc(&lease, data));
+	igt_assert_eq(0, prepare_crtc(data, false));
 
 	/* Paint something attractive */
-	paint_fb(lease.fd, &lease.primary_fb, "simple-lease",
-		 lease.mode->name, igt_output_name(lease.output), kmstest_pipe_name(pipe));
+	paint_fb(data->lease.fd, &data->lease.primary_fb, "simple-lease",
+		 data->lease.mode->name, igt_output_name(data->lease.output),
+		 kmstest_pipe_name(pipe));
 	igt_debug_wait_for_keypress("lease");
-	cleanup_crtc(&lease,
-		     connector_id_to_output(&lease.display, data->connector_id));
+	cleanup_crtc(&data->lease,
+		     connector_id_to_output(&data->lease.display, data->connector_id));
 
-	terminate_lease(&lease);
+	terminate_lease(data->lease.fd);
 }
 
 static void empty_lease(data_t *data)
@@ -432,7 +432,7 @@ static void page_flip_implicit_plane(data_t *data)
 	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
 
 	/* Set a mode on the leased output */
-	igt_assert_eq(0, prepare_crtc(&data->master, data));
+	igt_assert_eq(0, prepare_crtc(data, true));
 
 	/* sanity check */
 	do_or_die(drmModePageFlip(data->master.fd, data->crtc_id,
@@ -509,7 +509,7 @@ static void setcrtc_implicit_plane(data_t *data)
 	}
 
 	/* Set a mode on the leased output */
-	igt_assert_eq(0, prepare_crtc(&data->master, data));
+	igt_assert_eq(0, prepare_crtc(data, true));
 
 	/* sanity check */
 	ret = drmModeSetCrtc(data->master.fd, data->crtc_id, -1,
@@ -522,6 +522,7 @@ static void setcrtc_implicit_plane(data_t *data)
 
 	object_ids[mcl.object_count++] = wrong_plane_id;
 	do_or_die(create_lease(data->master.fd, &mcl));
+	data->lease.fd = mcl.fd;
 
 	igt_assert_eq(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
 				     0, 0, object_ids, 1, mode),
@@ -552,7 +553,7 @@ static void cursor_implicit_plane(data_t *data)
 	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
 
 	/* Set a mode on the leased output */
-	igt_assert_eq(0, prepare_crtc(&data->master, data));
+	igt_assert_eq(0, prepare_crtc(data, true));
 
 	/* sanity check */
 	do_or_die(drmModeSetCursor(data->master.fd, data->crtc_id, 0, 0, 0));
@@ -569,7 +570,7 @@ static void cursor_implicit_plane(data_t *data)
 
 	cleanup_crtc(&data->master,
 		     connector_id_to_output(&data->master.display, data->connector_id));
-}
+	}
 
 static void atomic_implicit_crtc(data_t *data)
 {
@@ -662,19 +663,18 @@ static void atomic_implicit_crtc(data_t *data)
 /* Test listing lessees */
 static void lessee_list(data_t *data)
 {
-	lease_t lease;
 	struct drm_mode_list_lessees mll;
 	uint32_t lessees[1];
 
 	mll.pad = 0;
 
 	/* Create a valid lease */
-	igt_assert_eq(make_lease(data, &lease), 0);
+	igt_assert_eq(make_lease(data), 0);
 
 	/* check for nested leases */
 	mll.count_lessees = 0;
 	mll.lessees_ptr = 0;
-	igt_assert_eq(list_lessees(lease.fd, &mll), 0);
+	igt_assert_eq(list_lessees(data->lease.fd, &mll), 0);
 	igt_assert_eq(mll.count_lessees, 0);
 
 	/* Get the number of lessees */
@@ -696,14 +696,14 @@ static void lessee_list(data_t *data)
 	igt_assert_eq(mll.count_lessees, 1);
 
 	/* Make sure the listed lease is the same as the one we created */
-	igt_assert_eq(lessees[0], lease.lessee_id);
+	igt_assert_eq(lessees[0], data->lease.lessee_id);
 
 	/* invalid pad */
 	mll.pad = -1;
 	igt_assert_eq(list_lessees(data->master.fd, &mll), -EINVAL);
 	mll.pad = 0;
 
-	terminate_lease(&lease);
+	terminate_lease(data->lease.fd);
 
 	/* Make sure the lease is gone */
 	igt_assert_eq(list_lessees(data->master.fd, &mll), 0);
@@ -713,7 +713,6 @@ static void lessee_list(data_t *data)
 /* Test getting the contents of a lease */
 static void lease_get(data_t *data)
 {
-	lease_t lease;
 	struct drm_mode_get_lease mgl;
 	int num_leased_obj = 3;
 	uint32_t objects[num_leased_obj];
@@ -722,12 +721,12 @@ static void lease_get(data_t *data)
 	mgl.pad = 0;
 
 	/* Create a valid lease */
-	igt_assert_eq(make_lease(data, &lease), 0);
+	igt_assert_eq(make_lease(data), 0);
 
 	/* Get the number of objects */
 	mgl.count_objects = 0;
 	mgl.objects_ptr = 0;
-	igt_assert_eq(get_lease(lease.fd, &mgl), 0);
+	igt_assert_eq(get_lease(data->lease.fd, &mgl), 0);
 
 	/* Make sure it's 2 */
 	igt_assert_eq(mgl.count_objects, num_leased_obj);
@@ -735,7 +734,7 @@ static void lease_get(data_t *data)
 	/* Get the objects */
 	mgl.objects_ptr = (uint64_t) (uintptr_t) objects;
 
-	igt_assert_eq(get_lease(lease.fd, &mgl), 0);
+	igt_assert_eq(get_lease(data->lease.fd, &mgl), 0);
 
 	/* Make sure it's 2 */
 	igt_assert_eq(mgl.count_objects, num_leased_obj);
@@ -761,28 +760,27 @@ static void lease_get(data_t *data)
 
 	/* invalid pad */
 	mgl.pad = -1;
-	igt_assert_eq(get_lease(lease.fd, &mgl), -EINVAL);
+	igt_assert_eq(get_lease(data->lease.fd, &mgl), -EINVAL);
 	mgl.pad = 0;
 
 	/* invalid pointer */
 	mgl.objects_ptr = 0;
-	igt_assert_eq(get_lease(lease.fd, &mgl), -EFAULT);
+	igt_assert_eq(get_lease(data->lease.fd, &mgl), -EFAULT);
 
-	terminate_lease(&lease);
+	terminate_lease(data->lease.fd);
 }
 
 static void lease_unleased_crtc(data_t *data)
 {
-	lease_t lease;
 	enum pipe p;
 	uint32_t bad_crtc_id;
 	drmModeCrtc *crtc;
 	int ret;
 
 	/* Create a valid lease */
-	igt_assert_eq(make_lease(data, &lease), 0);
+	igt_assert_eq(make_lease(data), 0);
 
-	igt_display_require(&lease.display, lease.fd);
+	igt_display_require(&data->lease.display, data->lease.fd);
 
 	/* Find another CRTC that we don't control */
 	bad_crtc_id = 0;
@@ -798,36 +796,35 @@ static void lease_unleased_crtc(data_t *data)
 	igt_skip_on(bad_crtc_id == 0);
 
 	/* sanity check */
-	ret = drmModeSetCrtc(lease.fd, data->crtc_id, 0, 0, 0, NULL, 0, NULL);
+	ret = drmModeSetCrtc(data->lease.fd, data->crtc_id, 0, 0, 0, NULL, 0, NULL);
 	igt_assert_eq(ret, 0);
-	crtc = drmModeGetCrtc(lease.fd, data->crtc_id);
+	crtc = drmModeGetCrtc(data->lease.fd, data->crtc_id);
 	igt_assert(crtc);
 	drmModeFreeCrtc(crtc);
 
 	/* Attempt to use the unleased crtc id. We need raw ioctl to bypass the
 	 * igt_kms helpers.
 	 */
-	ret = drmModeSetCrtc(lease.fd, bad_crtc_id, 0, 0, 0, NULL, 0, NULL);
+	ret = drmModeSetCrtc(data->lease.fd, bad_crtc_id, 0, 0, 0, NULL, 0, NULL);
 	igt_assert_eq(ret, -ENOENT);
-	crtc = drmModeGetCrtc(lease.fd, bad_crtc_id);
+	crtc = drmModeGetCrtc(data->lease.fd, bad_crtc_id);
 	igt_assert(!crtc);
 	igt_assert_eq(errno, ENOENT);
 	drmModeFreeCrtc(crtc);
 
-	terminate_lease(&lease);
+	terminate_lease(data->lease.fd);
 }
 
 static void lease_unleased_connector(data_t *data)
 {
-	lease_t lease;
 	int o;
 	uint32_t bad_connector_id;
 	drmModeConnector *c;
 
 	/* Create a valid lease */
-	igt_assert_eq(make_lease(data, &lease), 0);
+	igt_assert_eq(make_lease(data), 0);
 
-	igt_display_require(&lease.display, lease.fd);
+	igt_display_require(&data->lease.display, data->lease.fd);
 
 	/* Find another connector that we don't control */
 	bad_connector_id = 0;
@@ -840,72 +837,70 @@ static void lease_unleased_connector(data_t *data)
 	igt_skip_on(bad_connector_id == 0);
 
 	/* sanity check */
-	c = drmModeGetConnector(lease.fd, data->connector_id);
+	c = drmModeGetConnector(data->lease.fd, data->connector_id);
 	igt_assert(c);
 
 	/* Attempt to use the unleased connector id. Note that the
 	 */
-	c = drmModeGetConnector(lease.fd, bad_connector_id);
+	c = drmModeGetConnector(data->lease.fd, bad_connector_id);
 	igt_assert(!c);
 	igt_assert_eq(errno, ENOENT);
 
-	terminate_lease(&lease);
+	terminate_lease(data->lease.fd);
 }
 
 /* Test revocation of lease */
 static void lease_revoke(data_t *data)
 {
-	lease_t lease;
 	struct drm_mode_revoke_lease mrl;
 	int ret;
 
 	/* Create a valid lease */
-	igt_assert_eq(make_lease(data, &lease), 0);
+	igt_assert_eq(make_lease(data), 0);
 
-	igt_display_require(&lease.display, lease.fd);
+	igt_display_require(&data->lease.display, data->lease.fd);
 
 	/* try to revoke an invalid lease */
 	mrl.lessee_id = 0;
 	igt_assert_eq(revoke_lease(data->master.fd, &mrl), -ENOENT);
 
 	/* try to revoke with the wrong fd */
-	mrl.lessee_id = lease.lessee_id;
-	igt_assert_eq(revoke_lease(lease.fd, &mrl), -EACCES);
+	mrl.lessee_id = data->lease.lessee_id;
+	igt_assert_eq(revoke_lease(data->lease.fd, &mrl), -EACCES);
 
 	/* Revoke the lease using the master fd */
-	mrl.lessee_id = lease.lessee_id;
+	mrl.lessee_id = data->lease.lessee_id;
 	igt_assert_eq(revoke_lease(data->master.fd, &mrl), 0);
 
 	/* Try to use the leased objects */
-	ret = prepare_crtc(&lease, data);
+	ret = prepare_crtc(data, false);
 
 	/* Ensure that the expected error is returned */
 	igt_assert_eq(ret, -ENOENT);
 
-	terminate_lease(&lease);
+	terminate_lease(data->lease.fd);
 
 	/* make sure the lease is gone */
-	mrl.lessee_id = lease.lessee_id;
+	mrl.lessee_id = data->lease.lessee_id;
 	igt_assert_eq(revoke_lease(data->master.fd, &mrl), -ENOENT);
 }
 
 /* Test leasing objects more than once */
 static void lease_again(data_t *data)
 {
-	lease_t lease_a, lease_b;
 
 	/* Create a valid lease */
-	igt_assert_eq(make_lease(data, &lease_a), 0);
+	igt_assert_eq(make_lease(data), 0);
 
 	/* Attempt to re-lease the same objects */
-	igt_assert_eq(make_lease(data, &lease_b), -EBUSY);
+	igt_assert_eq(make_lease(data), -EBUSY);
 
-	terminate_lease(&lease_a);
+	terminate_lease(data->lease.fd);
 
 	/* Now attempt to lease the same objects */
-	igt_assert_eq(make_lease(data, &lease_b), 0);
+	igt_assert_eq(make_lease(data), 0);
 
-	terminate_lease(&lease_b);
+	terminate_lease(data->lease.fd);
 }
 
 #define assert_unleased(ret) \
@@ -915,14 +910,13 @@ static void lease_again(data_t *data)
 /* Test leasing an invalid connector */
 static void lease_invalid_connector(data_t *data)
 {
-	lease_t lease;
 	uint32_t save_connector_id;
 	int ret;
 
 	/* Create an invalid lease */
 	save_connector_id = data->connector_id;
 	data->connector_id = 0xbaadf00d;
-	ret = make_lease(data, &lease);
+	ret = make_lease(data);
 	data->connector_id = save_connector_id;
 	assert_unleased(ret);
 }
@@ -930,28 +924,26 @@ static void lease_invalid_connector(data_t *data)
 /* Test leasing an invalid crtc */
 static void lease_invalid_crtc(data_t *data)
 {
-	lease_t lease;
 	uint32_t save_crtc_id;
 	int ret;
 
 	/* Create an invalid lease */
 	save_crtc_id = data->crtc_id;
 	data->crtc_id = 0xbaadf00d;
-	ret = make_lease(data, &lease);
+	ret = make_lease(data);
 	data->crtc_id = save_crtc_id;
 	assert_unleased(ret);
 }
 
 static void lease_invalid_plane(data_t *data)
 {
-	lease_t lease;
 	uint32_t save_plane_id;
 	int ret;
 
 	/* Create an invalid lease */
 	save_plane_id = data->plane_id;
 	data->plane_id = 0xbaadf00d;
-	ret = make_lease(data, &lease);
+	ret = make_lease(data);
 	data->plane_id = save_plane_id;
 	assert_unleased(ret);
 }
@@ -1393,6 +1385,7 @@ igt_main
 									DRM_PLANE_TYPE_PRIMARY)->drm_plane->plane_id;
 						f->func(&data);
 					}
+					terminate_lease(data.lease.fd);
 				}
 			}
 		}
-- 
2.25.1



More information about the igt-dev mailing list