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

Commit 940ac15e authored by Nagesh Subba Reddy's avatar Nagesh Subba Reddy Committed by Peter Liu
Browse files

msm: isp: Fix list corruption



List corruption can happen due to stale entries in the list. Also, list
corruption can happen if we add the same buffer twice to the shared list.

With this fix,
 1. Whenever we get a flush call, we clear all elements in the
     shared list.
 2. Changed the list_del_init api to list_del.
 3. Added bufq lock protection to msm_isp_update_put_buf_cnt

Change-Id: I0ea4bd8bf706c3871e2f280f80f6e70c37d9eb36
Signed-off-by: default avatarNagesh Reddy <nageshsreddy@codeaurora.org>
Signed-off-by: default avatarPeter Liu <pingchie@codeaurora.org>
parent 377d2dff
Loading
Loading
Loading
Loading
+18 −23
Original line number Original line Diff line number Diff line
@@ -221,7 +221,7 @@ static void msm_isp_unprepare_v4l2_buf(
			if (buf_pending->mapped_info == mapped_info) {
			if (buf_pending->mapped_info == mapped_info) {
				cam_smmu_put_phy_addr(iommu_hdl,
				cam_smmu_put_phy_addr(iommu_hdl,
							mapped_info->buf_fd);
							mapped_info->buf_fd);
				list_del_init(&buf_pending->list);
				list_del(&buf_pending->list);
				kfree(buf_pending);
				kfree(buf_pending);
				break;
				break;
			}
			}
@@ -425,7 +425,7 @@ static int msm_isp_get_buf(struct msm_isp_buf_mgr *buf_mgr, uint32_t id,
				temp_buf_info->buf_get_count++;
				temp_buf_info->buf_get_count++;
				if (temp_buf_info->buf_get_count ==
				if (temp_buf_info->buf_get_count ==
					bufq->buf_client_count)
					bufq->buf_client_count)
					list_del_init(
					list_del(
					&temp_buf_info->share_list);
					&temp_buf_info->share_list);
				if (temp_buf_info->buf_reuse_flag) {
				if (temp_buf_info->buf_reuse_flag) {
					kfree(temp_buf_info);
					kfree(temp_buf_info);
@@ -461,7 +461,7 @@ static int msm_isp_get_buf(struct msm_isp_buf_mgr *buf_mgr, uint32_t id,
						&& (mped_info_tmp1->paddr ==
						&& (mped_info_tmp1->paddr ==
						mped_info_tmp2->paddr)) {
						mped_info_tmp2->paddr)) {
						/* found one buf */
						/* found one buf */
						list_del_init(
						list_del(
							&temp_buf_info->list);
							&temp_buf_info->list);
						*buf_info = temp_buf_info;
						*buf_info = temp_buf_info;
						break;
						break;
@@ -581,12 +581,12 @@ static int msm_isp_put_buf(struct msm_isp_buf_mgr *buf_mgr,
		return rc;
		return rc;
	}
	}


	spin_lock_irqsave(&bufq->bufq_lock, flags);


	buf_info->buf_get_count = 0;
	buf_info->buf_get_count = 0;
	buf_info->buf_put_count = 0;
	buf_info->buf_put_count = 0;
	memset(buf_info->buf_used, 0, sizeof(buf_info->buf_used));
	memset(buf_info->buf_used, 0, sizeof(buf_info->buf_used));


	spin_lock_irqsave(&bufq->bufq_lock, flags);
	switch (buf_info->state) {
	switch (buf_info->state) {
	case MSM_ISP_BUFFER_STATE_PREPARED:
	case MSM_ISP_BUFFER_STATE_PREPARED:
		if (MSM_ISP_BUFFER_SRC_SCRATCH == BUF_SRC(bufq->stream_id))
		if (MSM_ISP_BUFFER_SRC_SCRATCH == BUF_SRC(bufq->stream_id))
@@ -707,7 +707,6 @@ static int msm_isp_update_put_buf_cnt(struct msm_isp_buf_mgr *buf_mgr,
		pr_warn("%s: Invalid state\n", __func__);
		pr_warn("%s: Invalid state\n", __func__);
	}
	}
	spin_unlock_irqrestore(&bufq->bufq_lock, flags);
	spin_unlock_irqrestore(&bufq->bufq_lock, flags);

	return 0;
	return 0;
}
}


@@ -767,7 +766,6 @@ static int msm_isp_flush_buf(struct msm_isp_buf_mgr *buf_mgr,
		uint32_t bufq_handle, enum msm_isp_buffer_flush_t flush_type)
		uint32_t bufq_handle, enum msm_isp_buffer_flush_t flush_type)
{
{
	int rc = -1, i;
	int rc = -1, i;
	unsigned long flags;
	struct msm_isp_bufq *bufq = NULL;
	struct msm_isp_bufq *bufq = NULL;
	struct msm_isp_buffer *buf_info = NULL;
	struct msm_isp_buffer *buf_info = NULL;


@@ -777,13 +775,14 @@ static int msm_isp_flush_buf(struct msm_isp_buf_mgr *buf_mgr,
		return rc;
		return rc;
	}
	}


	spin_lock(&bufq->bufq_lock);

	for (i = 0; i < bufq->num_bufs; i++) {
	for (i = 0; i < bufq->num_bufs; i++) {
		buf_info = msm_isp_get_buf_ptr(buf_mgr, bufq_handle, i);
		buf_info = msm_isp_get_buf_ptr(buf_mgr, bufq_handle, i);
		if (!buf_info) {
		if (!buf_info) {
			pr_err("%s: buf not found\n", __func__);
			pr_err("%s: buf not found\n", __func__);
			continue;
			continue;
		}
		}
		spin_lock_irqsave(&bufq->bufq_lock, flags);
		if (flush_type == MSM_ISP_BUFFER_FLUSH_DIVERTED &&
		if (flush_type == MSM_ISP_BUFFER_FLUSH_DIVERTED &&
			buf_info->state == MSM_ISP_BUFFER_STATE_DIVERTED) {
			buf_info->state == MSM_ISP_BUFFER_STATE_DIVERTED) {
			buf_info->state = MSM_ISP_BUFFER_STATE_QUEUED;
			buf_info->state = MSM_ISP_BUFFER_STATE_QUEUED;
@@ -793,24 +792,22 @@ static int msm_isp_flush_buf(struct msm_isp_buf_mgr *buf_mgr,
					__func__);
					__func__);
			} else if (buf_info->state ==
			} else if (buf_info->state ==
				MSM_ISP_BUFFER_STATE_DEQUEUED) {
				MSM_ISP_BUFFER_STATE_DEQUEUED) {
				if (buf_info->buf_get_count ==
					ISP_SHARE_BUF_CLIENT) {
				msm_isp_put_buf_unsafe(buf_mgr,
				msm_isp_put_buf_unsafe(buf_mgr,
					bufq_handle, buf_info->buf_idx);
					bufq_handle, buf_info->buf_idx);
				} else {
					buf_info->state =
						MSM_ISP_BUFFER_STATE_DEQUEUED;
					buf_info->buf_get_count = 0;
					buf_info->buf_put_count = 0;
					memset(buf_info->buf_used, 0,
						sizeof(uint8_t) * 2);
			}
			}
		}
		}
	}
	}


		spin_unlock_irqrestore(&bufq->bufq_lock, flags);
	if (bufq->buf_type == ISP_SHARE_BUF) {
		while (!list_empty(&bufq->share_head)) {
			buf_info = list_entry((&bufq->share_head)->next,
				typeof(*buf_info), share_list);
			list_del(&(buf_info->share_list));
			if (buf_info->buf_reuse_flag)
				kfree(buf_info);
		 }
		 }

	}
	spin_unlock(&bufq->bufq_lock);
	return 0;
	return 0;
}
}


@@ -855,7 +852,6 @@ static int msm_isp_buf_divert(struct msm_isp_buf_mgr *buf_mgr,
		buf_info->tv = tv;
		buf_info->tv = tv;
	}
	}
	spin_unlock_irqrestore(&bufq->bufq_lock, flags);
	spin_unlock_irqrestore(&bufq->bufq_lock, flags);

	return 0;
	return 0;
}
}


@@ -1019,7 +1015,6 @@ static int msm_isp_request_bufq(struct msm_isp_buf_mgr *buf_mgr,
		bufq->bufs[i].state = MSM_ISP_BUFFER_STATE_INITIALIZED;
		bufq->bufs[i].state = MSM_ISP_BUFFER_STATE_INITIALIZED;
		bufq->bufs[i].bufq_handle = bufq->bufq_handle;
		bufq->bufs[i].bufq_handle = bufq->bufq_handle;
		bufq->bufs[i].buf_idx = i;
		bufq->bufs[i].buf_idx = i;
		spin_lock_init(&bufq->bufs[i].lock);
	}
	}


	return 0;
	return 0;
+0 −1
Original line number Original line Diff line number Diff line
@@ -82,7 +82,6 @@ struct msm_isp_buffer {


	/*Vb2 buffer data*/
	/*Vb2 buffer data*/
	struct vb2_buffer *vb2_buf;
	struct vb2_buffer *vb2_buf;
	spinlock_t lock;


	/*Share buffer cache state*/
	/*Share buffer cache state*/
	struct list_head share_list;
	struct list_head share_list;
+1 −4
Original line number Original line Diff line number Diff line
@@ -76,7 +76,6 @@ static int32_t msm_isp_stats_buf_divert(struct vfe_device *vfe_dev,
	int32_t rc = 0, frame_id = 0, drop_buffer = 0;
	int32_t rc = 0, frame_id = 0, drop_buffer = 0;
	struct msm_isp_stats_event *stats_event = NULL;
	struct msm_isp_stats_event *stats_event = NULL;
	struct msm_isp_sw_framskip *sw_skip = NULL;
	struct msm_isp_sw_framskip *sw_skip = NULL;
	unsigned long flags;


	if (!vfe_dev || !done_buf || !ts || !buf_event || !stream_info ||
	if (!vfe_dev || !done_buf || !ts || !buf_event || !stream_info ||
		!comp_stats_type_mask) {
		!comp_stats_type_mask) {
@@ -106,7 +105,6 @@ static int32_t msm_isp_stats_buf_divert(struct vfe_device *vfe_dev,
		}
		}
	}
	}


	spin_lock_irqsave(&done_buf->lock, flags);
	rc = vfe_dev->buf_mgr->ops->buf_divert(
	rc = vfe_dev->buf_mgr->ops->buf_divert(
		vfe_dev->buf_mgr, done_buf->bufq_handle,
		vfe_dev->buf_mgr, done_buf->bufq_handle,
		done_buf->buf_idx, &ts->buf_time,
		done_buf->buf_idx, &ts->buf_time,
@@ -115,15 +113,14 @@ static int32_t msm_isp_stats_buf_divert(struct vfe_device *vfe_dev,
		ISP_DBG("%s: vfe_id %d buf_id %d bufq %x put_cnt 1\n", __func__,
		ISP_DBG("%s: vfe_id %d buf_id %d bufq %x put_cnt 1\n", __func__,
			vfe_dev->pdev->id, done_buf->buf_idx,
			vfe_dev->pdev->id, done_buf->buf_idx,
			done_buf->bufq_handle);
			done_buf->bufq_handle);
		spin_unlock_irqrestore(&done_buf->lock, flags);
		*comp_stats_type_mask |=
		*comp_stats_type_mask |=
			1 << stream_info->stats_type;
			1 << stream_info->stats_type;
		stats_event->stats_buf_idxs
		stats_event->stats_buf_idxs
			[stream_info->stats_type] =
			[stream_info->stats_type] =
			done_buf->buf_idx;
			done_buf->buf_idx;
		done_buf->frame_id = frame_id;
		return rc;
		return rc;
	}
	}
	spin_unlock_irqrestore(&done_buf->lock, flags);


	if (drop_buffer) {
	if (drop_buffer) {
		vfe_dev->buf_mgr->ops->put_buf(
		vfe_dev->buf_mgr->ops->put_buf(