123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282 |
- 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,
- @@ -874,6 +877,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)
- @@ -882,6 +892,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)
|