Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1650646 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] mailbox: fix completion order for blocking requests Alexey Klimov <alexey.klimov@arm.com> - 2017-05-25 18:50 +0200
    Re: [PATCH] mailbox: fix completion order for blocking requests Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-25 19:40 +0200

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

Hi Jassi,

Sorry for delay -- this is not my main activity so please be patient.

On Sun, Apr 23, 2017 at 03:33:39PM +0530, Jassi Brar wrote:
> On Tue, Apr 11, 2017 at 6:15 PM, Alexey Klimov <alexey.klimov@arm.com> 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@arm.com> 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@arm.com>
> >> > 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@arm.com>
> >> > ---
> >> >  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?

Yes, for such kind of behaviour to make things work I need to apply Sudeep's patches
and this your (or mine) patch that changes completion behaviour.
Or do I miss something and have you already applied it?

Thanks!

Alexey.

[toc] | [next] | [standalone]

#1650698

On Thu, May 25, 2017 at 10:10 PM, Alexey Klimov <alexey.klimov@arm.com> wrote:
> On Sun, Apr 23, 2017 at 03:33:39PM +0530, Jassi Brar wrote:

>
> Sorry for delay -- this is not my main activity so please be patient.
>
No problem.

>> >
>> > 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?
>
> Yes, for such kind of behaviour to make things work I need to apply Sudeep's patches
> and this your (or mine) patch that changes completion behaviour.
> Or do I miss something and have you already applied it?
>
I have already applied three patches from Sudeep. The patch in this
thread is not yet applied, I was hoping to get some tested-by, though
I think it is OK.

Thanks

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web