Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934145AbdDFRPf (ORCPT ); Thu, 6 Apr 2017 13:15:35 -0400 Received: from mail-qk0-f176.google.com ([209.85.220.176]:35405 "EHLO mail-qk0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934043AbdDFRP1 (ORCPT ); Thu, 6 Apr 2017 13:15:27 -0400 MIME-Version: 1.0 In-Reply-To: <20170406165806.GA27174@arm.com> References: <1490809381-28869-1-git-send-email-jaswinder.singh@linaro.org> <64c77cc8-7327-d822-5976-c84b3d452ac5@arm.com> <20170406165806.GA27174@arm.com> From: Jassi Brar Date: Thu, 6 Apr 2017 22:45:26 +0530 Message-ID: Subject: Re: [PATCH] mailbox: fix completion order for blocking requests To: Alexey Klimov Cc: Sudeep Holla , Jassi Brar , lkml Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2196 Lines: 59 On 6 April 2017 at 22:28, Alexey Klimov wrote: > Hi Jassi/Sudeep, > > On Wed, Mar 29, 2017 at 07:01:09PM +0100, Sudeep Holla wrote: >> >> >> On 29/03/17 18:43, Jassi Brar wrote: ... >> > 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]); >> >> reinit would be better. > Of course. .... > From: Alexey Klimov > Date: Thu, 6 Apr 2017 13:57:02 +0100 > Subject: [RFC][PATCH] mailbox: per-channel arrays with msg data and completion > structures > > When a mailbox client doesn't serialize sending of the message itself, > and asks mailbox framework to block on mbox_send_message(), one > completion structure per channel is not enough. Client can make a few > mbox_send_message() calls at the same time, and there is no guaranteed > order of going to sleep on completion. > > If mailbox controller acks a message transfer, then tx_tick() wakes up > the first thread that waits on completion. > If mailbox controller doesn't ack the transfer and timeout happens, then > tx_tick() calls complete, and the next caller trying to sleep on > completion wakes up immediately. > > This patch fixes this by changing completion structures to be inserted > into an array that contains a) pointer to data provided by client and > b) the completion structure. Thus active_req field tracks the index of > the current running request that was submitted to mailbox controller. > > Signed-off-by: Alexey Klimov > --- > drivers/mailbox/mailbox.c | 40 +++++++++++++++++++++++--------------- > drivers/mailbox/pcc.c | 10 +++++++--- > include/linux/mailbox_controller.h | 24 +++++++++++++++++------ ... > 3 files changed, 49 insertions(+), 25 deletions(-) > Versus 4 files changed, 17 insertions(+), 8 deletions(-) I think we should just keep it simpler if it works just as fine. Thanks.