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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJe_ZhdV_5zxSygOBngqxc86GVWgRNjjZtNTL+dHQtwa=qgruA@mail.gmail.com>
Date:   Thu, 6 Apr 2017 22:45:26 +0530
From:   Jassi Brar <jaswinder.singh@...aro.org>
To:     Alexey Klimov <alexey.klimov@....com>
Cc:     Sudeep Holla <sudeep.holla@....com>,
        Jassi Brar <jassisinghbrar@...il.com>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mailbox: fix completion order for blocking requests

On 6 April 2017 at 22:28, Alexey Klimov <alexey.klimov@....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@....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@....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.

Thanks.