SPI, priority inversion tweak

This is an updated version of the patch from Mark Underwood, handling the no-memory case better and using SLAB_KERNEL not SLAB_ATOMIC.

Please apply it on top of the current SPI code in the MM tree.

- Dave

------------------------------------------------------------------------

Update the SPI framework to remove a potential priority inversion case by reverting to kmalloc if the pre-allocated DMA-safe buffer isn't available.

From: Mark Underwood <[email protected]>

Signed-off-by David Brownell <[email protected]>

drivers/spi/spi.c +18 -6

@@ -541,22 +541,30 @@ int spi_write_then_read(struct spi_devic
 	int			status;
 	struct spi_message	message;
 	struct spi_transfer	x[2];
+	u8			*local_buf;
 
 	/* Use preallocated DMA-safe buffer.  We can't avoid copying here,
 	 * (as a pure convenience thing), but we can keep heap costs
-	 * out of the hot path.
+	 * out of the hot path ...
 	 */
 	if ((n_tx + n_rx) > SPI_BUFSIZ)
 		return -EINVAL;
 
-	down(&lock);
+	/* ... unless someone else is using the pre-allocated buffer */
+	if (down_trylock(&lock)) {
+		local_buf = kmalloc(SPI_BUFSIZ, GFP_KERNEL);
+		if (!local_buf)
+			return -ENOMEM;
+	} else
+		local_buf = buf;
+
 	memset(x, 0, sizeof x);
 
-	memcpy(buf, txbuf, n_tx);
-	x[0].tx_buf = buf;
+	memcpy(local_buf, txbuf, n_tx);
+	x[0].tx_buf = local_buf;
 	x[0].len = n_tx;
 
-	x[1].rx_buf = buf + n_tx;
+	x[1].rx_buf = local_buf + n_tx;
 	x[1].len = n_rx;
 
 	/* do the i/o */

@@ -568,7 +576,11 @@ int spi_write_then_read(struct spi_devic
 		status = message.status;
 	}
 
-	up(&lock);
+	if (x[0].tx_buf == buf)
+		up(&lock);
+	else
+		kfree(local_buf);
+
 	return status;
 }
 EXPORT_SYMBOL_GPL(spi_write_then_read);

So you're turning this to be unsafe if the buffer is in use, right? Funny...

Quoted 3 lines · David Brownell

This is an updated version of the patch from Mark Underwood, handling the no-memory case better and using SLAB_KERNEL not SLAB_ATOMIC.

Quoted 3 lines · Vitaly Wool

So you're turning this to be unsafe if the buffer is in use, right? Funny...

I have no idea what you mean by that comment. The parameters to that function have always been documented as "will copy", and the two branches (busy/not) differ only in _which_ buffer they use from the heap (the fast pre-allocated one, or a freshly allocated scratch buffer). Heap buffers are by definition DMA-safe.

- Dave

Quoted 9 lines

Quoted 5 lines · David Brownell

This is an updated version of the patch from Mark Underwood, handling the no-memory case better and using SLAB_KERNEL not SLAB_ATOMIC.