Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit eb42ea6d authored by Maarten Lankhorst's avatar Maarten Lankhorst
Browse files

drm/crc: Handle opening and closing crc better 



When I was doing a grep . -r /sys/kernel/debug/dri/0 I noticed a WARN
appearing when I aborted the grep with ^C.

After investigating I've also noticed that the error handling was
lacking and there are race conditions involving multiple calls to
open/close simultaneously.

Fix this by setting the opened flag first and using crc->entries to
decide when crc can be collected.

Also call unset crc source before cleaning up, this way there is
no race with a future open().

This patch has been tested with all the tests in igt with CRC in their
name.

Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170621110007.11674-1-maarten.lankhorst@linux.intel.com


Reviewed-by: default avatarTomeu Vizoso <tomeu.vizoso@collabora.com>
[mlankhorst: Add description that this patch has been tested with IGT,
based on tomeu's feedback]
parent ec878c07

drivers/gpu/drm/drm_debugfs_crc.c

+31 −15
Original line number Diff line number Diff line
@@ -136,21 +136,38 @@ static int crtc_crc_data_count(struct drm_crtc_crc *crc) 
	return CIRC_CNT(crc->head, crc->tail, DRM_CRC_ENTRIES_NR);

}



static void crtc_crc_cleanup(struct drm_crtc_crc *crc)

{

	kfree(crc->entries);

	crc->entries = NULL;

	crc->head = 0;

	crc->tail = 0;

	crc->values_cnt = 0;

	crc->opened = false;

}



static int crtc_crc_open(struct inode *inode, struct file *filep)

{

	struct drm_crtc *crtc = inode->i_private;

	struct drm_crtc_crc *crc = &crtc->crc;

	struct drm_crtc_crc_entry *entries = NULL;

	size_t values_cnt;

	int ret;

	int ret = 0;



	if (crc->opened)

		return -EBUSY;

	spin_lock_irq(&crc->lock);

	if (!crc->opened)

		crc->opened = true;

	else

		ret = -EBUSY;

	spin_unlock_irq(&crc->lock);



	ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt);

	if (ret)

		return ret;



	ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt);

	if (ret)

		goto err;



	if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) {

		ret = -EINVAL;

		goto err_disable;
@@ -170,7 +187,6 @@ static int crtc_crc_open(struct inode *inode, struct file *filep) 
	spin_lock_irq(&crc->lock);

	crc->entries = entries;

	crc->values_cnt = values_cnt;

	crc->opened = true;



	/*

	 * Only return once we got a first frame, so userspace doesn't have to
@@ -182,12 +198,17 @@ static int crtc_crc_open(struct inode *inode, struct file *filep) 
						crc->lock);

	spin_unlock_irq(&crc->lock);



	WARN_ON(ret);

	if (ret)

		goto err_disable;



	return 0;



err_disable:

	crtc->funcs->set_crc_source(crtc, NULL, &values_cnt);

err:

	spin_lock_irq(&crc->lock);

	crtc_crc_cleanup(crc);

	spin_unlock_irq(&crc->lock);

	return ret;

}
@@ -197,17 +218,12 @@ static int crtc_crc_release(struct inode *inode, struct file *filep) 
	struct drm_crtc_crc *crc = &crtc->crc;

	size_t values_cnt;



	crtc->funcs->set_crc_source(crtc, NULL, &values_cnt);



	spin_lock_irq(&crc->lock);

	kfree(crc->entries);

	crc->entries = NULL;

	crc->head = 0;

	crc->tail = 0;

	crc->values_cnt = 0;

	crc->opened = false;

	crtc_crc_cleanup(crc);

	spin_unlock_irq(&crc->lock);



	crtc->funcs->set_crc_source(crtc, NULL, &values_cnt);



	return 0;

}
@@ -334,7 +350,7 @@ int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, 
	spin_lock(&crc->lock);



	/* Caller may not have noticed yet that userspace has stopped reading */

	if (!crc->opened) {

	if (!crc->entries) {

		spin_unlock(&crc->lock);

		return -EINVAL;

	}