spi: race in spi_finalize_current_message starting queue_kthread_work before the message is unmapped

[Mark Brown]

On Sat, May 09, 2015 at 08:40:12AM +0200, Martin Sperl wrote:
> > On 09.05.2015, at 00:13, Mark Brown <broonie at kernel.org> wrote:
> > On Fri, May 08, 2015 at 10:02:44PM +0200, Martin Sperl wrote:

> >> Note that I am not sure about the locking, so I left unmap outside the
> >> lock for now.

> > It needs to be, you can't do anything big like unmapping in a
> > non-sleeping context.

> Well, right now it is not in a locking context but outside, so
> I wonder if it is really needed, but then for spi_finalize_current_message
> it is not specified if it can also get run from interrupt context or not.

I'm having a hard time following the above, sorry.  What is "it" here?

Ideally we do want to be able to finalize in any context, though as
you've seen that's not true for all feature sets.  It's fixable if
someone has a need though.

> * dummy_RX/TX get allocated/released even when can_dma returns false
>   should only run this “mapping if necessary. I guess that it is mostly
>   usefull for DMA transfers - PIO drivers typically implement this via
>   a simple if when tx/rx_buf is NULL.

No, it's supposed to be usable for PIO too - it can keep logic simpler.

> * dummy_tx/rx get allocated/released for every spi_message, which is
>   wasting CPU

Right, ideally we want to push that logic out so that the free happens
when the controller goes idle.

> * we allocate spi_transfer->len bytes to this dummy buffer and with a
>   spi-transfer of 16MB this would try to allocate another 16MB - and possibly
>   fail...
>   Here maybe a fixed allocated (and mapped) buffer of master->max_dma_len
>   could be helpfull that would get reused multiple times in the scatter/gather
>   list to minimize the memory footprint.
>   Ideally there would be support for this inside dma-engine in the form of
>   a flag when calling dmaengine_prep_slave_sg to indicate that data comes
>   from /dev/zero or goes to /dev/null, as some silicon dma-engines
>   (like bcm2835) supports such types of transfers in hardware.

Indeed, just having a page that gets reused would be better.  I'm hoping
that someone who needs this will get around to optimizing it at some
point.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-rpi-kernel/attachments/20150511/179366f4/attachment.sig>