Re: [PATCH] mailbox: stop the release and reacquire of the chan lock

[Peng Fan <peng.fan@oss.nxp.com>]

Hi Jassi, Tudor
On Sat, Jul 05, 2025 at 09:19:08PM -0500, Jassi Brar wrote:
>On Fri, Jul 4, 2025 at 7:30???AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>>
>> Hi, Jassi,
>>
>> Sorry for the delay, I was out for a while.
>>
>> On 6/23/25 12:41 AM, Jassi Brar wrote:
>> > On Fri, Jun 6, 2025 at 8:41???AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>> >>
>> >> There are two cases where the chan lock is released and reacquired
>> >> were it shouldn't really be:
>> >>
>> >> 1/ released at the end of add_to_rbuf() and reacquired at the beginning
>> >> of msg_submit(). After the lock is released at the end of add_to_rbuf(),
>> >> if the mailbox core is under heavy load, the mailbox software queue may
>> >> fill up without any of the threads getting the chance to drain the
>> >> software queue.
>> >>         T#0 acquires chan lock, fills rbuf, releases the lock, then
>> >>         T#1 acquires chan lock, fills rbuf, releases the lock, then
>> >>         ...
>> >>         T#MBOX_TX_QUEUE_LEN returns -ENOBUFS;
>> >> We shall drain the software queue as fast as we can, while still holding
>> >> the channel lock.
>> >>
>> > I don't see any issue to fix to begin with.
>> > T#0 does drain the queue by moving on to submit the message after
>> > adding it to the rbuf.
>>
>> The problem is that the code releases the chan->lock after adding the
>> message to rbuf and then reacquires it on submit. A thread can be
>> preempted after add_to_rbuf(), without getting the chance to get to
>> msg_submit().
>>
>> Let's assume that
>> T#0 adds to rbuf and gets preempted by T#1
>> T#1 adds to rbuf and gets preempted by T#2
>> ...
>> T#n-1 adds to rbuf and gets preempted by T#n
>>
>> We fill the mailbox software queue without any thread getting to
>> msg_submit().

I try to understand this case. Is this patch from code inspection or
the issue could be reproduced on real platform?

If the client continuously issues 'mbox_send_message',
there might be possibility that 'msg_submit' does not have chance to get
get chan lock.

>>
>I get that but I still don't see a problem.
>When the queue gets filled the next submission will be rejected and
>have to wait until the mailbox gets some work done. Which is the
>expected behaviour. And will be the same even if we don't release the
>lock between add_to_rbug and and msg_submit and there are enough
>mbox_send_message calls coming in faster than the transmission bus.

Not sure I get it clear, do you mean this?
When the mailbox queue is full, any new message submission will be rejected.
The sender must wait until space becomes available as messages are processed.

This is the expected behavior and ensures proper flow control. Even if we
avoid releasing the lock between add_to_rbuf() and msg_submit(), the outcome
remains the same: if mbox_send_message() calls arrive faster than the bus can
transmit, the queue will eventually fill up and block further submissions.

Thanks,
Peng

>Thanks.
>