You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
282 lines
12 KiB
282 lines
12 KiB
From b36f09c3c441a6e59eab9315032e7d546571de3f Mon Sep 17 00:00:00 2001
|
|
From: Lars-Peter Clausen <lars@metafoo.de>
|
|
Date: Tue, 20 Oct 2015 11:46:28 +0200
|
|
Subject: [PATCH] dmaengine: Add transfer termination synchronization support
|
|
|
|
The DMAengine API has a long standing race condition that is inherent to
|
|
the API itself. Calling dmaengine_terminate_all() is supposed to stop and
|
|
abort any pending or active transfers that have previously been submitted.
|
|
Unfortunately it is possible that this operation races against a currently
|
|
running (or with some drivers also scheduled) completion callback.
|
|
|
|
Since the API allows dmaengine_terminate_all() to be called from atomic
|
|
context as well as from within a completion callback it is not possible to
|
|
synchronize to the execution of the completion callback from within
|
|
dmaengine_terminate_all() itself.
|
|
|
|
This means that a user of the DMAengine API does not know when it is safe
|
|
to free resources used in the completion callback, which can result in a
|
|
use-after-free race condition.
|
|
|
|
This patch addresses the issue by introducing an explicit synchronization
|
|
primitive to the DMAengine API called dmaengine_synchronize().
|
|
|
|
The existing dmaengine_terminate_all() is deprecated in favor of
|
|
dmaengine_terminate_sync() and dmaengine_terminate_async(). The former
|
|
aborts all pending and active transfers and synchronizes to the current
|
|
context, meaning it will wait until all running completion callbacks have
|
|
finished. This means it is only possible to call this function from
|
|
non-atomic context. The later function does not synchronize, but can still
|
|
be used in atomic context or from within a complete callback. It has to be
|
|
followed up by dmaengine_synchronize() before a client can free the
|
|
resources used in a completion callback.
|
|
|
|
In addition to this the semantics of the device_terminate_all() callback
|
|
are slightly relaxed by this patch. It is now OK for a driver to only
|
|
schedule the termination of the active transfer, but does not necessarily
|
|
have to wait until the DMA controller has completely stopped. The driver
|
|
must ensure though that the controller has stopped and no longer accesses
|
|
any memory when the device_synchronize() callback returns.
|
|
|
|
This was in part done since most drivers do not pay attention to this
|
|
anyway at the moment and to emphasize that this needs to be done when the
|
|
device_synchronize() callback is implemented. But it also helps with
|
|
implementing support for devices where stopping the controller can require
|
|
operations that may sleep.
|
|
|
|
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
|
|
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
|
|
---
|
|
Documentation/dmaengine/client.txt | 38 ++++++++++++++-
|
|
Documentation/dmaengine/provider.txt | 20 +++++++-
|
|
drivers/dma/dmaengine.c | 5 +-
|
|
include/linux/dmaengine.h | 90 ++++++++++++++++++++++++++++++++++++
|
|
4 files changed, 148 insertions(+), 5 deletions(-)
|
|
|
|
--- a/Documentation/dmaengine/client.txt
|
|
+++ b/Documentation/dmaengine/client.txt
|
|
@@ -117,7 +117,7 @@ The slave DMA usage consists of followin
|
|
transaction.
|
|
|
|
For cyclic DMA, a callback function may wish to terminate the
|
|
- DMA via dmaengine_terminate_all().
|
|
+ DMA via dmaengine_terminate_async().
|
|
|
|
Therefore, it is important that DMA engine drivers drop any
|
|
locks before calling the callback function which may cause a
|
|
@@ -155,12 +155,29 @@ The slave DMA usage consists of followin
|
|
|
|
Further APIs:
|
|
|
|
-1. int dmaengine_terminate_all(struct dma_chan *chan)
|
|
+1. int dmaengine_terminate_sync(struct dma_chan *chan)
|
|
+ int dmaengine_terminate_async(struct dma_chan *chan)
|
|
+ int dmaengine_terminate_all(struct dma_chan *chan) /* DEPRECATED */
|
|
|
|
This causes all activity for the DMA channel to be stopped, and may
|
|
discard data in the DMA FIFO which hasn't been fully transferred.
|
|
No callback functions will be called for any incomplete transfers.
|
|
|
|
+ Two variants of this function are available.
|
|
+
|
|
+ dmaengine_terminate_async() might not wait until the DMA has been fully
|
|
+ stopped or until any running complete callbacks have finished. But it is
|
|
+ possible to call dmaengine_terminate_async() from atomic context or from
|
|
+ within a complete callback. dmaengine_synchronize() must be called before it
|
|
+ is safe to free the memory accessed by the DMA transfer or free resources
|
|
+ accessed from within the complete callback.
|
|
+
|
|
+ dmaengine_terminate_sync() will wait for the transfer and any running
|
|
+ complete callbacks to finish before it returns. But the function must not be
|
|
+ called from atomic context or from within a complete callback.
|
|
+
|
|
+ dmaengine_terminate_all() is deprecated and should not be used in new code.
|
|
+
|
|
2. int dmaengine_pause(struct dma_chan *chan)
|
|
|
|
This pauses activity on the DMA channel without data loss.
|
|
@@ -186,3 +203,20 @@ Further APIs:
|
|
a running DMA channel. It is recommended that DMA engine users
|
|
pause or stop (via dmaengine_terminate_all()) the channel before
|
|
using this API.
|
|
+
|
|
+5. void dmaengine_synchronize(struct dma_chan *chan)
|
|
+
|
|
+ Synchronize the termination of the DMA channel to the current context.
|
|
+
|
|
+ This function should be used after dmaengine_terminate_async() to synchronize
|
|
+ the termination of the DMA channel to the current context. The function will
|
|
+ wait for the transfer and any running complete callbacks to finish before it
|
|
+ returns.
|
|
+
|
|
+ If dmaengine_terminate_async() is used to stop the DMA channel this function
|
|
+ must be called before it is safe to free memory accessed by previously
|
|
+ submitted descriptors or to free any resources accessed within the complete
|
|
+ callback of previously submitted descriptors.
|
|
+
|
|
+ The behavior of this function is undefined if dma_async_issue_pending() has
|
|
+ been called between dmaengine_terminate_async() and this function.
|
|
--- a/Documentation/dmaengine/provider.txt
|
|
+++ b/Documentation/dmaengine/provider.txt
|
|
@@ -327,8 +327,24 @@ supported.
|
|
|
|
* device_terminate_all
|
|
- Aborts all the pending and ongoing transfers on the channel
|
|
- - This command should operate synchronously on the channel,
|
|
- terminating right away all the channels
|
|
+ - For aborted transfers the complete callback should not be called
|
|
+ - Can be called from atomic context or from within a complete
|
|
+ callback of a descriptor. Must not sleep. Drivers must be able
|
|
+ to handle this correctly.
|
|
+ - Termination may be asynchronous. The driver does not have to
|
|
+ wait until the currently active transfer has completely stopped.
|
|
+ See device_synchronize.
|
|
+
|
|
+ * device_synchronize
|
|
+ - Must synchronize the termination of a channel to the current
|
|
+ context.
|
|
+ - Must make sure that memory for previously submitted
|
|
+ descriptors is no longer accessed by the DMA controller.
|
|
+ - Must make sure that all complete callbacks for previously
|
|
+ submitted descriptors have finished running and none are
|
|
+ scheduled to run.
|
|
+ - May sleep.
|
|
+
|
|
|
|
Misc notes (stuff that should be documented, but don't really know
|
|
where to put them)
|
|
--- a/drivers/dma/dmaengine.c
|
|
+++ b/drivers/dma/dmaengine.c
|
|
@@ -266,8 +266,11 @@ static void dma_chan_put(struct dma_chan
|
|
module_put(dma_chan_to_owner(chan));
|
|
|
|
/* This channel is not in use anymore, free it */
|
|
- if (!chan->client_count && chan->device->device_free_chan_resources)
|
|
+ if (!chan->client_count && chan->device->device_free_chan_resources) {
|
|
+ /* Make sure all operations have completed */
|
|
+ dmaengine_synchronize(chan);
|
|
chan->device->device_free_chan_resources(chan);
|
|
+ }
|
|
|
|
/* If the channel is used via a DMA request router, free the mapping */
|
|
if (chan->router && chan->router->route_free) {
|
|
--- a/include/linux/dmaengine.h
|
|
+++ b/include/linux/dmaengine.h
|
|
@@ -681,6 +681,8 @@ struct dma_filter {
|
|
* paused. Returns 0 or an error code
|
|
* @device_terminate_all: Aborts all transfers on a channel. Returns 0
|
|
* or an error code
|
|
+ * @device_synchronize: Synchronizes the termination of a transfers to the
|
|
+ * current context.
|
|
* @device_tx_status: poll for transaction completion, the optional
|
|
* txstate parameter can be supplied with a pointer to get a
|
|
* struct with auxiliary transfer status information, otherwise the call
|
|
@@ -765,6 +767,7 @@ struct dma_device {
|
|
int (*device_pause)(struct dma_chan *chan);
|
|
int (*device_resume)(struct dma_chan *chan);
|
|
int (*device_terminate_all)(struct dma_chan *chan);
|
|
+ void (*device_synchronize)(struct dma_chan *chan);
|
|
|
|
enum dma_status (*device_tx_status)(struct dma_chan *chan,
|
|
dma_cookie_t cookie,
|
|
@@ -856,6 +859,13 @@ static inline struct dma_async_tx_descri
|
|
src_sg, src_nents, flags);
|
|
}
|
|
|
|
+/**
|
|
+ * dmaengine_terminate_all() - Terminate all active DMA transfers
|
|
+ * @chan: The channel for which to terminate the transfers
|
|
+ *
|
|
+ * This function is DEPRECATED use either dmaengine_terminate_sync() or
|
|
+ * dmaengine_terminate_async() instead.
|
|
+ */
|
|
static inline int dmaengine_terminate_all(struct dma_chan *chan)
|
|
{
|
|
if (chan->device->device_terminate_all)
|
|
@@ -864,6 +874,86 @@ static inline int dmaengine_terminate_al
|
|
return -ENOSYS;
|
|
}
|
|
|
|
+/**
|
|
+ * dmaengine_terminate_async() - Terminate all active DMA transfers
|
|
+ * @chan: The channel for which to terminate the transfers
|
|
+ *
|
|
+ * Calling this function will terminate all active and pending descriptors
|
|
+ * that have previously been submitted to the channel. It is not guaranteed
|
|
+ * though that the transfer for the active descriptor has stopped when the
|
|
+ * function returns. Furthermore it is possible the complete callback of a
|
|
+ * submitted transfer is still running when this function returns.
|
|
+ *
|
|
+ * dmaengine_synchronize() needs to be called before it is safe to free
|
|
+ * any memory that is accessed by previously submitted descriptors or before
|
|
+ * freeing any resources accessed from within the completion callback of any
|
|
+ * perviously submitted descriptors.
|
|
+ *
|
|
+ * This function can be called from atomic context as well as from within a
|
|
+ * complete callback of a descriptor submitted on the same channel.
|
|
+ *
|
|
+ * If none of the two conditions above apply consider using
|
|
+ * dmaengine_terminate_sync() instead.
|
|
+ */
|
|
+static inline int dmaengine_terminate_async(struct dma_chan *chan)
|
|
+{
|
|
+ if (chan->device->device_terminate_all)
|
|
+ return chan->device->device_terminate_all(chan);
|
|
+
|
|
+ return -EINVAL;
|
|
+}
|
|
+
|
|
+/**
|
|
+ * dmaengine_synchronize() - Synchronize DMA channel termination
|
|
+ * @chan: The channel to synchronize
|
|
+ *
|
|
+ * Synchronizes to the DMA channel termination to the current context. When this
|
|
+ * function returns it is guaranteed that all transfers for previously issued
|
|
+ * descriptors have stopped and and it is safe to free the memory assoicated
|
|
+ * with them. Furthermore it is guaranteed that all complete callback functions
|
|
+ * for a previously submitted descriptor have finished running and it is safe to
|
|
+ * free resources accessed from within the complete callbacks.
|
|
+ *
|
|
+ * The behavior of this function is undefined if dma_async_issue_pending() has
|
|
+ * been called between dmaengine_terminate_async() and this function.
|
|
+ *
|
|
+ * This function must only be called from non-atomic context and must not be
|
|
+ * called from within a complete callback of a descriptor submitted on the same
|
|
+ * channel.
|
|
+ */
|
|
+static inline void dmaengine_synchronize(struct dma_chan *chan)
|
|
+{
|
|
+ if (chan->device->device_synchronize)
|
|
+ chan->device->device_synchronize(chan);
|
|
+}
|
|
+
|
|
+/**
|
|
+ * dmaengine_terminate_sync() - Terminate all active DMA transfers
|
|
+ * @chan: The channel for which to terminate the transfers
|
|
+ *
|
|
+ * Calling this function will terminate all active and pending transfers
|
|
+ * that have previously been submitted to the channel. It is similar to
|
|
+ * dmaengine_terminate_async() but guarantees that the DMA transfer has actually
|
|
+ * stopped and that all complete callbacks have finished running when the
|
|
+ * function returns.
|
|
+ *
|
|
+ * This function must only be called from non-atomic context and must not be
|
|
+ * called from within a complete callback of a descriptor submitted on the same
|
|
+ * channel.
|
|
+ */
|
|
+static inline int dmaengine_terminate_sync(struct dma_chan *chan)
|
|
+{
|
|
+ int ret;
|
|
+
|
|
+ ret = dmaengine_terminate_async(chan);
|
|
+ if (ret)
|
|
+ return ret;
|
|
+
|
|
+ dmaengine_synchronize(chan);
|
|
+
|
|
+ return 0;
|
|
+}
|
|
+
|
|
static inline int dmaengine_pause(struct dma_chan *chan)
|
|
{
|
|
if (chan->device->device_pause)
|
|
|