From patchwork Thu Aug 24 07:21:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sricharan Ramabadhran X-Patchwork-Id: 9919359 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B914260327 for ; Thu, 24 Aug 2017 07:50:06 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A946128B99 for ; Thu, 24 Aug 2017 07:50:06 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9E44B28BA7; Thu, 24 Aug 2017 07:50:06 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_LOW autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E4F3128B99 for ; Thu, 24 Aug 2017 07:50:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:References: In-Reply-To:Message-Id:Date:Subject:To:From:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=S53SQRQHaYMC0yQwVrK+eronVVZQbqUGir2XOsPwZ5A=; b=VuIEa77Ajvn23ByZGAArpE9I1M +X8/96Ivcn3iYuZAk2HND6IJNgNGmGuEUkUWCVCT6YQsNq0yVDbNrZRFd5ldEkhHfIl7dcKDO8heQ VLqm0/+6ESr4IyfNK/sF1JsRlP6MoLxckobmPGBhB3gw4miYb4BWpsDdyPx2oZCvEJ09PdqiAq4bw DHh64Fgu5dvIr4hU50xuI3mmQjwncMxaNJFPaFwDIlOSYEAwkyO/yX21l5eF3Upa0hbp+bR8tZVju 2PWO0KqZmejteOeJKmySi+6r41nHRMSYkjD/jsFQpYygYkC1M9h0IkIybehzrqCECyXK1S7EsIAVw hRsh03sg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dkmtr-0002cD-At; Thu, 24 Aug 2017 07:49:59 +0000 Received: from merlin.infradead.org ([2001:8b0:10b:1231::1]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dkmt9-00022b-VK for linux-arm-kernel@bombadil.infradead.org; Thu, 24 Aug 2017 07:49:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=merlin.20170209; h=References:In-Reply-To:Message-Id:Date: Subject:Cc:To:From:Sender:Reply-To:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=BDXye/6tG/vJc/1fhHaMOquJ5MXl86B65dCuMTsBbnI=; b=elIz5QDM13OvklSi8xThYESbe pVK5XB4MdPUIWXMa1m/INIsiJT/M8FsWMHMB+f2cvPP2FL/08qKIf2AmaJ2qBkZJxrL+Uy5UgOqhZ KFKQerIO2p9A+XyCUx8vusnPaTAwOLaoqpQpX5ylPvag3vdDT7Koe0HL/KTGpCEdNGoMLK1LNFzYC UeEIaurAEzpAxbk9qeuzK4BKO5WhRFcG9Cl8ASsB1YtVE/fpjvB62+Tko/Zu14ReSMJubmFw3A5Gl dMRS3ez8DZ/r95mzRFybzESJAfZOtcNn1I2Dmp+srL1fBxQx42+QUGN9gMZJOHac2dBp23cX10tq4 ee5kDnIUA==; Received: from smtp.codeaurora.org ([198.145.29.96]) by merlin.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dkmTr-0004iQ-2e for linux-arm-kernel@lists.infradead.org; Thu, 24 Aug 2017 07:23:08 +0000 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 2413B60710; Thu, 24 Aug 2017 07:22:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1503559360; bh=qgQiHl+AJCp7UnWnDMBpAoo1kY1wFgs1N12NpEFsuNU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=h41xvY4zsli+0YQC+bkD+wd3gJBXLektsO4aKghMjX+n2jyA2MTgBltxOwGUhSy2A U/N7ZMU0GQwqvnrbgF7PZRCKcqSfscu5w7lqtKxPSIiNGIjGxATzGUX0KxysgoQxIv FPMXc0AH6xNKd/rYH7xxpXQ3+iS7nOYvE2tRROrY= Received: from srichara-linux.qualcomm.com (blr-c-bdr-fw-01_globalnat_allzones-outside.qualcomm.com [103.229.19.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: sricharan@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 9CD3B607A0; Thu, 24 Aug 2017 07:22:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1503559349; bh=qgQiHl+AJCp7UnWnDMBpAoo1kY1wFgs1N12NpEFsuNU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hBg+csoClj5iEePWzC6Hq1r8c0BBQN2Ue3l+VKJts4O17FL9uBNVVAdn5/51jCmz4 gs4ln10BR2q9V5wSh9HhArORBoNjblK6uAbxoVGMbz14mRScB/ZuQRgSiaXd/dBWTp O6k1S/ThWzpCuzqyT/tEO4F2nFkv5KkXgxrgnAyg= DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 9CD3B607A0 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=sricharan@codeaurora.org From: Sricharan R To: ohad@wizery.com, bjorn.andersson@linaro.org, linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 11/20] rpmsg: glink: Fix idr_lock from mutex to spinlock Date: Thu, 24 Aug 2017 12:51:33 +0530 Message-Id: <1503559302-3744-12-git-send-email-sricharan@codeaurora.org> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1503559302-3744-1-git-send-email-sricharan@codeaurora.org> References: <1503559302-3744-1-git-send-email-sricharan@codeaurora.org> X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: sricharan@codeaurora.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP 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. Signed-off-by: Sricharan R Signed-off-by: Bjorn Andersson Acked-by: Arun Kumar Neelakantam --- drivers/rpmsg/qcom_glink_native.c | 58 +++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c index 777ac6b..588a56c 100644 --- a/drivers/rpmsg/qcom_glink_native.c +++ b/drivers/rpmsg/qcom_glink_native.c @@ -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); + 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); @@ -621,6 +627,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)) @@ -644,9 +651,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 */ @@ -703,11 +710,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); @@ -829,11 +839,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); @@ -844,15 +857,15 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid, create_device = true; } - mutex_lock(&glink->idr_lock); + spin_lock_irqsave(&glink->idr_lock, flags); ret = idr_alloc(&glink->rcids, channel, rcid, rcid + 1, GFP_KERNEL); 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); @@ -886,10 +899,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) @@ -902,8 +915,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; @@ -917,10 +933,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); } @@ -928,15 +944,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); } @@ -1017,7 +1036,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); @@ -1060,6 +1079,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); @@ -1068,12 +1088,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); }