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: <20250930024146.GA17703@nxa18884-linux.ap.freescale.net>
Date: Tue, 30 Sep 2025 10:41:46 +0800
From: Peng Fan <peng.fan@....nxp.com>
To: Tanmay Shah <tanmay.shah@....com>,
	Mathieu Poirier <mathieu.poirier@...aro.org>
Cc: Mathieu Poirier <mathieu.poirier@...aro.org>, jassisinghbrar@...il.com,
	andersson@...nel.org, linux-kernel@...r.kernel.org,
	linux-remoteproc@...r.kernel.org
Subject: Re: [PATCH] mailbox: check mailbox queue is full or not

On Mon, Sep 29, 2025 at 02:26:01PM -0500, Tanmay Shah wrote:
>> > > > 
>> > > > > +	res = (chan->msg_count == (MBOX_TX_QUEUE_LEN - 1));
>> 
>> Please have a look at this condition again - the implementation of
>> addr_to_rbuf() [1] is checking for space differently.

Tanmay,

May I know why you want to compare with "MBOX_TX_QUEUE_LEN - 1",
not "MBOX_TX_QUEUE_LEN"?

>> 
>> [1]. https://elixir.bootlin.com/linux/v6.17/source/drivers/mailbox/mailbox.c#L32
>> 
>> > > > > +	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.
>> > 
>> > I think check whether mbox_send_message returns -ENOBUFS or not should
>> > work for you. If the "Try increasing MBOX_TX_QUEUE_LEN" message
>> > bothers you, it could be update to dev_dbg per my understanding.
>> > 
>> 
>> This new API is trying to avoid calling mbox_send_message(), no checking if it
>> succeeded or not. 

I think it may not deserve to introduce a new API. add_to_rbuf is almost
the first function that mbox_send_message calls. But if Tanmay insists on
adding a new API, I am fine. Jassi may comment more. 

>> Moving dev_err() nto dev_dbg() is also the wrong approach.
>> 
>
>Correct.

The caller of mbox_send_message detect error value and choose
to add dev_err or not in caller driver, so I think dev_dbg is fine here.

I would appreciate if there is explaination on why dev_dbg is not correct :)

Thanks,
Peng

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