On Tue, Jul 21, 2020 at 03:39:44PM +0200, Thomas Petazzoni wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > > I'm not sure what you mean by "it might want to consider error
> > > checking". Could you explain?  

> > It wasn't checking the return values of SPI API calls.

> Right, but this is unlikely to be the source of the CPU consumption
> issue we're seeing.

Indeed, that was just something I noticed while reviewing.

> > Actually looking at the code in __spi_pump_messages() again I think that
> > in the case where we don't have any cleanup to do we should be able to
> > avoid kicking the thread for that which should help a bit for spi-atmel.
> > Can you give the patch below a go (compile tested only, not even tried
> > to boot)?

> I gave it a try, and unfortunately there is no difference. But it is
> not too surprising, as top shows something like this:

>    80     2 root     SW       0   0%  24% [irq/50-spi1.3]
>    57     2 root     IW       0   0%   2% [kworker/0:1-eve]

> So it's not the kthread of the SPI subsystem that is consuming most of
> the CPU time, but really the threaded handler of the MAX14830 driver.
> What your patch does is avoid offloading to the SPI subsystem kthread
> some cleanup work when it can be done in the current context, if I
> understand correctly.

It does, yes.  Depending on the particular performance characteristics
of the system it can sometimes help noticably to cut out those context
thrashes if just the context switching ends up being a noticable
overhead, regardless of how much work each thread does when it does get
control.  In any case that's now merged in -next, I got some testing on
other boards, so hopefully it'll help someone.

> > You can generally get a good idea of what's going on with regard to
> > context switching at the SPI level from the SPI tracepoints, and about
> > any latencies in there too.

> I'll have a look there, but I don't really have any latency issue,
> rather a CPU consumption issue, which is quite different.

Yeah, that's basically where the overhead in the SPI subsystem itself
(rather than the specific driver) tends to be unfortunately.