forked from Minki/linux
rpmsg: glink: Fix idr_lock from mutex to spinlock
The channel members lcids, rcids synchronised using the idr_lock is accessed in both atomic/non-atomic contexts. The readers are not currently synchronised. That no correct, so add the readers as well under the lock and use a spinlock. Acked-by: Arun Kumar Neelakantam <aneela@codeaurora.org> Signed-off-by: Sricharan R <sricharan@codeaurora.org> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
This commit is contained in:
parent
d31ad615f6
commit
44f6df922a
@ -92,7 +92,7 @@ struct qcom_glink {
|
||||
|
||||
struct mutex tx_lock;
|
||||
|
||||
struct mutex idr_lock;
|
||||
spinlock_t idr_lock;
|
||||
struct idr lcids;
|
||||
struct idr rcids;
|
||||
unsigned long features;
|
||||
@ -309,14 +309,15 @@ static int qcom_glink_send_open_req(struct qcom_glink *glink,
|
||||
int name_len = strlen(channel->name) + 1;
|
||||
int req_len = ALIGN(sizeof(req.msg) + name_len, 8);
|
||||
int ret;
|
||||
unsigned long flags;
|
||||
|
||||
kref_get(&channel->refcount);
|
||||
|
||||
mutex_lock(&glink->idr_lock);
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
ret = idr_alloc_cyclic(&glink->lcids, channel,
|
||||
RPM_GLINK_CID_MIN, RPM_GLINK_CID_MAX,
|
||||
GFP_KERNEL);
|
||||
mutex_unlock(&glink->idr_lock);
|
||||
GFP_ATOMIC);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
if (ret < 0)
|
||||
return ret;
|
||||
|
||||
@ -334,10 +335,10 @@ static int qcom_glink_send_open_req(struct qcom_glink *glink,
|
||||
return 0;
|
||||
|
||||
remove_idr:
|
||||
mutex_lock(&glink->idr_lock);
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
idr_remove(&glink->lcids, channel->lcid);
|
||||
channel->lcid = 0;
|
||||
mutex_unlock(&glink->idr_lock);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
|
||||
return ret;
|
||||
}
|
||||
@ -463,6 +464,7 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail)
|
||||
unsigned int chunk_size;
|
||||
unsigned int left_size;
|
||||
unsigned int rcid;
|
||||
unsigned long flags;
|
||||
|
||||
if (avail < sizeof(hdr)) {
|
||||
dev_dbg(glink->dev, "Not enough data in fifo\n");
|
||||
@ -482,7 +484,9 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail)
|
||||
return -EINVAL;
|
||||
|
||||
rcid = le16_to_cpu(hdr.msg.param1);
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
channel = idr_find(&glink->rcids, rcid);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
if (!channel) {
|
||||
dev_dbg(glink->dev, "Data on non-existing channel\n");
|
||||
|
||||
@ -543,11 +547,13 @@ static int qcom_glink_rx_open_ack(struct qcom_glink *glink, unsigned int lcid)
|
||||
{
|
||||
struct glink_channel *channel;
|
||||
|
||||
spin_lock(&glink->idr_lock);
|
||||
channel = idr_find(&glink->lcids, lcid);
|
||||
if (!channel) {
|
||||
dev_err(glink->dev, "Invalid open ack packet\n");
|
||||
return -EINVAL;
|
||||
}
|
||||
spin_unlock(&glink->idr_lock);
|
||||
|
||||
complete(&channel->open_ack);
|
||||
|
||||
@ -620,6 +626,7 @@ static struct glink_channel *qcom_glink_create_local(struct qcom_glink *glink,
|
||||
{
|
||||
struct glink_channel *channel;
|
||||
int ret;
|
||||
unsigned long flags;
|
||||
|
||||
channel = qcom_glink_alloc_channel(glink, name);
|
||||
if (IS_ERR(channel))
|
||||
@ -643,9 +650,9 @@ static struct glink_channel *qcom_glink_create_local(struct qcom_glink *glink,
|
||||
|
||||
err_timeout:
|
||||
/* qcom_glink_send_open_req() did register the channel in lcids*/
|
||||
mutex_lock(&glink->idr_lock);
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
idr_remove(&glink->lcids, channel->lcid);
|
||||
mutex_unlock(&glink->idr_lock);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
|
||||
release_channel:
|
||||
/* Release qcom_glink_send_open_req() reference */
|
||||
@ -702,11 +709,14 @@ static struct rpmsg_endpoint *qcom_glink_create_ept(struct rpmsg_device *rpdev,
|
||||
const char *name = chinfo.name;
|
||||
int cid;
|
||||
int ret;
|
||||
unsigned long flags;
|
||||
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
idr_for_each_entry(&glink->rcids, channel, cid) {
|
||||
if (!strcmp(channel->name, name))
|
||||
break;
|
||||
}
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
|
||||
if (!channel) {
|
||||
channel = qcom_glink_create_local(glink, name);
|
||||
@ -828,11 +838,14 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid,
|
||||
struct device_node *node;
|
||||
int lcid;
|
||||
int ret;
|
||||
unsigned long flags;
|
||||
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
idr_for_each_entry(&glink->lcids, channel, lcid) {
|
||||
if (!strcmp(channel->name, name))
|
||||
break;
|
||||
}
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
|
||||
if (!channel) {
|
||||
channel = qcom_glink_alloc_channel(glink, name);
|
||||
@ -843,15 +856,15 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid,
|
||||
create_device = true;
|
||||
}
|
||||
|
||||
mutex_lock(&glink->idr_lock);
|
||||
ret = idr_alloc(&glink->rcids, channel, rcid, rcid + 1, GFP_KERNEL);
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
ret = idr_alloc(&glink->rcids, channel, rcid, rcid + 1, GFP_ATOMIC);
|
||||
if (ret < 0) {
|
||||
dev_err(glink->dev, "Unable to insert channel into rcid list\n");
|
||||
mutex_unlock(&glink->idr_lock);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
goto free_channel;
|
||||
}
|
||||
channel->rcid = ret;
|
||||
mutex_unlock(&glink->idr_lock);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
|
||||
complete(&channel->open_req);
|
||||
|
||||
@ -885,10 +898,10 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid,
|
||||
free_rpdev:
|
||||
kfree(rpdev);
|
||||
rcid_remove:
|
||||
mutex_lock(&glink->idr_lock);
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
idr_remove(&glink->rcids, channel->rcid);
|
||||
channel->rcid = 0;
|
||||
mutex_unlock(&glink->idr_lock);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
free_channel:
|
||||
/* Release the reference, iff we took it */
|
||||
if (create_device)
|
||||
@ -901,8 +914,11 @@ static void qcom_glink_rx_close(struct qcom_glink *glink, unsigned int rcid)
|
||||
{
|
||||
struct rpmsg_channel_info chinfo;
|
||||
struct glink_channel *channel;
|
||||
unsigned long flags;
|
||||
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
channel = idr_find(&glink->rcids, rcid);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
if (WARN(!channel, "close request on unknown channel\n"))
|
||||
return;
|
||||
|
||||
@ -916,10 +932,10 @@ static void qcom_glink_rx_close(struct qcom_glink *glink, unsigned int rcid)
|
||||
|
||||
qcom_glink_send_close_ack(glink, channel->rcid);
|
||||
|
||||
mutex_lock(&glink->idr_lock);
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
idr_remove(&glink->rcids, channel->rcid);
|
||||
channel->rcid = 0;
|
||||
mutex_unlock(&glink->idr_lock);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
|
||||
kref_put(&channel->refcount, qcom_glink_channel_release);
|
||||
}
|
||||
@ -927,15 +943,18 @@ static void qcom_glink_rx_close(struct qcom_glink *glink, unsigned int rcid)
|
||||
static void qcom_glink_rx_close_ack(struct qcom_glink *glink, unsigned int lcid)
|
||||
{
|
||||
struct glink_channel *channel;
|
||||
unsigned long flags;
|
||||
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
channel = idr_find(&glink->lcids, lcid);
|
||||
if (WARN(!channel, "close ack on unknown channel\n"))
|
||||
if (WARN(!channel, "close ack on unknown channel\n")) {
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
return;
|
||||
}
|
||||
|
||||
mutex_lock(&glink->idr_lock);
|
||||
idr_remove(&glink->lcids, channel->lcid);
|
||||
channel->lcid = 0;
|
||||
mutex_unlock(&glink->idr_lock);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
|
||||
kref_put(&channel->refcount, qcom_glink_channel_release);
|
||||
}
|
||||
@ -1016,7 +1035,7 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
|
||||
INIT_LIST_HEAD(&glink->rx_queue);
|
||||
INIT_WORK(&glink->rx_work, qcom_glink_work);
|
||||
|
||||
mutex_init(&glink->idr_lock);
|
||||
spin_lock_init(&glink->idr_lock);
|
||||
idr_init(&glink->lcids);
|
||||
idr_init(&glink->rcids);
|
||||
|
||||
@ -1059,6 +1078,7 @@ void qcom_glink_native_remove(struct qcom_glink *glink)
|
||||
struct glink_channel *channel;
|
||||
int cid;
|
||||
int ret;
|
||||
unsigned long flags;
|
||||
|
||||
disable_irq(glink->irq);
|
||||
cancel_work_sync(&glink->rx_work);
|
||||
@ -1067,12 +1087,14 @@ void qcom_glink_native_remove(struct qcom_glink *glink)
|
||||
if (ret)
|
||||
dev_warn(glink->dev, "Can't remove GLINK devices: %d\n", ret);
|
||||
|
||||
spin_lock_irqsave(&glink->idr_lock, flags);
|
||||
/* Release any defunct local channels, waiting for close-ack */
|
||||
idr_for_each_entry(&glink->lcids, channel, cid)
|
||||
kref_put(&channel->refcount, qcom_glink_channel_release);
|
||||
|
||||
idr_destroy(&glink->lcids);
|
||||
idr_destroy(&glink->rcids);
|
||||
spin_unlock_irqrestore(&glink->idr_lock, flags);
|
||||
mbox_free_channel(glink->mbox_chan);
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user