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

Commit a0694e41 authored by Junzhe Zou's avatar Junzhe Zou Committed by Gerrit - the friendly Code Review server
Browse files

msm: camera: isp: acquire tasklet cmd before processing top half



If enqueue_tasklet fails, the payload is leaked because the release
happens in bottom_half which will never be called. Acquire tasklet
cmd before processing IRQ so that no resource is allocated unless
IRQ can be scheduled to tasklet.

Change-Id: I7c38189055c50300ca643a8e94f16a0598b435ee
Signed-off-by: default avatarJunzhe Zou <jnzhezou@codeaurora.org>
parent 09356118
Loading
Loading
Loading
Loading
+48 −41
Original line number Diff line number Diff line
@@ -70,25 +70,27 @@ struct cam_tasklet_info {
	void                              *ctx_priv;
};

/**
 * cam_tasklet_get_cmd()
 *
 * @brief:              Get free cmd from tasklet
 *
 * @tasklet:            Tasklet Info structure to get cmd from
 * @tasklet_cmd:        Return tasklet_cmd pointer if successful
 *
 * @return:             0: Success
 *                      Negative: Failure
 */
static int cam_tasklet_get_cmd(
	struct cam_tasklet_info        *tasklet,
	struct cam_tasklet_queue_cmd  **tasklet_cmd)
struct cam_irq_bh_api tasklet_bh_api = {
	.bottom_half_enqueue_func = cam_tasklet_enqueue_cmd,
	.get_bh_payload_func = cam_tasklet_get_cmd,
	.put_bh_payload_func = cam_tasklet_put_cmd,
};

int cam_tasklet_get_cmd(
	void                         *bottom_half,
	void                        **bh_cmd)
{
	int           rc = 0;
	unsigned long flags;
	struct cam_tasklet_info        *tasklet = bottom_half;
	struct cam_tasklet_queue_cmd   *tasklet_cmd = NULL;

	*tasklet_cmd = NULL;
	*bh_cmd = NULL;

	if (tasklet == NULL) {
		CAM_ERR_RATE_LIMIT(CAM_ISP, "tasklet is NULL");
		return -EINVAL;
	}

	if (!atomic_read(&tasklet->tasklet_active)) {
		CAM_ERR_RATE_LIMIT(CAM_ISP, "Tasklet is not active!\n");
@@ -102,9 +104,10 @@ static int cam_tasklet_get_cmd(
		rc = -ENODEV;
		goto spin_unlock;
	} else {
		*tasklet_cmd = list_first_entry(&tasklet->free_cmd_list,
		tasklet_cmd = list_first_entry(&tasklet->free_cmd_list,
			struct cam_tasklet_queue_cmd, list);
		list_del_init(&(*tasklet_cmd)->list);
		list_del_init(&(tasklet_cmd)->list);
		*bh_cmd = tasklet_cmd;
	}

spin_unlock:
@@ -112,25 +115,28 @@ static int cam_tasklet_get_cmd(
	return rc;
}

/**
 * cam_tasklet_put_cmd()
 *
 * @brief:              Put back cmd to free list
 *
 * @tasklet:            Tasklet Info structure to put cmd into
 * @tasklet_cmd:        tasklet_cmd pointer that needs to be put back
 *
 * @return:             Void
 */
static void cam_tasklet_put_cmd(
	struct cam_tasklet_info        *tasklet,
	struct cam_tasklet_queue_cmd  **tasklet_cmd)
void cam_tasklet_put_cmd(
	void                         *bottom_half,
	void                        **bh_cmd)
{
	unsigned long flags;
	struct cam_tasklet_info        *tasklet = bottom_half;
	struct cam_tasklet_queue_cmd   *tasklet_cmd = *bh_cmd;

	if (tasklet == NULL) {
		CAM_ERR_RATE_LIMIT(CAM_ISP, "tasklet is NULL");
		return;
	}

	if (tasklet_cmd == NULL) {
		CAM_ERR_RATE_LIMIT(CAM_ISP, "Invalid tasklet_cmd");
		return;
	}

	spin_lock_irqsave(&tasklet->tasklet_lock, flags);
	list_add_tail(&(*tasklet_cmd)->list,
		&tasklet->free_cmd_list);
	list_del_init(&tasklet_cmd->list);
	list_add_tail(&tasklet_cmd->list, &tasklet->free_cmd_list);
	*bh_cmd = NULL;
	spin_unlock_irqrestore(&tasklet->tasklet_lock, flags);
}

@@ -181,23 +187,26 @@ static int cam_tasklet_dequeue_cmd(
	return rc;
}

int cam_tasklet_enqueue_cmd(
void cam_tasklet_enqueue_cmd(
	void                              *bottom_half,
	void                              *bh_cmd,
	void                              *handler_priv,
	void                              *evt_payload_priv,
	CAM_IRQ_HANDLER_BOTTOM_HALF        bottom_half_handler)
{
	struct cam_tasklet_info       *tasklet = bottom_half;
	struct cam_tasklet_queue_cmd  *tasklet_cmd = NULL;
	unsigned long                  flags;
	int                            rc;
	struct cam_tasklet_queue_cmd  *tasklet_cmd = bh_cmd;
	struct cam_tasklet_info       *tasklet = bottom_half;

	if (!bottom_half) {
		CAM_ERR(CAM_ISP, "NULL bottom half");
		return -EINVAL;
		return;
	}

	rc = cam_tasklet_get_cmd(tasklet, &tasklet_cmd);
	if (!bh_cmd) {
		CAM_ERR(CAM_ISP, "NULL bh cmd");
		return;
	}

	if (tasklet_cmd) {
		CAM_DBG(CAM_ISP, "Enqueue tasklet cmd");
@@ -211,8 +220,6 @@ int cam_tasklet_enqueue_cmd(
	} else {
		CAM_ERR(CAM_ISP, "tasklet cmd is NULL!");
	}

	return rc;
}

int cam_tasklet_init(
@@ -323,7 +330,7 @@ static void cam_tasklet_action(unsigned long data)
	while (!cam_tasklet_dequeue_cmd(tasklet_info, &tasklet_cmd)) {
		tasklet_cmd->bottom_half_handler(tasklet_info->ctx_priv,
			tasklet_cmd->payload);
		cam_tasklet_put_cmd(tasklet_info, &tasklet_cmd);
		cam_tasklet_put_cmd(tasklet_info, (void **)(&tasklet_cmd));
	}
}
+31 −3
Original line number Diff line number Diff line
@@ -76,6 +76,7 @@ void cam_tasklet_stop(void *tasklet);
 * @brief:               Enqueue the tasklet_cmd to used list
 *
 * @bottom_half:         Tasklet info to enqueue onto
 * @bh_cmd:              Tasklet cmd used to enqueue task
 * @handler_priv:        Private Handler data that will be passed to the
 *                       handler function
 * @evt_payload_priv:    Event payload that will be passed to the handler
@@ -83,13 +84,40 @@ void cam_tasklet_stop(void *tasklet);
 * @bottom_half_handler: Callback function that will be called by tasklet
 *                       for handling event
 *
 * @return:              0: Success
 *                       Negative: Failure
 * @return:              Void
 */
int cam_tasklet_enqueue_cmd(
void cam_tasklet_enqueue_cmd(
	void                              *bottom_half,
	void                              *bh_cmd,
	void                              *handler_priv,
	void                              *evt_payload_priv,
	CAM_IRQ_HANDLER_BOTTOM_HALF        bottom_half_handler);

/**
 * cam_tasklet_get_cmd()
 *
 * @brief:              Get free cmd from tasklet
 *
 * @bottom_half:        Tasklet Info structure to get cmd from
 * @bh_cmd:             Return tasklet_cmd pointer if successful
 *
 * @return:             0: Success
 *                      Negative: Failure
 */
int cam_tasklet_get_cmd(void *bottom_half, void **bh_cmd);

/**
 * cam_tasklet_put_cmd()
 *
 * @brief:              Put back cmd to free list
 *
 * @bottom_half:        Tasklet Info structure to put cmd into
 * @bh_cmd:             tasklet_cmd pointer that needs to be put back
 *
 * @return:             Void
 */
void cam_tasklet_put_cmd(void *bottom_half, void **bh_cmd);

extern struct cam_irq_bh_api tasklet_bh_api;

#endif /* _CAM_TASKLET_UTIL_H_ */
+49 −13
Original line number Diff line number Diff line
@@ -13,6 +13,8 @@
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/list.h>
#include <linux/ratelimit.h>

#include "cam_io_util.h"
#include "cam_irq_controller.h"
#include "cam_debug_util.h"
@@ -42,7 +44,7 @@ struct cam_irq_evt_handler {
	CAM_IRQ_HANDLER_TOP_HALF           top_half_handler;
	CAM_IRQ_HANDLER_BOTTOM_HALF        bottom_half_handler;
	void                              *bottom_half;
	CAM_IRQ_BOTTOM_HALF_ENQUEUE_FUNC   bottom_half_enqueue_func;
	struct cam_irq_bh_api              irq_bh_api;
	struct list_head                   list_node;
	struct list_head                   th_list_node;
	int                                index;
@@ -232,7 +234,7 @@ int cam_irq_controller_subscribe_irq(void *irq_controller,
	CAM_IRQ_HANDLER_TOP_HALF           top_half_handler,
	CAM_IRQ_HANDLER_BOTTOM_HALF        bottom_half_handler,
	void                              *bottom_half,
	CAM_IRQ_BOTTOM_HALF_ENQUEUE_FUNC   bottom_half_enqueue_func)
	struct cam_irq_bh_api             *irq_bh_api)
{
	struct cam_irq_controller  *controller  = irq_controller;
	struct cam_irq_evt_handler *evt_handler = NULL;
@@ -255,12 +257,24 @@ int cam_irq_controller_subscribe_irq(void *irq_controller,
	}

	if (bottom_half_handler &&
		(!bottom_half || !bottom_half_enqueue_func)) {
		(!bottom_half || !irq_bh_api)) {
		CAM_ERR(CAM_ISP,
			"Invalid params: bh_handler=%pK bh=%pK bh_enq_f=%pK",
			bottom_half_handler,
			bottom_half,
			bottom_half_enqueue_func);
			irq_bh_api);
		return -EINVAL;
	}

	if (irq_bh_api &&
		(!irq_bh_api->bottom_half_enqueue_func ||
		!irq_bh_api->get_bh_payload_func ||
		!irq_bh_api->put_bh_payload_func)) {
		CAM_ERR(CAM_ISP,
			"Invalid: enqueue_func=%pK get_bh=%pK put_bh=%pK",
			irq_bh_api->bottom_half_enqueue_func,
			irq_bh_api->get_bh_payload_func,
			irq_bh_api->put_bh_payload_func);
		return -EINVAL;
	}

@@ -295,9 +309,11 @@ int cam_irq_controller_subscribe_irq(void *irq_controller,
	evt_handler->top_half_handler         = top_half_handler;
	evt_handler->bottom_half_handler      = bottom_half_handler;
	evt_handler->bottom_half              = bottom_half;
	evt_handler->bottom_half_enqueue_func = bottom_half_enqueue_func;
	evt_handler->index                    = controller->hdl_idx++;

	if (irq_bh_api)
		evt_handler->irq_bh_api       = *irq_bh_api;

	/* Avoid rollover to negative values */
	if (controller->hdl_idx > 0x3FFFFFFF)
		controller->hdl_idx = 1;
@@ -564,6 +580,8 @@ static void cam_irq_controller_th_processing(
	bool                            is_irq_match;
	int                             rc = -EINVAL;
	int                             i;
	void                           *bh_cmd = NULL;
	struct cam_irq_bh_api          *irq_bh_api = NULL;

	CAM_DBG(CAM_ISP, "Enter");

@@ -588,6 +606,19 @@ static void cam_irq_controller_th_processing(
				evt_handler->evt_bit_mask_arr[i];
		}

		irq_bh_api = &evt_handler->irq_bh_api;
		bh_cmd = NULL;

		if (evt_handler->bottom_half_handler) {
			rc = irq_bh_api->get_bh_payload_func(
				evt_handler->bottom_half, &bh_cmd);
			if (rc || !bh_cmd) {
				CAM_ERR_RATE_LIMIT(CAM_ISP,
					"Can't get bh payload");
				continue;
			}
		}

		/*
		 * irq_status_arr[0] is dummy argument passed. the entire
		 * status array is passed in th_payload.
@@ -597,18 +628,23 @@ static void cam_irq_controller_th_processing(
				controller->irq_status_arr[0],
				(void *)th_payload);

		if (!rc && evt_handler->bottom_half_handler) {
		if (rc && bh_cmd) {
			irq_bh_api->put_bh_payload_func(
				evt_handler->bottom_half, &bh_cmd);
			continue;
		}

		if (evt_handler->bottom_half_handler) {
			CAM_DBG(CAM_ISP, "Enqueuing bottom half for %s",
				controller->name);
			if (evt_handler->bottom_half_enqueue_func) {
				evt_handler->bottom_half_enqueue_func(
			irq_bh_api->bottom_half_enqueue_func(
				evt_handler->bottom_half,
				bh_cmd,
				evt_handler->handler_priv,
				th_payload->evt_payload_priv,
				evt_handler->bottom_half_handler);
		}
	}
	}

	CAM_DBG(CAM_ISP, "Exit");
}
+15 −3
Original line number Diff line number Diff line
@@ -111,10 +111,22 @@ typedef int (*CAM_IRQ_HANDLER_TOP_HALF)(uint32_t evt_id,
typedef int (*CAM_IRQ_HANDLER_BOTTOM_HALF)(void *handler_priv,
	void *evt_payload_priv);

typedef int (*CAM_IRQ_BOTTOM_HALF_ENQUEUE_FUNC)(void *bottom_half,
	void *handler_priv, void *evt_payload_priv,
typedef void (*CAM_IRQ_BOTTOM_HALF_ENQUEUE_FUNC)(void *bottom_half,
	void *bh_cmd, void *handler_priv, void *evt_payload_priv,
	CAM_IRQ_HANDLER_BOTTOM_HALF);

typedef int (*CAM_IRQ_GET_TASKLET_PAYLOAD_FUNC)(void *bottom_half,
	void **bh_cmd);

typedef void (*CAM_IRQ_PUT_TASKLET_PAYLOAD_FUNC)(void *bottom_half,
	void **bh_cmd);

struct cam_irq_bh_api {
	CAM_IRQ_BOTTOM_HALF_ENQUEUE_FUNC bottom_half_enqueue_func;
	CAM_IRQ_GET_TASKLET_PAYLOAD_FUNC get_bh_payload_func;
	CAM_IRQ_PUT_TASKLET_PAYLOAD_FUNC put_bh_payload_func;
};

/*
 * cam_irq_controller_init()
 *
@@ -165,7 +177,7 @@ int cam_irq_controller_subscribe_irq(void *irq_controller,
	CAM_IRQ_HANDLER_TOP_HALF           top_half_handler,
	CAM_IRQ_HANDLER_BOTTOM_HALF        bottom_half_handler,
	void                              *bottom_half,
	CAM_IRQ_BOTTOM_HALF_ENQUEUE_FUNC   bottom_half_enqueue_func);
	struct cam_irq_bh_api             *irq_bh_api);

/*
 * cam_irq_controller_unsubscribe_irq()
+3 −3
Original line number Diff line number Diff line
@@ -560,7 +560,7 @@ int cam_vfe_start(void *hw_priv, void *start_args, uint32_t arg_size)
					cam_vfe_irq_top_half,
					cam_ife_mgr_do_tasklet,
					isp_res->tasklet_info,
					cam_tasklet_enqueue_cmd);
					&tasklet_bh_api);
			if (isp_res->irq_handle < 1)
				rc = -ENOMEM;
		} else if (isp_res->rdi_only_ctx) {
@@ -573,7 +573,7 @@ int cam_vfe_start(void *hw_priv, void *start_args, uint32_t arg_size)
					cam_vfe_irq_top_half,
					cam_ife_mgr_do_tasklet,
					isp_res->tasklet_info,
					cam_tasklet_enqueue_cmd);
					&tasklet_bh_api);
			if (isp_res->irq_handle < 1)
				rc = -ENOMEM;
		}
@@ -606,7 +606,7 @@ int cam_vfe_start(void *hw_priv, void *start_args, uint32_t arg_size)
				cam_vfe_irq_err_top_half,
				cam_ife_mgr_do_tasklet,
				core_info->tasklet_info,
				cam_tasklet_enqueue_cmd);
				&tasklet_bh_api);
		if (core_info->irq_err_handle < 1) {
			CAM_ERR(CAM_ISP, "Error handle subscribe failure");
			rc = -ENOMEM;
Loading