On Wed, Mar 29, 2017 at 5:04 PM, Sudeep Holla <[email protected]> wrote: > > On 28/03/17 19:20, Jassi Brar wrote: >> Hi Sudeep, >> >> On Tue, Mar 21, 2017 at 5:00 PM, Sudeep Holla <[email protected]> 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 waiting for the completion always if Tx >>> is in blocking mode. >>> >>> Fixes: 2b6d83e2b8b7 ("mailbox: Introduce framework for mailbox") >>> Cc: Jassi Brar <[email protected]> >>> Reported-by: Alexey Klimov <[email protected]> >>> Signed-off-by: Sudeep Holla <[email protected]> >>> --- >>> drivers/mailbox/mailbox.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> Hi Jassi, >>> >>> Here are fixes for few issues we encountered when dealing with multiple >>> requests on multiple channels simultaneously. >>> >> Thanks for finding the bug. >> >> I see patch-1 tries to fix the bug. Patch-2,3 try to fix the >> ramifications of the bug but they may change behaviour for some users. >> Do you face any issue even after applying patch-1 ? >> > > Unfortunately yes. Are you concerned with the change in return value on > timeout ? I understand and then I chose -ETIME vs -ETIMEDOUT as hardware > can still use it and we can distinguish the software timer expiry from > that. Even -EIO seems incorrect for s/w timeout as it exists today, but > I agree it has some impact on existing users. > > Also Patch 3 seems independent for me just to avoid complete call if it > was empty message. > > Alexey also brought up another issue which is relating to ordering and > may require completion flags per message instead of per channel. Today > we can't guarantee that first blocker on the wait queue is same as the > first in the mailbox queue. > > e.g.: > Thread#1(T1) Thread#2(T2) > mbox_send_message mbox_send_message > | | > V | > add_to_rbuf(M1) V > | add_to_rbuf(M2) > | | > | V > V msg_submit(picks M1) > msg_submit | > | V > V wait_for_completion(on M2) > wait_for_completion(on M1) | (1st in waitQ) > | (2nd in waitQ) V > V wake_up(on completion of M1)<--incorrect > Yes, that is a possibility. I have sent a fix for this. It would help if Alexey/you could give it a try.
Thanks
