Re: [PATCH 03/15] can: m_can: Cache tx putidx and transmits in flight

[Marc Kleine-Budde]

On 02.12.2022 09:37:40, Markus Schneider-Pargmann wrote:
> Hi Marc,
> 
> On Thu, Dec 01, 2022 at 12:14:50PM +0100, Marc Kleine-Budde wrote:
> > On 16.11.2022 21:52:56, Markus Schneider-Pargmann wrote:
> > > On peripheral chips every read/write can be costly. Avoid reading easily
> > > trackable information and cache them internally. This saves multiple
> > > reads.
> > > 
> > > Transmit FIFO put index is cached, this is increased for every time we
> > > enqueue a transmit request.
> > > 
> > > The transmits in flight is cached as well. With each transmit request it
> > > is increased when reading the finished transmit event it is decreased.
> > > 
> > > A submit limit is cached to avoid submitting too many transmits at once,
> > > either because the TX FIFO or the TXE FIFO is limited. This is currently
> > > done very conservatively as the minimum of the fifo sizes. This means we
> > > can reach FIFO full events but won't drop anything.
> > 
> > You have a dedicated in_flight variable, which is read-modify-write in 2
> > different code path, i.e. this looks racy.
> 
> True, of course, thank you. Yes I have to redesign this a bit for
> concurrency.
> 
> > If you allow only power-of-two FIFO size, you can use 2 unsigned
> > variables, i.e. a head and a tail pointer. You can apply a mask to get
> > the index to the FIFO. The difference between head and tail is the fill
> > level of the FIFO. See mcp251xfd driver for this.
> 
> Maybe that is a trivial question but what's wrong with using atomics
> instead?

I think it's Ok to use an atomic for the fill level. The put index
doesn't need to be. No need to cache the get index, as it's in the same
register as the fill level.

As the mcp251xfd benefits from caching both indexes, a head and tail
pointer felt like the right choice. As both are only written in 1
location, no need for atomics or locking.

> The tcan mram size is limited to 2048 so I would like to avoid limiting
> the possible sizes of the tx fifos.

What FIFO sizes are you using currently?

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: PGP signature