linux-kernel - Re: [PATCH] mailbox: check mailbox queue is full or not

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] [day] [month] [year] [list]

Message-ID: <e93f0ee7-687a-4f47-a847-90cc1ea87290@amd.com>
Date: Fri, 26 Sep 2025 10:40:09 -0500
From: Tanmay Shah <tanmay.shah@....com>
To: Peng Fan <peng.fan@....nxp.com>
CC: <jassisinghbrar@...il.com>, <andersson@...nel.org>,
	<mathieu.poirier@...aro.org>, <linux-kernel@...r.kernel.org>,
	<linux-remoteproc@...r.kernel.org>
Subject: Re: [PATCH] mailbox: check mailbox queue is full or not

Hi Peng,

Thanks for reviews. Please find my comments below.

On 9/26/25 2:37 AM, Peng Fan wrote:
> Hi Tanmay,
> 
> On Thu, Sep 25, 2025 at 11:50:44AM -0700, Tanmay Shah wrote:
>> Sometimes clients need to know if mailbox queue is full or not before
>> posting new message via mailbox. If mailbox queue is full clients can
>> choose not to post new message. This doesn't mean current queue length
>> should be increased, but clients may want to wait till previous Tx is
>> done. This API can help avoid false positive warning from mailbox
>> framework "Try increasing MBOX_TX_QUEUE_LEN".
>>
>> Signed-off-by: Tanmay Shah <tanmay.shah@....com>
>> ---
>> drivers/mailbox/mailbox.c               | 24 ++++++++++++++++++++++++
>> drivers/remoteproc/xlnx_r5_remoteproc.c |  4 ++++
>> include/linux/mailbox_client.h          |  1 +
> 
> The mailbox and remoteproc should be separated.
> 

Mailbox framework is introducing new API. I wanted the use case to be in 
the same patch-set, otherwise we might see unused API warning.

Hence, both in the same patch makes more sense. If maintainers prefer, I 
will separate them.

>> 3 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
>> index 5cd8ae222073..7afdb2c9006d 100644
>> --- a/drivers/mailbox/mailbox.c
>> +++ b/drivers/mailbox/mailbox.c
>> @@ -217,6 +217,30 @@ bool mbox_client_peek_data(struct mbox_chan *chan)
>> }
>> EXPORT_SYMBOL_GPL(mbox_client_peek_data);
>>
>> +/**
>> + * mbox_queue_full - check if mailbox queue is full or not
>> + * @chan: Mailbox channel assigned to this client.
>> + *
>> + * Clients can choose not to send new msg if mbox queue is full.
>> + *
>> + * Return: true if queue is full else false. < 0 for error
>> + */
>> +int mbox_queue_full(struct mbox_chan *chan)
>> +{
>> +	unsigned long flags;
>> +	int res;
>> +
>> +	if (!chan)
>> +		return -EINVAL;
>> +
>> +	spin_lock_irqsave(&chan->lock, flags);
> 
> Use scoped_guard.

Other APIs use spin_lock_irqsave. Probably scoped_guard should be 
introduced in a different patch for all APIs in the mailbox.

> 
>> +	res = (chan->msg_count == (MBOX_TX_QUEUE_LEN - 1));
>> +	spin_unlock_irqrestore(&chan->lock, flags);
>> +
>> +	return res;
>> +}
>> +EXPORT_SYMBOL_GPL(mbox_queue_full);
> 
> add_to_rbuf is able to return ENOBUFS when call mbox_send_message.
> Does checking mbox_send_message return value works for you?
> 

That is the problem. mbox_send_message uses add_to_rbuf and fails. But 
during failure, it prints warning message:

dev_err(chan->mbox->dev, "Try increasing MBOX_TX_QUEUE_LEN\n");

In some cases there are lot of such messages on terminal. Functionally 
nothing is wrong and everything is working but user keeps getting false 
positive warning about increasing mbox tx queue length. That is why we 
need API to check if mbox queue length is full or not before doing 
mbox_send_message. Not all clients need to use it, but some cane make 
use of it.


>> +
>> /**
>>   * mbox_send_message -	For client to submit a message to be
>>   *				sent to the remote.
> 
> Regards
> Peng