2017-11-14 14:35:39

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 00/10] dmaengine: fix race with vchan_complete

Hi,

With the introduction of .device_synchronize callback it was thought that the
race caused crash observed in vchan_complete is fixed, but unfortunately it can
still happen.

The observed scenario (really hard to reproduce) is:
Cyclic mode
- DMA period interrupt
- call to vchan_cyclic_callback() which sets vc->cyclic to vd and schedules the
vchan_complete tasklet
- .terminate_all is called
- we make sure that no further DMA irqs are going to be handled, but the
tasklet had been already scheduled
- we free up the descriptor to avoid leaking memory
- the vchan_complete tasklet starts to execute and it checks vc->cyclic, which
is not NULL, saves the vd pointer (which points to an already freed up memory)
and try to access it later to call the callback
- the tasklet_kill() in .device_synchronize will make sure that the tasklet is
going to finish it's execution if it is already scheduled, it can only help if
the tasklet is yet to be executed.

At this point it is just matter of luck if the vc->cyclic is still pointing to
an unchanged memory location or it is taken into use and thus it is corrupted.

My first approach was to just set vc->cyclic to NULL in the .terminate_all
callback, but that still have theoritical race: if the vchan_complete is
executing and it saves the vd from vc->cyclic (protected by the vc->lock). If at
that point the .terminate_all is called it will wait for the lock and start
executing. _if_ the .terminate_all free up the vd before the vchan_complete
reaches the point when it is going to call the callback, then we have the race.

The series will do this:
- the drivers should call vchan_terminate_vdesc() instead of directly freeing up
the descriptor. vchan_terminate_vdesc() will save the vd as vc->vd_terminated
and will set the vc->cyclic to NULL, all while holding the lock.
- the drivers must implement the .device_synchronize callback and within the
vchan_synchronize() we free up the vc->vd_terminated after we killed the
tasklet.

I have tested this on platforms using TI's eDMA and sDMA and have not seen any
side effect so far and a client tested similar set on a setup where it was easy
to reproduce the race.

By looking for similar patterns in other drivers I have implemented the fix for
the ones where it looked straight forward.

Regards,
Peter
---
Peter Ujfalusi (10):
dmaengine: virt-dma: Add helper to free/reuse a descriptor
dmaengine: virt-dma: Support for race free transfer termination
dmaengine: omap-dma: Use vchan_terminate_vdesc() instead of desc_free
dmaengine: edma: Use vchan_terminate_vdesc() instead of desc_free
dmaengine: bcm2835-dma: Use vchan_terminate_vdesc() instead of
desc_free
dmaengine: dma-jz4780: Use vchan_terminate_vdesc() instead of
desc_free
dmaengine: amba-pl08x: Use vchan_terminate_vdesc() instead of
desc_free
dmaengine: img-mdc-dma: Use vchan_terminate_vdesc() instead of
desc_free
dmaengine: k3dma: Use vchan_terminate_vdesc() instead of desc_free
dmaengine: s3c24xx-dma: Use vchan_terminate_vdesc() instead of
desc_free

drivers/dma/amba-pl08x.c | 11 ++++++++++-
drivers/dma/bcm2835-dma.c | 10 +++++++++-
drivers/dma/dma-jz4780.c | 10 +++++++++-
drivers/dma/edma.c | 7 ++-----
drivers/dma/img-mdc-dma.c | 17 ++++++++++++-----
drivers/dma/k3dma.c | 10 +++++++++-
drivers/dma/omap-dma.c | 2 +-
drivers/dma/s3c24xx-dma.c | 11 ++++++++++-
drivers/dma/virt-dma.c | 5 +----
drivers/dma/virt-dma.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
10 files changed, 107 insertions(+), 20 deletions(-)

--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1585866334898387852@xxx Mon Dec 04 15:06:32 +0000 2017
X-GM-THRID: 1584965695969556311
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread


2017-11-14 14:37:26

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 05/10] dmaengine: bcm2835-dma: Use vchan_terminate_vdesc() instead of desc_free

To avoid race with vchan_complete, use the race free way to terminate
running transfer.

Implement the device_synchronize callback to make sure that the terminated
descriptor is freed.

CC: Martin Sperl <[email protected]>
CC: Eric Anholt <[email protected]>
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/bcm2835-dma.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c
index 6204cc32d09c..847f84a41a69 100644
--- a/drivers/dma/bcm2835-dma.c
+++ b/drivers/dma/bcm2835-dma.c
@@ -812,7 +812,7 @@ static int bcm2835_dma_terminate_all(struct dma_chan *chan)
* c->desc is NULL and exit.)
*/
if (c->desc) {
- bcm2835_dma_desc_free(&c->desc->vd);
+ vchan_terminate_vdesc(&c->desc->vd);
c->desc = NULL;
bcm2835_dma_abort(c->chan_base);

@@ -836,6 +836,13 @@ static int bcm2835_dma_terminate_all(struct dma_chan *chan)
return 0;
}

+static void bcm2835_dma_synchronize(struct dma_chan *chan)
+{
+ struct bcm2835_chan *c = to_bcm2835_dma_chan(chan);
+
+ vchan_synchronize(&c->vc);
+}
+
static int bcm2835_dma_chan_init(struct bcm2835_dmadev *d, int chan_id,
int irq, unsigned int irq_flags)
{
@@ -942,6 +949,7 @@ static int bcm2835_dma_probe(struct platform_device *pdev)
od->ddev.device_prep_dma_memcpy = bcm2835_dma_prep_dma_memcpy;
od->ddev.device_config = bcm2835_dma_slave_config;
od->ddev.device_terminate_all = bcm2835_dma_terminate_all;
+ od->ddev.device_synchronize = bcm2835_dma_synchronize;
od->ddev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
od->ddev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
od->ddev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV) |
--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1584667187112064591@xxx Tue Nov 21 09:26:36 +0000 2017
X-GM-THRID: 1584667187112064591
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-14 14:38:15

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 01/10] dmaengine: virt-dma: Add helper to free/reuse a descriptor

The vchan_vdesc_fini() can be used to free or reuse a given descriptor
after it has been marked as completed.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/virt-dma.c | 5 +----
drivers/dma/virt-dma.h | 14 ++++++++++++++
2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/virt-dma.c b/drivers/dma/virt-dma.c
index 545e97279083..88ad8ed2a8d6 100644
--- a/drivers/dma/virt-dma.c
+++ b/drivers/dma/virt-dma.c
@@ -107,10 +107,7 @@ static void vchan_complete(unsigned long arg)
dmaengine_desc_get_callback(&vd->tx, &cb);

list_del(&vd->node);
- if (dmaengine_desc_test_reuse(&vd->tx))
- list_add(&vd->node, &vc->desc_allocated);
- else
- vc->desc_free(vd);
+ vchan_vdesc_fini(vd);

dmaengine_desc_callback_invoke(&cb, NULL);
}
diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
index 3f776a46a29c..2edb05505102 100644
--- a/drivers/dma/virt-dma.h
+++ b/drivers/dma/virt-dma.h
@@ -103,6 +103,20 @@ static inline void vchan_cookie_complete(struct virt_dma_desc *vd)
tasklet_schedule(&vc->task);
}

+/**
+ * vchan_vdesc_fini - Free or reuse a descriptor
+ * @vd: virtual descriptor to free/reuse
+ */
+static inline void vchan_vdesc_fini(struct virt_dma_desc *vd)
+{
+ struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
+
+ if (dmaengine_desc_test_reuse(&vd->tx))
+ list_add(&vd->node, &vc->desc_allocated);
+ else
+ vc->desc_free(vd);
+}
+
/**
* vchan_cyclic_callback - report the completion of a period
* @vd: virtual descriptor
--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1586884385115638014@xxx Fri Dec 15 20:48:01 +0000 2017
X-GM-THRID: 1584146042639869552
X-Gmail-Labels: Inbox,Category Forums

2017-11-14 15:43:26

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 09/10] dmaengine: k3dma: Use vchan_terminate_vdesc() instead of desc_free

To avoid race with vchan_complete, use the race free way to terminate
running transfer.

Implement the device_synchronize callback to make sure that the terminated
descriptor is freed.

CC: Zhangfei Gao <[email protected]>
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/k3dma.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index 01d2a750a621..26b67455208f 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -719,7 +719,7 @@ static int k3_dma_terminate_all(struct dma_chan *chan)
c->phy = NULL;
p->vchan = NULL;
if (p->ds_run) {
- k3_dma_free_desc(&p->ds_run->vd);
+ vchan_terminate_vdesc(&p->ds_run->vd);
p->ds_run = NULL;
}
p->ds_done = NULL;
@@ -730,6 +730,13 @@ static int k3_dma_terminate_all(struct dma_chan *chan)
return 0;
}

+static void k3_dma_synchronize(struct dma_chan *chan)
+{
+ struct k3_dma_chan *c = to_k3_chan(chan);
+
+ vchan_synchronize(&c->vc);
+}
+
static int k3_dma_transfer_pause(struct dma_chan *chan)
{
struct k3_dma_chan *c = to_k3_chan(chan);
@@ -868,6 +875,7 @@ static int k3_dma_probe(struct platform_device *op)
d->slave.device_pause = k3_dma_transfer_pause;
d->slave.device_resume = k3_dma_transfer_resume;
d->slave.device_terminate_all = k3_dma_terminate_all;
+ d->slave.device_synchronize = k3_dma_synchronize;
d->slave.copy_align = DMAENGINE_ALIGN_8_BYTES;

/* init virtual channel */
--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1595632912953396613@xxx Thu Mar 22 10:22:07 +0000 2018
X-GM-THRID: 1584123135258654096
X-Gmail-Labels: Inbox,Category Forums,Downloaded_2018-03

2017-11-14 14:39:08

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 02/10] dmaengine: virt-dma: Support for race free transfer termination

Even with the introduced vchan_synchronize() we can face race when
terminating a cyclic transfer.

If the terminate_all is called after the interrupt handler called
vchan_cyclic_callback(), but before the vchan_complete tasklet is called:
vc->cyclic is set to the cyclic descriptor, but the descriptor itself was
freed up in the driver's terminate_all() callback.
When the vhan_complete() is executed it will try to fetch the vc->cyclic
vdesc, but the pointer is pointing now to uninitialized memory leading to
(hard to reproduce) kernel crash.

In order to fix this, drivers should:
- call vchan_terminate_vdesc() from their terminate_all callback instead
calling their free_desc function to free up the descriptor.
- implement device_synchronize callback and call vchan_synchronize().

This way we can make sure that the descriptor is only going to be freed up
after the vchan_callback was executed in a safe manner.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/virt-dma.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)

diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
index 2edb05505102..b09b75ab0751 100644
--- a/drivers/dma/virt-dma.h
+++ b/drivers/dma/virt-dma.h
@@ -35,6 +35,7 @@ struct virt_dma_chan {
struct list_head desc_completed;

struct virt_dma_desc *cyclic;
+ struct virt_dma_desc *vd_terminated;
};

static inline struct virt_dma_chan *to_virt_chan(struct dma_chan *chan)
@@ -129,6 +130,25 @@ static inline void vchan_cyclic_callback(struct virt_dma_desc *vd)
tasklet_schedule(&vc->task);
}

+/**
+ * vchan_terminate_vdesc - Disable pending cyclic callback
+ * @vd: virtual descriptor to be terminated
+ *
+ * vc.lock must be held by caller
+ */
+static inline void vchan_terminate_vdesc(struct virt_dma_desc *vd)
+{
+ struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
+
+ /* free up stuck descriptor */
+ if (vc->vd_terminated)
+ vchan_vdesc_fini(vc->vd_terminated);
+
+ vc->vd_terminated = vd;
+ if (vc->cyclic == vd)
+ vc->cyclic = NULL;
+}
+
/**
* vchan_next_desc - peek at the next descriptor to be processed
* @vc: virtual channel to obtain descriptor from
@@ -182,10 +202,20 @@ static inline void vchan_free_chan_resources(struct virt_dma_chan *vc)
* Makes sure that all scheduled or active callbacks have finished running. For
* proper operation the caller has to ensure that no new callbacks are scheduled
* after the invocation of this function started.
+ * Free up the terminated cyclic descriptor to prevent memory leakage.
*/
static inline void vchan_synchronize(struct virt_dma_chan *vc)
{
+ unsigned long flags;
+
tasklet_kill(&vc->task);
+
+ spin_lock_irqsave(&vc->lock, flags);
+ if (vc->vd_terminated) {
+ vchan_vdesc_fini(vc->vd_terminated);
+ vc->vd_terminated = NULL;
+ }
+ spin_unlock_irqrestore(&vc->lock, flags);
}

#endif
--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1584507071688026620@xxx Sun Nov 19 15:01:38 +0000 2017
X-GM-THRID: 1584506413281899864
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-14 15:42:18

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 08/10] dmaengine: img-mdc-dma: Use vchan_terminate_vdesc() instead of desc_free

To avoid race with vchan_complete, use the race free way to terminate
running transfer.

Implement the device_synchronize callback to make sure that the terminated
descriptor is freed.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/img-mdc-dma.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/img-mdc-dma.c b/drivers/dma/img-mdc-dma.c
index 0391f930aecc..25cec9c243e1 100644
--- a/drivers/dma/img-mdc-dma.c
+++ b/drivers/dma/img-mdc-dma.c
@@ -694,7 +694,6 @@ static unsigned int mdc_get_new_events(struct mdc_chan *mchan)
static int mdc_terminate_all(struct dma_chan *chan)
{
struct mdc_chan *mchan = to_mdc_chan(chan);
- struct mdc_tx_desc *mdesc;
unsigned long flags;
LIST_HEAD(head);

@@ -703,21 +702,28 @@ static int mdc_terminate_all(struct dma_chan *chan)
mdc_chan_writel(mchan, MDC_CONTROL_AND_STATUS_CANCEL,
MDC_CONTROL_AND_STATUS);

- mdesc = mchan->desc;
- mchan->desc = NULL;
+ if (mchan->desc) {
+ vchan_terminate_vdesc(&mchan->desc->vd);
+ mchan->desc = NULL;
+ }
vchan_get_all_descriptors(&mchan->vc, &head);

mdc_get_new_events(mchan);

spin_unlock_irqrestore(&mchan->vc.lock, flags);

- if (mdesc)
- mdc_desc_free(&mdesc->vd);
vchan_dma_desc_free_list(&mchan->vc, &head);

return 0;
}

+static void mdc_synchronize(struct dma_chan *chan)
+{
+ struct mdc_chan *mchan = to_mdc_chan(chan);
+
+ vchan_synchronize(&mchan->vc);
+}
+
static int mdc_slave_config(struct dma_chan *chan,
struct dma_slave_config *config)
{
@@ -952,6 +958,7 @@ static int mdc_dma_probe(struct platform_device *pdev)
mdma->dma_dev.device_tx_status = mdc_tx_status;
mdma->dma_dev.device_issue_pending = mdc_issue_pending;
mdma->dma_dev.device_terminate_all = mdc_terminate_all;
+ mdma->dma_dev.device_synchronize = mdc_synchronize;
mdma->dma_dev.device_config = mdc_slave_config;

mdma->dma_dev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1584093007622661800@xxx Wed Nov 15 01:20:16 +0000 2017
X-GM-THRID: 1584079370451099651
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-14 15:48:27

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 06/10] dmaengine: dma-jz4780: Use vchan_terminate_vdesc() instead of desc_free

To avoid race with vchan_complete, use the race free way to terminate
running transfer.

Implement the device_synchronize callback to make sure that the terminated
descriptor is freed.

CC: Zubair Lutfullah Kakakhel <[email protected]>
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/dma-jz4780.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 7373b7a555ec..85820a2d69d4 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -511,7 +511,7 @@ static int jz4780_dma_terminate_all(struct dma_chan *chan)
/* Clear the DMA status and stop the transfer. */
jz4780_dma_writel(jzdma, JZ_DMA_REG_DCS(jzchan->id), 0);
if (jzchan->desc) {
- jz4780_dma_desc_free(&jzchan->desc->vdesc);
+ vchan_terminate_vdesc(&jzchan->desc->vdesc);
jzchan->desc = NULL;
}

@@ -523,6 +523,13 @@ static int jz4780_dma_terminate_all(struct dma_chan *chan)
return 0;
}

+static void jz4780_dma_synchronize(struct dma_chan *chan)
+{
+ struct jz4780_dma_chan *jzchan = to_jz4780_dma_chan(chan);
+
+ vchan_synchronize(&jzchan->vchan);
+}
+
static int jz4780_dma_config(struct dma_chan *chan,
struct dma_slave_config *config)
{
@@ -813,6 +820,7 @@ static int jz4780_dma_probe(struct platform_device *pdev)
dd->device_prep_dma_memcpy = jz4780_dma_prep_dma_memcpy;
dd->device_config = jz4780_dma_config;
dd->device_terminate_all = jz4780_dma_terminate_all;
+ dd->device_synchronize = jz4780_dma_synchronize;
dd->device_tx_status = jz4780_dma_tx_status;
dd->device_issue_pending = jz4780_dma_issue_pending;
dd->src_addr_widths = JZ_DMA_BUSWIDTHS;
--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1584093043603910855@xxx Wed Nov 15 01:20:50 +0000 2017
X-GM-THRID: 1580943426679342147
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-14 14:36:11

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 03/10] dmaengine: omap-dma: Use vchan_terminate_vdesc() instead of desc_free

To avoid race with vchan_complete, use the race free way to terminate
running transfer.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/omap-dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index f6dd849159d8..d21c19822feb 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -1311,7 +1311,7 @@ static int omap_dma_terminate_all(struct dma_chan *chan)
* c->desc is NULL and exit.)
*/
if (c->desc) {
- omap_dma_desc_free(&c->desc->vd);
+ vchan_terminate_vdesc(&c->desc->vd);
c->desc = NULL;
/* Avoid stopping the dma twice */
if (!c->paused)
--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1583984666274585715@xxx Mon Nov 13 20:38:13 +0000 2017
X-GM-THRID: 1583984666274585715
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-14 14:35:48

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 04/10] dmaengine: edma: Use vchan_terminate_vdesc() instead of desc_free

To avoid race with vchan_complete, use the race free way to terminate
running transfer.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/edma.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 9364a3ed345a..948df1ab5f1a 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -860,11 +860,8 @@ static int edma_terminate_all(struct dma_chan *chan)
/* Move the cyclic channel back to default queue */
if (!echan->tc && echan->edesc->cyclic)
edma_assign_channel_eventq(echan, EVENTQ_DEFAULT);
- /*
- * free the running request descriptor
- * since it is not in any of the vdesc lists
- */
- edma_desc_free(&echan->edesc->vdesc);
+
+ vchan_terminate_vdesc(&echan->edesc->vdesc);
echan->edesc = NULL;
}

--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1583944234139766929@xxx Mon Nov 13 09:55:34 +0000 2017
X-GM-THRID: 1583941184996232231
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-14 14:37:45

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 10/10] dmaengine: s3c24xx-dma: Use vchan_terminate_vdesc() instead of desc_free

To avoid race with vchan_complete, use the race free way to terminate
running transfer.

Implement the device_synchronize callback to make sure that the terminated
descriptor is freed.

CC: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/s3c24xx-dma.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/s3c24xx-dma.c b/drivers/dma/s3c24xx-dma.c
index f04c4702d98b..cd92d696bcf9 100644
--- a/drivers/dma/s3c24xx-dma.c
+++ b/drivers/dma/s3c24xx-dma.c
@@ -732,7 +732,7 @@ static int s3c24xx_dma_terminate_all(struct dma_chan *chan)

/* Dequeue current job */
if (s3cchan->at) {
- s3c24xx_dma_desc_free(&s3cchan->at->vd);
+ vchan_terminate_vdesc(&s3cchan->at->vd);
s3cchan->at = NULL;
}

@@ -744,6 +744,13 @@ static int s3c24xx_dma_terminate_all(struct dma_chan *chan)
return ret;
}

+static void s3c24xx_dma_synchronize(struct dma_chan *chan)
+{
+ struct s3c24xx_dma_chan *s3cchan = to_s3c24xx_dma_chan(chan);
+
+ vchan_synchronize(&s3cchan->vc);
+}
+
static void s3c24xx_dma_free_chan_resources(struct dma_chan *chan)
{
/* Ensure all queued descriptors are freed */
@@ -1282,6 +1289,7 @@ static int s3c24xx_dma_probe(struct platform_device *pdev)
s3cdma->memcpy.device_issue_pending = s3c24xx_dma_issue_pending;
s3cdma->memcpy.device_config = s3c24xx_dma_set_runtime_config;
s3cdma->memcpy.device_terminate_all = s3c24xx_dma_terminate_all;
+ s3cdma->memcpy.device_synchronize = s3c24xx_dma_synchronize;

/* Initialize slave engine for SoC internal dedicated peripherals */
dma_cap_set(DMA_SLAVE, s3cdma->slave.cap_mask);
@@ -1296,6 +1304,7 @@ static int s3c24xx_dma_probe(struct platform_device *pdev)
s3cdma->slave.device_prep_dma_cyclic = s3c24xx_dma_prep_dma_cyclic;
s3cdma->slave.device_config = s3c24xx_dma_set_runtime_config;
s3cdma->slave.device_terminate_all = s3c24xx_dma_terminate_all;
+ s3cdma->slave.device_synchronize = s3c24xx_dma_synchronize;
s3cdma->slave.filter.map = pdata->slave_map;
s3cdma->slave.filter.mapcnt = pdata->slavecnt;
s3cdma->slave.filter.fn = s3c24xx_dma_filter;
--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1583802166625277288@xxx Sat Nov 11 20:17:28 +0000 2017
X-GM-THRID: 1583730621455347206
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-14 14:36:28

by Peter Ujfalusi

[permalink] [raw]
Subject: [PATCH 07/10] dmaengine: amba-pl08x: Use vchan_terminate_vdesc() instead of desc_free

To avoid race with vchan_complete, use the race free way to terminate
running transfer.

Implement the device_synchronize callback to make sure that the terminated
descriptor is freed.

CC: Linus Walleij <[email protected]>
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/amba-pl08x.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index b52b0d55247e..97483df1f82e 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -2182,7 +2182,7 @@ static int pl08x_terminate_all(struct dma_chan *chan)
}
/* Dequeue jobs and free LLIs */
if (plchan->at) {
- pl08x_desc_free(&plchan->at->vd);
+ vchan_terminate_vdesc(&plchan->at->vd);
plchan->at = NULL;
}
/* Dequeue jobs not yet fired as well */
@@ -2193,6 +2193,13 @@ static int pl08x_terminate_all(struct dma_chan *chan)
return 0;
}

+static void pl08x_synchronize(struct dma_chan *chan)
+{
+ struct pl08x_dma_chan *plchan = to_pl08x_chan(chan);
+
+ vchan_synchronize(&plchan->vc);
+}
+
static int pl08x_pause(struct dma_chan *chan)
{
struct pl08x_dma_chan *plchan = to_pl08x_chan(chan);
@@ -2773,6 +2780,7 @@ static int pl08x_probe(struct amba_device *adev, const struct amba_id *id)
pl08x->memcpy.device_pause = pl08x_pause;
pl08x->memcpy.device_resume = pl08x_resume;
pl08x->memcpy.device_terminate_all = pl08x_terminate_all;
+ pl08x->memcpy.device_synchronize = pl08x_synchronize;
pl08x->memcpy.src_addr_widths = PL80X_DMA_BUSWIDTHS;
pl08x->memcpy.dst_addr_widths = PL80X_DMA_BUSWIDTHS;
pl08x->memcpy.directions = BIT(DMA_MEM_TO_MEM);
@@ -2802,6 +2810,7 @@ static int pl08x_probe(struct amba_device *adev, const struct amba_id *id)
pl08x->slave.device_pause = pl08x_pause;
pl08x->slave.device_resume = pl08x_resume;
pl08x->slave.device_terminate_all = pl08x_terminate_all;
+ pl08x->slave.device_synchronize = pl08x_synchronize;
pl08x->slave.src_addr_widths = PL80X_DMA_BUSWIDTHS;
pl08x->slave.dst_addr_widths = PL80X_DMA_BUSWIDTHS;
pl08x->slave.directions =
--
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


From 1583859582482649616@xxx Sun Nov 12 11:30:04 +0000 2017
X-GM-THRID: 1583854673980966490
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread