Re: [PATCH] mailbox: fix completion order for blocking requests

[Jassi Brar]

On Tue, Apr 11, 2017 at 6:15 PM, Alexey Klimov <alexey.klimov@xxxxxxx> wrote:
> On Thu, Apr 06, 2017 at 10:45:26PM +0530, Jassi Brar wrote:
>> On 6 April 2017 at 22:28, Alexey Klimov <alexey.klimov@xxxxxxx> 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 <alexey.klimov@xxxxxxx>
>> > 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 <alexey.klimov@xxxxxxx>
>> > ---
>> >  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.
>
> Along with this patch you still need at least one patch from Sudeep with subject:
> "[PATCH 1/3] mailbox: always wait in mbox_send_message for blocking Tx mode"
>
Yes. Just so we are on same page, can you please redo your tests and
see if this and Sudeep's patch-1/3 does the trick?

Thanks