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

Commit 6bc7c13f authored by Manoj Prabhu B's avatar Manoj Prabhu B Committed by Gerrit - the friendly Code Review server
Browse files

diag: Use mutex instead of spinlock for synchronization 



Usage of spinlock for rpmsg send call in rpmsg write leads to sleep
issue while in atomic context. Replace spinlock inplace of mutex to
synchronize rpmsg read and write.

Change-Id: I6384961fcbb25bca21781c2b88353e141f1614bc
Signed-off-by: default avatarManoj Prabhu B <bmanoj@codeaurora.org>
parent efe4f92d

drivers/char/diag/diagchar.h

+1 −1
Original line number Diff line number Diff line
@@ -780,7 +780,7 @@ struct diagchar_dev { 
	int dci_tag;

	int dci_client_id[MAX_DCI_CLIENTS];

	struct mutex dci_mutex;

	spinlock_t rpmsginfo_lock[NUM_PERIPHERALS];

	struct mutex rpmsginfo_mutex[NUM_PERIPHERALS];

	int num_dci_client;

	unsigned char *apps_dci_buf;

	int dci_state;

drivers/char/diag/diagchar_core.c

+1 −1
Original line number Diff line number Diff line
@@ -4360,7 +4360,7 @@ static int __init diagchar_init(void) 
	mutex_init(&driver->hdlc_recovery_mutex);

	for (i = 0; i < NUM_PERIPHERALS; i++) {

		mutex_init(&driver->diagfwd_channel_mutex[i]);

		spin_lock_init(&driver->rpmsginfo_lock[i]);

		mutex_init(&driver->rpmsginfo_mutex[i]);

		driver->diag_id_sent[i] = 0;

	}

	init_waitqueue_head(&driver->wait_q);

drivers/char/diag/diagfwd_rpmsg.c

+34 −65
Original line number Diff line number Diff line
@@ -391,17 +391,12 @@ static void diag_state_open_rpmsg(void *ctxt) 
static void diag_rpmsg_queue_read(void *ctxt)

{

	struct diag_rpmsg_info *rpmsg_info = NULL;

	unsigned long flags;



	if (!ctxt)

		return;



	rpmsg_info = (struct diag_rpmsg_info *)ctxt;

	spin_lock_irqsave(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	if (rpmsg_info->hdl && rpmsg_info->wq &&

		atomic_read(&rpmsg_info->opened))

	queue_work(rpmsg_info->wq, &(rpmsg_info->read_work));

	spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

}



static void diag_state_close_rpmsg(void *ctxt)
@@ -435,7 +430,6 @@ static int diag_rpmsg_read(void *ctxt, unsigned char *buf, int buf_len) 
	struct diag_rpmsg_info *rpmsg_info =  NULL;

	struct diagfwd_info *fwd_info = NULL;

	int ret_val = 0;

	unsigned long flags;



	if (!ctxt || !buf || buf_len <= 0)

		return -EIO;
@@ -446,16 +440,15 @@ static int diag_rpmsg_read(void *ctxt, unsigned char *buf, int buf_len) 
		return -EIO;

	}



	spin_lock_irqsave(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_lock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

	if (!atomic_read(&rpmsg_info->opened) ||

		!rpmsg_info->hdl || !rpmsg_info->inited) {

		DIAG_LOG(DIAG_DEBUG_PERIPHERALS,

			"diag:RPMSG channel not opened");

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

		return -EIO;

	}

	spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);



	fwd_info = rpmsg_info->fwd_ctxt;
@@ -479,25 +472,22 @@ static void diag_rpmsg_read_work_fn(struct work_struct *work) 
	struct diag_rpmsg_info *rpmsg_info = container_of(work,

							struct diag_rpmsg_info,

							read_work);

	unsigned long flags;



	if (!rpmsg_info)

		return;



	spin_lock_irqsave(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_lock(&driver->rpmsginfo_mutex[PERI_RPMSG]);



	if (!atomic_read(&rpmsg_info->opened)) {

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

		return;

	}

	if (!rpmsg_info->inited) {

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

		diag_ws_release();

		return;

	}

	spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);



	diagfwd_channel_read(rpmsg_info->fwd_ctxt);

}
@@ -507,7 +497,6 @@ static int diag_rpmsg_write(void *ctxt, unsigned char *buf, int len) 
	struct diag_rpmsg_info *rpmsg_info = NULL;

	int err = 0;

	struct rpmsg_device *rpdev = NULL;

	unsigned long flags;



	if (!ctxt || !buf)

		return -EIO;
@@ -519,16 +508,14 @@ static int diag_rpmsg_write(void *ctxt, unsigned char *buf, int len) 
		return -EINVAL;

	}



	spin_lock_irqsave(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_lock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

	if (!rpmsg_info->inited || !rpmsg_info->hdl ||

		!atomic_read(&rpmsg_info->opened)) {

		pr_err_ratelimited("diag: In %s, rpmsg not inited, rpmsg_info: %pK, buf: %pK, len: %d\n",

				 __func__, rpmsg_info, buf, len);

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

		return -ENODEV;

	}

	spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG], flags);



	rpdev = (struct rpmsg_device *)rpmsg_info->hdl;

	err = rpmsg_send(rpdev->ept, buf, len);
@@ -538,6 +525,7 @@ static int diag_rpmsg_write(void *ctxt, unsigned char *buf, int len) 
	} else

		err = -ENOMEM;



	mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

	return err;



}
@@ -547,18 +535,16 @@ static void diag_rpmsg_late_init_work_fn(struct work_struct *work) 
	struct diag_rpmsg_info *rpmsg_info = container_of(work,

							struct diag_rpmsg_info,

							late_init_work);

	unsigned long flags;



	if (!rpmsg_info)

		return;



	spin_lock_irqsave(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_lock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

	if (!rpmsg_info->hdl) {

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

		return;

	}

	spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);



	diagfwd_channel_open(rpmsg_info->fwd_ctxt);

	DIAG_LOG(DIAG_DEBUG_PERIPHERALS, "rpmsg late init p: %d t: %d\n",
@@ -571,18 +557,16 @@ static void diag_rpmsg_open_work_fn(struct work_struct *work) 
	struct diag_rpmsg_info *rpmsg_info = container_of(work,

							struct diag_rpmsg_info,

							open_work);

	unsigned long flags;



	if (!rpmsg_info)

		return;



	spin_lock_irqsave(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_lock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

	if (!rpmsg_info->inited) {

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

		return;

	}

	spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);



	if (rpmsg_info->type != TYPE_CNTL) {

		diagfwd_channel_open(rpmsg_info->fwd_ctxt);
@@ -597,19 +581,17 @@ static void diag_rpmsg_close_work_fn(struct work_struct *work) 
	struct diag_rpmsg_info *rpmsg_info = container_of(work,

							struct diag_rpmsg_info,

							close_work);

	unsigned long flags;



	if (!rpmsg_info)

		return;



	spin_lock_irqsave(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_lock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

	if (!rpmsg_info->inited || !rpmsg_info->hdl) {

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

		return;

	}

	rpmsg_info->hdl = NULL;

	spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

	diagfwd_channel_close(rpmsg_info->fwd_ctxt);

}
@@ -722,20 +704,18 @@ static void rpmsg_late_init(struct diag_rpmsg_info *rpmsg_info) 


int diag_rpmsg_init_peripheral(uint8_t peripheral)

{

	unsigned long flags;



	if (peripheral >= NUM_PERIPHERALS) {

		pr_err("diag: In %s, invalid peripheral %d\n", __func__,

			peripheral);

		return -EINVAL;

	}



	spin_lock_irqsave(&driver->rpmsginfo_lock[peripheral], flags);

	mutex_lock(&driver->rpmsginfo_mutex[peripheral]);

	rpmsg_late_init(&rpmsg_data[peripheral]);

	rpmsg_late_init(&rpmsg_dci[peripheral]);

	rpmsg_late_init(&rpmsg_cmd[peripheral]);

	rpmsg_late_init(&rpmsg_dci_cmd[peripheral]);

	spin_unlock_irqrestore(&driver->rpmsginfo_lock[peripheral], flags);

	mutex_unlock(&driver->rpmsginfo_mutex[peripheral]);



	return 0;

}
@@ -743,7 +723,6 @@ int diag_rpmsg_init_peripheral(uint8_t peripheral) 
static void __diag_rpmsg_init(struct diag_rpmsg_info *rpmsg_info)

{

	char wq_name[DIAG_RPMSG_NAME_SZ + 12];

	unsigned long flags;



	if (!rpmsg_info)

		return;
@@ -763,7 +742,7 @@ static void __diag_rpmsg_init(struct diag_rpmsg_info *rpmsg_info) 
	INIT_WORK(&(rpmsg_info->close_work), diag_rpmsg_close_work_fn);

	INIT_WORK(&(rpmsg_info->read_work), diag_rpmsg_read_work_fn);

	INIT_WORK(&(rpmsg_info->late_init_work), diag_rpmsg_late_init_work_fn);

	spin_lock_irqsave(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_lock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

	rpmsg_info->hdl = NULL;

	rpmsg_info->fwd_ctxt = NULL;

	atomic_set(&rpmsg_info->opened, 0);
@@ -772,7 +751,7 @@ static void __diag_rpmsg_init(struct diag_rpmsg_info *rpmsg_info) 
		"%s initialized fwd_ctxt: %pK hdl: %pK\n",

		rpmsg_info->name, rpmsg_info->fwd_ctxt,

		rpmsg_info->hdl);

	spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

	mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

}



void diag_rpmsg_invalidate(void *ctxt, struct diagfwd_info *fwd_ctxt)
@@ -790,7 +769,6 @@ int diag_rpmsg_init(void) 
{

	uint8_t peripheral;

	struct diag_rpmsg_info *rpmsg_info = NULL;

	unsigned long flags;



	for (peripheral = 0; peripheral < NUM_PERIPHERALS; peripheral++) {

		if (peripheral != PERIPHERAL_WDSP)
@@ -800,10 +778,9 @@ int diag_rpmsg_init(void) 
		diagfwd_cntl_register(TRANSPORT_RPMSG, rpmsg_info->peripheral,

					(void *)rpmsg_info, &rpmsg_ops,

					&(rpmsg_info->fwd_ctxt));

		spin_lock_irqsave(&driver->rpmsginfo_lock[peripheral], flags);

		mutex_lock(&driver->rpmsginfo_mutex[peripheral]);

		rpmsg_info->inited = 1;

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[peripheral],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[peripheral]);

		diagfwd_channel_open(rpmsg_info->fwd_ctxt);

		diagfwd_late_open(rpmsg_info->fwd_ctxt);

		__diag_rpmsg_init(&rpmsg_data[peripheral]);
@@ -836,31 +813,27 @@ static void __diag_rpmsg_exit(struct diag_rpmsg_info *rpmsg_info) 
void diag_rpmsg_early_exit(void)

{

	int peripheral = 0;

	unsigned long flags;



	for (peripheral = 0; peripheral < NUM_PERIPHERALS; peripheral++) {

		if (peripheral != PERIPHERAL_WDSP)

			continue;

		spin_lock_irqsave(&driver->rpmsginfo_lock[peripheral], flags);

		mutex_lock(&driver->rpmsginfo_mutex[peripheral]);

		__diag_rpmsg_exit(&rpmsg_cntl[peripheral]);

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[peripheral],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[peripheral]);

	}

}



void diag_rpmsg_exit(void)

{

	int peripheral = 0;

	unsigned long flags;



	for (peripheral = 0; peripheral < NUM_PERIPHERALS; peripheral++) {

		spin_lock_irqsave(&driver->rpmsginfo_lock[peripheral], flags);

		mutex_lock(&driver->rpmsginfo_mutex[peripheral]);

		__diag_rpmsg_exit(&rpmsg_data[peripheral]);

		__diag_rpmsg_exit(&rpmsg_cmd[peripheral]);

		__diag_rpmsg_exit(&rpmsg_dci[peripheral]);

		__diag_rpmsg_exit(&rpmsg_dci_cmd[peripheral]);

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[peripheral],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[peripheral]);

	}

}
@@ -886,7 +859,6 @@ static struct diag_rpmsg_info *diag_get_rpmsg_ptr(char *name) 
static int diag_rpmsg_probe(struct rpmsg_device *rpdev)

{

	struct diag_rpmsg_info *rpmsg_info = NULL;

	unsigned long flags;



	if (!rpdev)

		return 0;
@@ -896,11 +868,10 @@ static int diag_rpmsg_probe(struct rpmsg_device *rpdev) 
	rpmsg_info = diag_get_rpmsg_ptr(rpdev->id.name);

	if (rpmsg_info) {



		spin_lock_irqsave(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

		mutex_lock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

		rpmsg_info->hdl = rpdev;

		atomic_set(&rpmsg_info->opened, 1);

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);



		dev_set_drvdata(&rpdev->dev, rpmsg_info);

		diagfwd_channel_read(rpmsg_info->fwd_ctxt);
@@ -913,17 +884,15 @@ static int diag_rpmsg_probe(struct rpmsg_device *rpdev) 
static void diag_rpmsg_remove(struct rpmsg_device *rpdev)

{

	struct diag_rpmsg_info *rpmsg_info = NULL;

	unsigned long flags;



	if (!rpdev)

		return;



	rpmsg_info = diag_get_rpmsg_ptr(rpdev->id.name);

	if (rpmsg_info) {

		spin_lock_irqsave(&driver->rpmsginfo_lock[PERI_RPMSG], flags);

		mutex_lock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

		atomic_set(&rpmsg_info->opened, 0);

		spin_unlock_irqrestore(&driver->rpmsginfo_lock[PERI_RPMSG],

			flags);

		mutex_unlock(&driver->rpmsginfo_mutex[PERI_RPMSG]);

		queue_work(rpmsg_info->wq, &rpmsg_info->close_work);

	}

}