msm: vidc: fix deadlock between queue and flush buffer handling

qbuf ioctl acquired bufq[port].lock in one thread and flush
call acquired registeredbufs.lock in another thread. So
thread-1 is waiting for registeredbufs.lock & thread-2 is
waiting for bufq[port].lock i.e leading to deadlock. So
added change to avoid above mentioned deadlock.

Change-Id: Ie21984fdb562ca7a09f801f036f3a78429ceab94
Signed-off-by: Govindaraj Rajagopal <grajagop@codeaurora.org>
This commit is contained in:
Govindaraj Rajagopal 2020-06-24 20:31:31 +05:30
commit 7b3b1524c4
4 changed files with 49 additions and 38 deletions

View file

@ -380,13 +380,21 @@ int msm_vidc_qbuf(void *instance, struct media_device *mdev,
return -EINVAL;
}
q = msm_comm_get_vb2q(inst, b->type);
if (!q) {
s_vpr_e(inst->sid,
"Failed to find buffer queue. type %d\n", b->type);
return -EINVAL;
}
mutex_lock(&q->lock);
if ((inst->out_flush && b->type == OUTPUT_MPLANE) || inst->in_flush) {
s_vpr_e(inst->sid,
"%s: in flush, discarding qbuf, type %u, index %u\n",
__func__, b->type, b->index);
return -EINVAL;
rc = -EINVAL;
goto unlock;
}
inst->last_qbuf_time_ns = ktime_get_ns();
for (i = 0; i < b->length; i++) {
@ -409,7 +417,8 @@ int msm_vidc_qbuf(void *instance, struct media_device *mdev,
0, inst->sid);
if (rc) {
s_vpr_e(inst->sid, "Failed to store input tag");
return -EINVAL;
rc = -EINVAL;
goto unlock;
}
}
@ -430,7 +439,7 @@ int msm_vidc_qbuf(void *instance, struct media_device *mdev,
rc = msm_comm_store_timestamp(inst, timestamp_us);
if (rc)
return rc;
goto unlock;
inst->clk_data.frame_rate = msm_comm_get_max_framerate(inst);
}
if (is_encode_session(inst) && b->type == INPUT_MPLANE) {
@ -440,21 +449,14 @@ int msm_vidc_qbuf(void *instance, struct media_device *mdev,
rc = msm_venc_store_timestamp(inst, timestamp_us);
if (rc)
return rc;
goto unlock;
}
q = msm_comm_get_vb2q(inst, b->type);
if (!q) {
s_vpr_e(inst->sid,
"Failed to find buffer queue. type %d\n", b->type);
return -EINVAL;
}
mutex_lock(&q->lock);
rc = vb2_qbuf(&q->vb2_bufq, mdev, b);
mutex_unlock(&q->lock);
if (rc)
s_vpr_e(inst->sid, "Failed to qbuf, %d\n", rc);
unlock:
mutex_unlock(&q->lock);
return rc;
}
@ -1485,7 +1487,6 @@ void *msm_vidc_open(int core_id, int session_type)
mutex_init(&inst->bufq[OUTPUT_PORT].lock);
mutex_init(&inst->bufq[INPUT_PORT].lock);
mutex_init(&inst->lock);
mutex_init(&inst->flush_lock);
mutex_init(&inst->ubwc_stats_lock);
INIT_MSM_VIDC_LIST(&inst->scratchbufs);
@ -1596,7 +1597,6 @@ fail_bufq_capture:
mutex_destroy(&inst->bufq[OUTPUT_PORT].lock);
mutex_destroy(&inst->bufq[INPUT_PORT].lock);
mutex_destroy(&inst->lock);
mutex_destroy(&inst->flush_lock);
DEINIT_MSM_VIDC_LIST(&inst->scratchbufs);
DEINIT_MSM_VIDC_LIST(&inst->persistbufs);
@ -1742,7 +1742,6 @@ int msm_vidc_destroy(struct msm_vidc_inst *inst)
mutex_destroy(&inst->bufq[OUTPUT_PORT].lock);
mutex_destroy(&inst->bufq[INPUT_PORT].lock);
mutex_destroy(&inst->lock);
mutex_destroy(&inst->flush_lock);
msm_vidc_debugfs_deinit_inst(inst);

View file

@ -2028,7 +2028,10 @@ static void handle_session_flush(enum hal_command_response cmd, void *data)
return;
}
mutex_lock(&inst->flush_lock);
if (response->data.flush_type & HAL_FLUSH_INPUT)
mutex_lock(&inst->bufq[INPUT_PORT].lock);
if (response->data.flush_type & HAL_FLUSH_OUTPUT)
mutex_lock(&inst->bufq[OUTPUT_PORT].lock);
if (msm_comm_get_stream_output_mode(inst) ==
HAL_VIDEO_DECODER_SECONDARY) {
@ -2078,7 +2081,10 @@ static void handle_session_flush(enum hal_command_response cmd, void *data)
v4l2_event_queue_fh(&inst->event_handler, &flush_event);
exit:
mutex_unlock(&inst->flush_lock);
if (response->data.flush_type & HAL_FLUSH_OUTPUT)
mutex_unlock(&inst->bufq[OUTPUT_PORT].lock);
if (response->data.flush_type & HAL_FLUSH_INPUT)
mutex_unlock(&inst->bufq[INPUT_PORT].lock);
s_vpr_l(inst->sid, "handled: SESSION_FLUSH_DONE\n");
put_inst(inst);
}
@ -2290,7 +2296,7 @@ struct vb2_buffer *msm_comm_get_vb_using_vidc_buffer(
return NULL;
}
mutex_lock(&inst->bufq[port].lock);
WARN_ON(!mutex_is_locked(&inst->bufq[port].lock));
found = false;
q = &inst->bufq[port].vb2_bufq;
if (!q->streaming) {
@ -2306,7 +2312,6 @@ struct vb2_buffer *msm_comm_get_vb_using_vidc_buffer(
}
}
unlock:
mutex_unlock(&inst->bufq[port].lock);
if (!found) {
print_vidc_buffer(VIDC_ERR, "vb2 not found for", inst, mbuf);
return NULL;
@ -2321,6 +2326,7 @@ int msm_comm_vb2_buffer_done(struct msm_vidc_inst *inst,
struct vb2_buffer *vb2;
struct vb2_v4l2_buffer *vbuf;
u32 i, port;
int rc = 0;
if (!inst || !mbuf) {
d_vpr_e("%s: invalid params %pK %pK\n",
@ -2335,16 +2341,19 @@ int msm_comm_vb2_buffer_done(struct msm_vidc_inst *inst,
else
return -EINVAL;
vb2 = msm_comm_get_vb_using_vidc_buffer(inst, mbuf);
if (!vb2)
return -EINVAL;
/*
* access vb2 buffer under q->lock and if streaming only to
* ensure the buffer was not free'd by vb2 framework while
* we are accessing it here.
*/
mutex_lock(&inst->bufq[port].lock);
vb2 = msm_comm_get_vb_using_vidc_buffer(inst, mbuf);
if (!vb2) {
s_vpr_e(inst->sid, "%s: port %d buffer not found\n",
__func__, port);
rc = -EINVAL;
goto unlock;
}
if (inst->bufq[port].vb2_bufq.streaming) {
vbuf = to_vb2_v4l2_buffer(vb2);
vbuf->flags = mbuf->vvb.flags;
@ -2360,9 +2369,10 @@ int msm_comm_vb2_buffer_done(struct msm_vidc_inst *inst,
s_vpr_e(inst->sid, "%s: port %d is not streaming\n",
__func__, port);
}
unlock:
mutex_unlock(&inst->bufq[port].lock);
return 0;
return rc;
}
static bool is_eos_buffer(struct msm_vidc_inst *inst, u32 device_addr)
@ -5516,7 +5526,6 @@ int msm_comm_flush(struct msm_vidc_inst *inst, u32 flags)
ip_flush = !!(flags & V4L2_CMD_FLUSH_OUTPUT);
op_flush = !!(flags & V4L2_CMD_FLUSH_CAPTURE);
if (ip_flush && !op_flush) {
s_vpr_e(inst->sid,
"Input only flush not supported, making it flush all\n");
@ -5539,7 +5548,10 @@ int msm_comm_flush(struct msm_vidc_inst *inst, u32 flags)
goto exit;
}
mutex_lock(&inst->flush_lock);
if (ip_flush)
mutex_lock(&inst->bufq[INPUT_PORT].lock);
if (op_flush)
mutex_lock(&inst->bufq[OUTPUT_PORT].lock);
/* enable in flush */
inst->in_flush = ip_flush;
inst->out_flush = op_flush;
@ -5595,7 +5607,10 @@ int msm_comm_flush(struct msm_vidc_inst *inst, u32 flags)
rc = call_hfi_op(hdev, session_flush, inst->session,
HAL_FLUSH_OUTPUT);
}
mutex_unlock(&inst->flush_lock);
if (op_flush)
mutex_unlock(&inst->bufq[OUTPUT_PORT].lock);
if (ip_flush)
mutex_unlock(&inst->bufq[INPUT_PORT].lock);
if (rc) {
s_vpr_e(inst->sid,
"Sending flush to firmware failed, flush out all buffers\n");
@ -6661,7 +6676,6 @@ int msm_comm_flush_vidc_buffer(struct msm_vidc_inst *inst,
else
return -EINVAL;
mutex_lock(&inst->bufq[port].lock);
if (inst->bufq[port].vb2_bufq.streaming) {
vb->planes[0].bytesused = 0;
vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
@ -6669,7 +6683,6 @@ int msm_comm_flush_vidc_buffer(struct msm_vidc_inst *inst,
s_vpr_e(inst->sid, "%s: port %d is not streaming\n",
__func__, port);
}
mutex_unlock(&inst->bufq[port].lock);
return 0;
}
@ -7015,7 +7028,7 @@ void handle_release_buffer_reference(struct msm_vidc_inst *inst,
unsigned int i = 0;
u32 planes[VIDEO_MAX_PLANES] = {0};
mutex_lock(&inst->flush_lock);
mutex_lock(&inst->bufq[OUTPUT_PORT].lock);
mutex_lock(&inst->registeredbufs.lock);
found = false;
/* check if mbuf was not removed by any chance */
@ -7104,7 +7117,7 @@ unlock:
print_vidc_buffer(VIDC_ERR,
"rbr qbuf failed", inst, mbuf);
}
mutex_unlock(&inst->flush_lock);
mutex_unlock(&inst->bufq[OUTPUT_PORT].lock);
}
int msm_comm_unmap_vidc_buffer(struct msm_vidc_inst *inst,

View file

@ -494,7 +494,7 @@ struct msm_vidc_inst_smem_ops {
struct msm_vidc_inst {
struct list_head list;
struct mutex sync_lock, lock, flush_lock;
struct mutex sync_lock, lock;
struct msm_vidc_core *core;
enum session_type session_type;
void *session;

View file

@ -384,10 +384,9 @@ struct hal_fw_info {
};
enum hal_flush {
HAL_FLUSH_INPUT,
HAL_FLUSH_OUTPUT,
HAL_FLUSH_ALL,
HAL_UNUSED_FLUSH = 0x10000000,
HAL_FLUSH_INPUT = BIT(0),
HAL_FLUSH_OUTPUT = BIT(1),
HAL_FLUSH_ALL = HAL_FLUSH_INPUT | HAL_FLUSH_OUTPUT,
};
enum hal_event_type {