Re: [spi:for-4.20 83/83] include/linux/compiler.h:358:38: error: call to '__compiletime_assert_619' declared with attribute error: BUILD_BUG failed

[Lukas Wunner <lukas@xxxxxxxxx>]

Hi Mark,

On Thu, Nov 29, 2018 at 06:23:28AM +0800, kbuild test robot wrote:
> commit: e82b0b3828451c1cd331d9f304c6078fcd43b62e
>         [83/83] spi: bcm2835: Fix race on DMA termination

I was going to post a v2 of this series tomorrow, sorry for the delay.
I notice you've applied v1 to your for-4.20 and for-4.21 branches today.

These two commits on the for-4.20 branch are fine:
    spi: bcm2835: Avoid finishing transfer prematurely in IRQ mode
    spi: bcm2835: Fix book-keeping of DMA termination

They're missing these tags but no big deal:
    Tested-by: Eric Anholt <eric@xxxxxxxxxx>
    Reviewed-by: Martin Sperl <kernel@xxxxxxxxxxxxxxxx>

The third commit on the for-4.20 branch is broken, as 0-day has
correctly reported.  The rectified patch for v2 is below.

These two commits on the for-4.21 branch are fine (save for the tags above):
    spi: bcm2835: Drop unused code for native Chip Select
    spi: bcm2835: Document struct bcm2835_spi

The third commit on the for-4.21 branch I've improved slightly for
v2.  The series contained a seventh commit which has not been applied
to for-4.21 yet.

Please let me know how to proceed, should I post the whole series or
omit the two patches on for-4.20 which are fine or...?  Again, sorry
for not finalizing v2 earlier, I had to do some final tests which I
only got to today.

Thanks!

Lukas

-- >8 --
Subject: [PATCH 3/8] spi: bcm2835: Fix race on DMA termination
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If a DMA transfer finishes orderly right when spi_transfer_one_message()
determines that it has timed out, the callbacks bcm2835_spi_dma_done()
and bcm2835_spi_handle_err() race to call dmaengine_terminate_all(),
potentially leading to double termination.

Prevent by atomically changing the dma_pending flag with cmpxchg()
before calling dmaengine_terminate_all().

Change the flag's type from bool to unsigned int to avoid breaking the
build with COMPILE_TEST=y on arches whose cmpxchg() requires 32-bit
operands (xtensa, older arm ISAs).

Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
Reviewed-by: Martin Sperl <kernel@xxxxxxxxxxxxxxxx>
Tested-by: Eric Anholt <eric@xxxxxxxxxx>
Fixes: 3ecd37edaa2a ("spi: bcm2835: enable dma modes for transfers meeting certain conditions")
Cc: stable@xxxxxxxxxxxxxxx # v4.2+
Cc: Frank Pavlic <f.pavlic@xxxxxxxxx>
Cc: Noralf TrÞnnes <noralf@xxxxxxxxxxx>
---
 drivers/spi/spi-bcm2835.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 5c6dd19..23ec074 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -88,7 +88,7 @@ struct bcm2835_spi {
 	u8 *rx_buf;
 	int tx_len;
 	int rx_len;
-	bool dma_pending;
+	unsigned int dma_pending;
 };
 
 static inline u32 bcm2835_rd(struct bcm2835_spi *bs, unsigned reg)
@@ -232,10 +232,9 @@ static void bcm2835_spi_dma_done(void *data)
 	 * is called the tx-dma must have finished - can't get to this
 	 * situation otherwise...
 	 */
-	dmaengine_terminate_all(master->dma_tx);
-
-	/* mark as no longer pending */
-	bs->dma_pending = 0;
+	if (cmpxchg(&bs->dma_pending, true, false)) {
+		dmaengine_terminate_all(master->dma_tx);
+	}
 
 	/* and mark as completed */;
 	complete(&master->xfer_completion);
@@ -617,10 +616,9 @@ static void bcm2835_spi_handle_err(struct spi_master *master,
 	struct bcm2835_spi *bs = spi_master_get_devdata(master);
 
 	/* if an error occurred and we have an active dma, then terminate */
-	if (bs->dma_pending) {
+	if (cmpxchg(&bs->dma_pending, true, false)) {
 		dmaengine_terminate_all(master->dma_tx);
 		dmaengine_terminate_all(master->dma_rx);
-		bs->dma_pending = 0;
 	}
 	/* and reset */
 	bcm2835_spi_reset_hw(master);
-- 
2.19.2