| Message ID | 1490024410-10287-1-git-send-email-sudeep.holla@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Sudeep, thanks for sending this patch. On Mon, Mar 20, 2017 at 03:40:10PM +0000, Sudeep Holla wrote: > There exists a race when msg_submit return immediately as there was an > active request being processed which may have completed just before it's > checked again in mbox_send_message. This will result in return to the > caller without waiting in mbox_send_message even when it's blocking Tx. > > This patch fixes the issue by making use of non-negative token returned > by add_to_rbuf to check if the request was queued and block always if > so in blocking Tx mode. > > Fixes: 2b6d83e2b8b7 ("mailbox: Introduce framework for mailbox") > Cc: Jassi Brar <jassisinghbrar@gmail.com> > Reported-by: Alexey Klimov <alexey.klimov@arm.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > drivers/mailbox/mailbox.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c > index 4671f8a12872..d5895791ab5d 100644 > --- a/drivers/mailbox/mailbox.c > +++ b/drivers/mailbox/mailbox.c > @@ -260,7 +260,7 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg) > > msg_submit(chan); > > - if (chan->cl->tx_block && chan->active_req) { > + if (chan->cl->tx_block && t >= 0) { What do you think about removing t>=0 at all? If add_to_rbuf() above returns negative number then we won't reach this point in code at all and quit this function with error. If execution reaches this line then we can say that t is definetely >= 0 and maybe it shouldn't be checked. Best regards, Alexey
On 20/03/17 18:47, Alexey Klimov wrote: > Hi Sudeep, > > thanks for sending this patch. > > On Mon, Mar 20, 2017 at 03:40:10PM +0000, Sudeep Holla wrote: >> There exists a race when msg_submit return immediately as there was an >> active request being processed which may have completed just before it's >> checked again in mbox_send_message. This will result in return to the >> caller without waiting in mbox_send_message even when it's blocking Tx. >> >> This patch fixes the issue by making use of non-negative token returned >> by add_to_rbuf to check if the request was queued and block always if >> so in blocking Tx mode. >> >> Fixes: 2b6d83e2b8b7 ("mailbox: Introduce framework for mailbox") >> Cc: Jassi Brar <jassisinghbrar@gmail.com> >> Reported-by: Alexey Klimov <alexey.klimov@arm.com> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >> --- >> drivers/mailbox/mailbox.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c >> index 4671f8a12872..d5895791ab5d 100644 >> --- a/drivers/mailbox/mailbox.c >> +++ b/drivers/mailbox/mailbox.c >> @@ -260,7 +260,7 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg) >> >> msg_submit(chan); >> >> - if (chan->cl->tx_block && chan->active_req) { >> + if (chan->cl->tx_block && t >= 0) { > > What do you think about removing t>=0 at all? > If add_to_rbuf() above returns negative number then we won't reach this point > in code at all and quit this function with error. If execution reaches this line then > we can say that t is definetely >= 0 and maybe it shouldn't be checked. Ah right, sorry I missed to see that, will fix it. -- Regards, Sudeep
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c index 4671f8a12872..d5895791ab5d 100644 --- a/drivers/mailbox/mailbox.c +++ b/drivers/mailbox/mailbox.c @@ -260,7 +260,7 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg) msg_submit(chan); - if (chan->cl->tx_block && chan->active_req) { + if (chan->cl->tx_block && t >= 0) { unsigned long wait; int ret;
There exists a race when msg_submit return immediately as there was an active request being processed which may have completed just before it's checked again in mbox_send_message. This will result in return to the caller without waiting in mbox_send_message even when it's blocking Tx. This patch fixes the issue by making use of non-negative token returned by add_to_rbuf to check if the request was queued and block always if so in blocking Tx mode. Fixes: 2b6d83e2b8b7 ("mailbox: Introduce framework for mailbox") Cc: Jassi Brar <jassisinghbrar@gmail.com> Reported-by: Alexey Klimov <alexey.klimov@arm.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/mailbox/mailbox.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.7.4