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

Commit f950a20c authored by Arun Menon's avatar Arun Menon
Browse files

msm: vidc: Fix race condition during concurrent sessions 



The core->instances list structure is guarded by
mutex core->lock, whenever a new video instance is added
or deleted from the list. However this list was accessed
from certain functions without acquiring the lock, which
resulted in race condition, causing the device to crash.
Also clean up the core locking mechanism.

Change-Id: I6f431bc381d54948611783e19fac853c83a87203
Signed-off-by: default avatarArun Menon <avmenon@codeaurora.org>
parent 71a6aa0a

drivers/media/platform/msm/vidc/msm_v4l2_vidc.c

+0 −1
Original line number Diff line number Diff line
@@ -322,7 +322,6 @@ static int msm_vidc_initialize_core(struct platform_device *pdev, 
	}



	INIT_LIST_HEAD(&core->instances);

	mutex_init(&core->sync_lock);

	mutex_init(&core->lock);



	core->state = VIDC_CORE_UNINIT;

drivers/media/platform/msm/vidc/msm_vdec.c

+2 −6
Original line number Diff line number Diff line
@@ -1090,9 +1090,8 @@ int msm_vdec_s_parm(struct msm_vidc_inst *inst, struct v4l2_streamparm *a) 
		dprintk(VIDC_PROF, "reported fps changed for %p: %d->%d\n",

				inst, inst->prop.fps, fps);

		inst->prop.fps = fps;

		mutex_lock(&inst->core->sync_lock);



		msm_comm_scale_clocks_and_bus(inst);

		mutex_unlock(&inst->core->sync_lock);

	}

exit:

	return rc;
@@ -1446,9 +1445,8 @@ static inline int start_streaming(struct msm_vidc_inst *inst) 
			goto fail_start;

		}

	}

	mutex_lock(&inst->core->sync_lock);



	msm_comm_scale_clocks_and_bus(inst);

	mutex_unlock(&inst->core->sync_lock);



	rc = msm_comm_try_state(inst, MSM_VIDC_START_DONE);

	if (rc) {
@@ -1555,9 +1553,7 @@ static int msm_vdec_stop_streaming(struct vb2_queue *q) 
		break;

	}



	mutex_lock(&inst->core->sync_lock);

	msm_comm_scale_clocks_and_bus(inst);

	mutex_unlock(&inst->core->sync_lock);



	if (rc)

		dprintk(VIDC_ERR,

drivers/media/platform/msm/vidc/msm_venc.c

+1 −6
Original line number Diff line number Diff line
@@ -1213,9 +1213,7 @@ static inline int start_streaming(struct msm_vidc_inst *inst) 
		goto fail_start;

	}



	mutex_lock(&inst->core->sync_lock);

	msm_comm_scale_clocks_and_bus(inst);

	mutex_unlock(&inst->core->sync_lock);



	rc = msm_comm_try_state(inst, MSM_VIDC_START_DONE);

	if (rc) {
@@ -1292,9 +1290,7 @@ static int msm_venc_stop_streaming(struct vb2_queue *q) 
		break;

	}



	mutex_lock(&inst->core->sync_lock);

	msm_comm_scale_clocks_and_bus(inst);

	mutex_unlock(&inst->core->sync_lock);



	if (rc)

		dprintk(VIDC_ERR,
@@ -2774,9 +2770,8 @@ int msm_venc_s_parm(struct msm_vidc_inst *inst, struct v4l2_streamparm *a) 
			dprintk(VIDC_WARN,

				"Failed to set frame rate %d\n", rc);

		}

		mutex_lock(&inst->core->sync_lock);



		msm_comm_scale_clocks_and_bus(inst);

		mutex_unlock(&inst->core->sync_lock);

	}

exit:

	return rc;

drivers/media/platform/msm/vidc/msm_vidc.c

+4 −4
Original line number Diff line number Diff line
@@ -1313,9 +1313,9 @@ void *msm_vidc_open(int core_id, int session_type) 


	setup_event_queue(inst, &core->vdev[session_type].vdev);



	mutex_lock(&core->sync_lock);

	mutex_lock(&core->lock);

	list_add_tail(&inst->list, &core->instances);

	mutex_unlock(&core->sync_lock);

	mutex_unlock(&core->lock);

	return inst;

fail_init:

	vb2_queue_release(&inst->bufq[OUTPUT_PORT].vb2_bufq);
@@ -1412,13 +1412,13 @@ int msm_vidc_close(void *instance) 
	}



	core = inst->core;

	mutex_lock(&core->sync_lock);

	mutex_lock(&core->lock);

	list_for_each_safe(ptr, next, &core->instances) {

		temp = list_entry(ptr, struct msm_vidc_inst, list);

		if (temp == inst)

			list_del(&inst->list);

	}

	mutex_unlock(&core->sync_lock);

	mutex_unlock(&core->lock);



	if (inst->session_type == MSM_VIDC_DECODER)

		msm_vdec_ctrl_deinit(inst);

drivers/media/platform/msm/vidc/msm_vidc_common.c

+45 −38
Original line number Diff line number Diff line
@@ -133,6 +133,7 @@ static int msm_comm_get_load(struct msm_vidc_core *core, 
		return -EINVAL;

	}



	mutex_lock(&core->lock);

	list_for_each_entry(inst, &core->instances, list) {

		if (inst->session_type != type)

			continue;
@@ -141,6 +142,7 @@ static int msm_comm_get_load(struct msm_vidc_core *core, 
		num_mbs_per_sec += msm_comm_get_inst_load(inst, quirks);

		mutex_unlock(&inst->lock);

	}

	mutex_unlock(&core->lock);



	return num_mbs_per_sec;

}
@@ -235,6 +237,7 @@ static int msm_comm_vote_bus(struct msm_vidc_core *core) 
		return -EINVAL;

	}



	mutex_lock(&core->lock);

	list_for_each_entry(inst, &core->instances, list) {

		++vote_data_count;

	}
@@ -243,7 +246,8 @@ static int msm_comm_vote_bus(struct msm_vidc_core *core) 
			GFP_TEMPORARY);

	if (!vote_data) {

		dprintk(VIDC_ERR, "%s: failed to allocate memory\n", __func__);

		return -ENOMEM;

		rc = -ENOMEM;

		goto fail_alloc;

	}



	list_for_each_entry(inst, &core->instances, list) {
@@ -265,6 +269,7 @@ static int msm_comm_vote_bus(struct msm_vidc_core *core) 


		i++;

	}

	mutex_unlock(&core->lock);



	rc = call_hfi_op(hdev, vote_bus, hdev->hfi_device_data, vote_data,

			vote_data_count);
@@ -273,6 +278,10 @@ static int msm_comm_vote_bus(struct msm_vidc_core *core) 


	kfree(vote_data);

	return rc;



fail_alloc:

	mutex_unlock(&core->lock);

	return rc;

}



static void msm_comm_unvote_buses(struct msm_vidc_core *core)
@@ -1013,13 +1022,12 @@ void hw_sys_error_handler(struct work_struct *work) 
	core = handler->core;

	hdev = core->device;



	mutex_lock(&core->sync_lock);

	mutex_lock(&core->lock);

	/*

	* Restart the firmware to bring out of bad state.

	*/

	if ((core->state == VIDC_CORE_INVALID) &&

		hdev->resurrect_fw) {

		mutex_lock(&core->lock);

		rc = call_hfi_op(hdev, resurrect_fw,

				hdev->hfi_device_data);

		if (rc) {
@@ -1028,12 +1036,11 @@ void hw_sys_error_handler(struct work_struct *work) 
				__func__, rc);

		}

		core->state = VIDC_CORE_LOADED;

		mutex_unlock(&core->lock);

	} else {

		dprintk(VIDC_DBG,

			"fw unloaded after sys error, no need to resurrect\n");

	}

	mutex_unlock(&core->sync_lock);

	mutex_unlock(&core->lock);



exit:

	/* free sys error handler, allocated in handle_sys_err */
@@ -1066,14 +1073,11 @@ static void handle_sys_error(enum command_response cmd, void *data) 
	dprintk(VIDC_WARN, "SYS_ERROR %d received for core %p\n", cmd, core);

	mutex_lock(&core->lock);

	core->state = VIDC_CORE_INVALID;

	mutex_unlock(&core->lock);





	/*

	* 1. Delete each instance session from hfi list

	* 2. Notify all clients about hardware error.

	*/

	mutex_lock(&core->sync_lock);

	list_for_each_entry(inst, &core->instances,

			list) {

		mutex_lock(&inst->lock);
@@ -1095,7 +1099,7 @@ static void handle_sys_error(enum command_response cmd, void *data) 
		msm_vidc_queue_v4l2_event(inst,

				V4L2_EVENT_MSM_VIDC_SYS_ERROR);

	}

	mutex_unlock(&core->sync_lock);

	mutex_unlock(&core->lock);





	handler = kzalloc(sizeof(*handler), GFP_KERNEL);
@@ -1720,7 +1724,7 @@ static int msm_comm_init_core_done(struct msm_vidc_inst *inst) 
{

	struct msm_vidc_core *core = inst->core;

	int rc = 0;

	mutex_lock(&core->sync_lock);

	mutex_lock(&core->lock);

	if (core->state >= VIDC_CORE_INIT_DONE) {

		dprintk(VIDC_INFO, "Video core: %d is already in state: %d\n",

				core->id, core->state);
@@ -1736,16 +1740,14 @@ static int msm_comm_init_core_done(struct msm_vidc_inst *inst) 
		rc = -EIO;

		goto exit;

	} else {

		mutex_lock(&core->lock);

		core->state = VIDC_CORE_INIT_DONE;

		mutex_unlock(&core->lock);

	}

	dprintk(VIDC_DBG, "SYS_INIT_DONE!!!\n");

core_already_inited:

	change_inst_state(inst, MSM_VIDC_CORE_INIT_DONE);

	rc = 0;

exit:

	mutex_unlock(&core->sync_lock);

	mutex_unlock(&core->lock);

	return rc;

}
@@ -1759,12 +1761,13 @@ static int msm_comm_init_core(struct msm_vidc_inst *inst) 
		return -EINVAL;

	hdev = core->device;



	mutex_lock(&core->sync_lock);

	mutex_lock(&core->lock);

	if (core->state >= VIDC_CORE_INIT) {

		dprintk(VIDC_INFO, "Video core: %d is already in state: %d\n",

				core->id, core->state);

		goto core_already_inited;

	}

	mutex_unlock(&core->lock);



	rc = msm_comm_vote_bus(core);

	if (rc) {
@@ -1772,13 +1775,16 @@ static int msm_comm_init_core(struct msm_vidc_inst *inst) 
		goto fail_vote_bus;

	}



	mutex_lock(&core->lock);

	if (core->state < VIDC_CORE_LOADED) {

		rc = call_hfi_op(hdev, load_fw, hdev->hfi_device_data);

		if (rc) {

			dprintk(VIDC_ERR, "Failed to load video firmware\n");

			goto fail_load_fw;

		}

		core->state = VIDC_CORE_LOADED;

	}

	mutex_unlock(&core->lock);



	rc = msm_comm_scale_clocks(core);

	if (rc) {
@@ -1786,25 +1792,29 @@ static int msm_comm_init_core(struct msm_vidc_inst *inst) 
		goto fail_core_init;

	}



	init_completion(&core->completions[SYS_MSG_INDEX(SYS_INIT_DONE)]);

	mutex_lock(&core->lock);

	if (core->state == VIDC_CORE_LOADED) {

		init_completion(&core->completions

			[SYS_MSG_INDEX(SYS_INIT_DONE)]);

		rc = call_hfi_op(hdev, core_init, hdev->hfi_device_data);

		if (rc) {

		dprintk(VIDC_ERR, "Failed to init core, id = %d\n", core->id);

			dprintk(VIDC_ERR, "Failed to init core, id = %d\n",

				core->id);

			goto fail_core_init;

		}

	mutex_lock(&core->lock);

		core->state = VIDC_CORE_INIT;

	mutex_unlock(&core->lock);

	}



core_already_inited:

	change_inst_state(inst, MSM_VIDC_CORE_INIT);

	mutex_unlock(&core->sync_lock);

	mutex_unlock(&core->lock);

	return rc;

fail_core_init:

	call_hfi_op(hdev, unload_fw, hdev->hfi_device_data);

fail_load_fw:

	msm_comm_unvote_buses(core);

fail_vote_bus:

	mutex_unlock(&core->sync_lock);

	mutex_unlock(&core->lock);

	return rc;

}
@@ -1822,14 +1832,17 @@ static int msm_vidc_deinit_core(struct msm_vidc_inst *inst) 
	core = inst->core;

	hdev = core->device;



	mutex_lock(&core->sync_lock);

	mutex_lock(&core->lock);

	if (core->state == VIDC_CORE_UNINIT) {

		dprintk(VIDC_INFO, "Video core: %d is already in state: %d\n",

				core->id, core->state);

		goto core_already_uninited;

	}

	mutex_unlock(&core->lock);



	msm_comm_scale_clocks_and_bus(inst);



	mutex_lock(&core->lock);

	if (list_empty(&core->instances)) {

		if (core->state > VIDC_CORE_INIT) {

			if (core->resources.ocmem_size) {
@@ -1848,9 +1861,9 @@ static int msm_vidc_deinit_core(struct msm_vidc_inst *inst) 
				goto exit;

			}

		}

		mutex_lock(&core->lock);



		core->state = VIDC_CORE_UNINIT;

		mutex_unlock(&core->lock);



		call_hfi_op(hdev, unload_fw, hdev->hfi_device_data);

		msm_comm_unvote_buses(core);

	}
@@ -1858,7 +1871,7 @@ static int msm_vidc_deinit_core(struct msm_vidc_inst *inst) 
core_already_uninited:

	change_inst_state(inst, MSM_VIDC_CORE_UNINIT);

exit:

	mutex_unlock(&core->sync_lock);

	mutex_unlock(&core->lock);

	return rc;

}
@@ -1920,6 +1933,7 @@ static void msm_vidc_print_running_insts(struct msm_vidc_core *core) 
	dprintk(VIDC_ERR, "%4s|%4s|%4s|%4s|%4s\n",

			"type", "w", "h", "fps", "prop");



	mutex_lock(&core->lock);

	list_for_each_entry(temp, &core->instances, list) {

		mutex_lock(&temp->lock);

		if (temp->state >= MSM_VIDC_OPEN_DONE &&
@@ -1940,6 +1954,7 @@ static void msm_vidc_print_running_insts(struct msm_vidc_core *core) 
		}

		mutex_unlock(&temp->lock);

	}

	mutex_unlock(&core->lock);

}



static int msm_vidc_load_resources(int flipped_state,
@@ -1971,11 +1986,9 @@ static int msm_vidc_load_resources(int flipped_state, 
		return -EINVAL;

	}



	mutex_lock(&core->sync_lock);

	num_mbs_per_sec =

		msm_comm_get_load(core, MSM_VIDC_DECODER, quirks) +

		msm_comm_get_load(core, MSM_VIDC_ENCODER, quirks);

	mutex_unlock(&core->sync_lock);



	if (num_mbs_per_sec > core->resources.max_load) {

		dprintk(VIDC_ERR, "HW is overloaded, needed: %d max: %d\n",
@@ -1993,19 +2006,17 @@ static int msm_vidc_load_resources(int flipped_state, 
		goto exit;

	}

	if (core->resources.ocmem_size) {

		mutex_lock(&core->sync_lock);

		height = max(inst->prop.height[CAPTURE_PORT],

			inst->prop.height[OUTPUT_PORT]);

		width = max(inst->prop.width[CAPTURE_PORT],

			inst->prop.width[OUTPUT_PORT]);

		rc = msm_comm_vote_bus(core);

		mutex_unlock(&core->sync_lock);

		if (!rc) {

			mutex_lock(&core->sync_lock);

			mutex_lock(&core->lock);

			rc = call_hfi_op(hdev, alloc_ocmem,

					hdev->hfi_device_data,

					core->resources.ocmem_size);

			mutex_unlock(&core->sync_lock);

			mutex_unlock(&core->lock);

			if (rc) {

				dprintk(VIDC_WARN,

				"Failed to allocate OCMEM. Performance will be impacted\n");
@@ -3538,20 +3549,16 @@ static int msm_vidc_load_supported(struct msm_vidc_inst *inst) 
		LOAD_CALC_IGNORE_THUMBNAIL_LOAD;



	if (inst->state == MSM_VIDC_OPEN_DONE) {

		mutex_lock(&inst->core->sync_lock);

		num_mbs_per_sec = msm_comm_get_load(inst->core,

					MSM_VIDC_DECODER, quirks);

		num_mbs_per_sec += msm_comm_get_load(inst->core,

					MSM_VIDC_ENCODER, quirks);

		mutex_unlock(&inst->core->sync_lock);

		if (num_mbs_per_sec > inst->core->resources.max_load) {

			dprintk(VIDC_ERR,

				"H/W is overloaded. needed: %d max: %d\n",

				num_mbs_per_sec,

				inst->core->resources.max_load);

			mutex_lock(&inst->sync_lock);

			msm_vidc_print_running_insts(inst->core);

			mutex_unlock(&inst->sync_lock);

			return -EINVAL;

		}

	}