Re: [PATCH v4] mailbox: Make mbox_send_message() return error code when tx fails

[Joonwon Kang <joonwonkang@google.com>]

Hi Sudeep, I appreciate your review! And I apologize that I missed some
important context about this patch.

> On Tue, Apr 21, 2026 at 10:46:52AM +0000, Joonwon Kang wrote:
> > When the mailbox controller failed transmitting message, the error code
> > was only passed to the client's tx done handler and not to
> > mbox_send_message() in blocking mode. For this reason, the function could
> > return a false success. This commit resolves the issue by introducing the
> > tx status and checking it before mbox_send_message() returns.
> >
> `tx_complete` and `tx_status` are per-channel, not per-message. Although
> `mbox_send_message()` can queue multiple messages, all blocking callers wait
> on the same completion, so a completion is not associated with the thread or
> message that triggered it.
> 
> This creates two issues:
> 
> 1. Concurrent blocking senders can consume each other’s completions. When
>    message A completes, `tx_tick()` may submit message B, then set
>    `chan->tx_status` and complete the shared completion. Any waiter may wake,
>    including B’s sender, which can return while B is still in flight. It
>    happens even w/o this change but with possibly wrong return value after
>    this change.
> 
> 2. `tx_status` can be stale or overwritten. Since it is a single channel field
>    written just before `complete()`, a second(possibly fast) `tx_tick()` can
>    update it before the first awakened sender reads it. Because `msg_submit()`
>    happens before status publication, the next message can complete before the
>    previous status is observed if the controller re-enters `tx_tick()` for the
>    same channel.
> 
> We need to see if there are other issue that needs fixing before you can
> propagate the tx error code. Let me know if I am missing something.

Yes, the current mbox_send_message() in blocking mode does not support
multi-threads. I have tried adding the multi-threads support [1] since the
first patchset and adding this patch on top of it [2], but the author was
not convinced about the necessity of the multi-threads support and instead
preferred that clients, instead of the mailbox APIs, serialize the multiple
threads' access to the channel [3].

For this reason, I went with the author's preference [4] and clarified that
multi-threads is not supported in the API doc [5] so that clients can be
clearly aware of it and serialize its threads' access to the channel.

So, this patch is based on the assumption that such multi-threads
protection is given by the clients already, i.e. mbox_send_message() in
blocking mode is called on the same channel only when the previous call has
returned.

What is your opinion on this? Should we support multi-threads in the mailbox
APIs [1]? or should we go with the current decision [5]? I personally have
been thinking the former is the way to go.

[1] https://lore.kernel.org/all/20260402170641.2082547-1-joonwonkang@google.com
[2] https://lore.kernel.org/all/20260402170641.2082547-3-joonwonkang@google.com/
[3] https://lore.kernel.org/all/CABb+yY0uDQh-3cadPQONV=NJKjMtc4mJekgjmHYVaHnfHXvGZQ@mail.gmail.com/
[4] https://lore.kernel.org/all/20260404124428.3077670-1-joonwonkang@google.com/
[5] https://lore.kernel.org/all/20260421104652.211276-1-joonwonkang@google.com/

Thanks,
Joonwon Kang