From patchwork Wed Mar 29 17:43:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jassi Brar X-Patchwork-Id: 96210 Delivered-To: patch@linaro.org Received: by 10.140.89.233 with SMTP id v96csp2312651qgd; Wed, 29 Mar 2017 10:43:16 -0700 (PDT) X-Received: by 10.84.233.143 with SMTP id l15mr1943797plk.93.1490809396903; Wed, 29 Mar 2017 10:43:16 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 7si8061241pll.64.2017.03.29.10.43.16; Wed, 29 Mar 2017 10:43:16 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932189AbdC2RnP (ORCPT + 20 others); Wed, 29 Mar 2017 13:43:15 -0400 Received: from mail-pf0-f193.google.com ([209.85.192.193]:35589 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753126AbdC2RnM (ORCPT ); Wed, 29 Mar 2017 13:43:12 -0400 Received: by mail-pf0-f193.google.com with SMTP id n11so2677205pfg.2 for ; Wed, 29 Mar 2017 10:43:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=n8DeNJs7ysqO1k1Gay+fkBYXN6PZOsou+baqDBX5rjA=; b=NvkPPbKFCUQlf28uAzKwV3pK3KHGMcd+kFTu1wvJgppcqckzw8lcyWlrxjj5GD0Ss6 MGXna6YWQOdr0WAYczBX7opUlrac3zy7r94i+6TiRuUPNdb9xAfQHBBrywBVz2EJqh1d HD8tCk3NvqP8z10iKE2WEDNNXp80cUtc/Ie9cA/bD7/QoT3gsSij5XV8Eu2GX+FAPsj2 69O5ol1ybieKSL95Yslnp2rxG+ZQYbKEmqxLT9mAKysWNan1qRzlfIUT0/PhoKJzq1Tf Hp5U0dqs6U5IzQFLjaYOkjmQbGeusoZBuzqd7nRoSXrZkPAVT5GHYeG9qdPOxFDEXZz6 CpmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=n8DeNJs7ysqO1k1Gay+fkBYXN6PZOsou+baqDBX5rjA=; b=DWHSjnwyzNKV4Bjv3CzBpON1SULOxn8ChxNRl2QJcYWdUALtRxeFaxi5+w86kH8BqS 7lFvjndzoGDZy74c0mMEVCxqxEexQxzIrqyAyVEGDeI4UBpuDxbXJ/CyN6/Sc7q5kBx2 /lzEQy4OfnPQgFXJ3awXFpzW4gLS+ZgwtuuyvrExYoJaWJQ8w6Ije9cBwaaUJKVhSeDn I+gtMWycb6PyV1ffcMC53OKvvLTkazysTQjzWBrZh/yjmWWuZ+xxnKTKD3XkBmzxo7Vr MOuWnmK9NM2WKcJr1gqZtXItvQcn3fGDMgpwzQzahS/0gVPPJOh5xILaCLeXydwt0EcV ECgA== X-Gm-Message-State: AFeK/H0Q4/s1MuvAyaleRcZMFtwgtIdDMzEVMbQIFxCeIViYazKGdRgQqXubXa+dfuJylQ== X-Received: by 10.98.8.129 with SMTP id 1mr1788834pfi.268.1490809390587; Wed, 29 Mar 2017 10:43:10 -0700 (PDT) Received: from localhost.localdomain ([27.255.161.62]) by smtp.gmail.com with ESMTPSA id t6sm14721168pgo.42.2017.03.29.10.43.07 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 29 Mar 2017 10:43:09 -0700 (PDT) From: Jassi Brar X-Google-Original-From: Jassi Brar To: linux-kernel@vger.kernel.org Cc: alexey.klimov@arm.com, sudeep.holla@arm.com, Jassi Brar Subject: [PATCH] mailbox: fix completion order for blocking requests Date: Wed, 29 Mar 2017 23:13:01 +0530 Message-Id: <1490809381-28869-1-git-send-email-jaswinder.singh@linaro.org> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Currently two threads, wait on blocking requests, could wake up for completion of request of each other as ... Thread#1(T1) Thread#2(T2) mbox_send_message mbox_send_message | | V | add_to_rbuf(M1) V | add_to_rbuf(M2) | | | V V msg_submit(picks M1) msg_submit | | V V wait_for_completion(on M2) wait_for_completion(on M1) | (1st in waitQ) | (2nd in waitQ) V V wake_up(on completion of M1)<--incorrect Fix this situaion by assigning completion structures to each queued request, so that the threads could wait on their own completions. Reported-by: Alexey Klimov Signed-off-by: Jassi Brar --- drivers/mailbox/mailbox.c | 15 +++++++++++---- drivers/mailbox/omap-mailbox.c | 2 +- drivers/mailbox/pcc.c | 2 +- include/linux/mailbox_controller.h | 6 ++++-- 4 files changed, 17 insertions(+), 8 deletions(-) -- 2.7.4 Signed-off-by: Alexey Klimov diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c index 9dfbf7e..e06c50c 100644 --- a/drivers/mailbox/mailbox.c +++ b/drivers/mailbox/mailbox.c @@ -41,6 +41,7 @@ static int add_to_rbuf(struct mbox_chan *chan, void *mssg) idx = chan->msg_free; chan->msg_data[idx] = mssg; + init_completion(&chan->tx_cmpl[idx]); chan->msg_count++; if (idx == MBOX_TX_QUEUE_LEN - 1) @@ -73,6 +74,7 @@ static void msg_submit(struct mbox_chan *chan) idx += MBOX_TX_QUEUE_LEN - count; data = chan->msg_data[idx]; + chan->tx_complete = &chan->tx_cmpl[idx]; if (chan->cl->tx_prepare) chan->cl->tx_prepare(chan->cl, data); @@ -81,7 +83,8 @@ static void msg_submit(struct mbox_chan *chan) if (!err) { chan->active_req = data; chan->msg_count--; - } + } else + chan->tx_complete = NULL; exit: spin_unlock_irqrestore(&chan->lock, flags); @@ -92,12 +95,15 @@ static void msg_submit(struct mbox_chan *chan) static void tx_tick(struct mbox_chan *chan, int r) { + struct completion *tx_complete; unsigned long flags; void *mssg; spin_lock_irqsave(&chan->lock, flags); mssg = chan->active_req; + tx_complete = chan->tx_complete; chan->active_req = NULL; + chan->tx_complete = NULL; spin_unlock_irqrestore(&chan->lock, flags); /* Submit next message */ @@ -111,7 +117,7 @@ static void tx_tick(struct mbox_chan *chan, int r) chan->cl->tx_done(chan->cl, mssg, r); if (r != -ETIME && chan->cl->tx_block) - complete(&chan->tx_complete); + complete(tx_complete); } static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer) @@ -272,7 +278,7 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg) else wait = msecs_to_jiffies(chan->cl->tx_tout); - ret = wait_for_completion_timeout(&chan->tx_complete, wait); + ret = wait_for_completion_timeout(&chan->tx_cmpl[t], wait); if (ret == 0) { t = -ETIME; tx_tick(chan, t); @@ -348,7 +354,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index) chan->msg_count = 0; chan->active_req = NULL; chan->cl = cl; - init_completion(&chan->tx_complete); + chan->tx_complete = NULL; if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone) chan->txdone_method |= TXDONE_BY_ACK; @@ -414,6 +420,7 @@ void mbox_free_channel(struct mbox_chan *chan) spin_lock_irqsave(&chan->lock, flags); chan->cl = NULL; chan->active_req = NULL; + chan->tx_complete = NULL; if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK)) chan->txdone_method = TXDONE_BY_POLL; diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c index c5e8b9c..99b0841 100644 --- a/drivers/mailbox/omap-mailbox.c +++ b/drivers/mailbox/omap-mailbox.c @@ -449,7 +449,7 @@ struct mbox_chan *omap_mbox_request_channel(struct mbox_client *cl, chan->msg_count = 0; chan->active_req = NULL; chan->cl = cl; - init_completion(&chan->tx_complete); + chan->tx_complete = NULL; spin_unlock_irqrestore(&chan->lock, flags); ret = chan->mbox->ops->startup(chan); diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c index dd9ecd35..b26cc9c 100644 --- a/drivers/mailbox/pcc.c +++ b/drivers/mailbox/pcc.c @@ -263,7 +263,7 @@ struct mbox_chan *pcc_mbox_request_channel(struct mbox_client *cl, chan->msg_count = 0; chan->active_req = NULL; chan->cl = cl; - init_completion(&chan->tx_complete); + chan->tx_complete = NULL; if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone) chan->txdone_method |= TXDONE_BY_ACK; diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h index 74deadb..aac8659 100644 --- a/include/linux/mailbox_controller.h +++ b/include/linux/mailbox_controller.h @@ -106,11 +106,12 @@ struct mbox_controller { * @mbox: Pointer to the parent/provider of this channel * @txdone_method: Way to detect TXDone chosen by the API * @cl: Pointer to the current owner of this channel - * @tx_complete: Transmission completion + * @tx_complete: Pointer to current transmission completion * @active_req: Currently active request hook * @msg_count: No. of mssg currently queued * @msg_free: Index of next available mssg slot * @msg_data: Hook for data packet + * @tx_cmpl: Per-message completion structure * @lock: Serialise access to the channel * @con_priv: Hook for controller driver to attach private data */ @@ -118,10 +119,11 @@ struct mbox_chan { struct mbox_controller *mbox; unsigned txdone_method; struct mbox_client *cl; - struct completion tx_complete; + struct completion *tx_complete; void *active_req; unsigned msg_count, msg_free; void *msg_data[MBOX_TX_QUEUE_LEN]; + struct completion tx_cmpl[MBOX_TX_QUEUE_LEN]; spinlock_t lock; /* Serialise access to the channel */ void *con_priv; };