* [PATCH 08/18] DMAENGINE: ste_dma40: lock fix
@ 2010-06-01 12:21 Linus Walleij
2010-06-11 22:38 ` Dan Williams
0 siblings, 1 reply; 4+ messages in thread
From: Linus Walleij @ 2010-06-01 12:21 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-kernel, Jonas Aaberg, Linus Walleij
From: Jonas Aaberg <jonas.aberg@stericsson.com>
Fix up some locking issues found by enabling lock debugging.
Signed-off-by: Jonas Aaberg <jonas.aberg@stericsson.com>
Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
drivers/dma/ste_dma40.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index aba86bb..c4b0d37 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -441,6 +441,7 @@ static int d40_lcla_id_get(struct d40_chan *d40c,
pool->base + d40c->phy_chan->num * 1024;
int i;
int lli_per_log = d40c->base->plat_data->llis_per_log;
+ unsigned long flags;
if (d40c->lcla.src_id >= 0 && d40c->lcla.dst_id >= 0)
return 0;
@@ -448,7 +449,7 @@ static int d40_lcla_id_get(struct d40_chan *d40c,
if (pool->num_blocks > 32)
return -EINVAL;
- spin_lock(&pool->lock);
+ spin_lock_irqsave(&pool->lock, flags);
for (i = 0; i < pool->num_blocks; i++) {
if (!(pool->alloc_map[d40c->phy_chan->num] & (0x1 << i))) {
@@ -477,10 +478,10 @@ static int d40_lcla_id_get(struct d40_chan *d40c,
d40c->lcla.src = lcla_lidx_base + src_id * lli_per_log + 1;
- spin_unlock(&pool->lock);
+ spin_unlock_irqrestore(&pool->lock, flags);
return 0;
err:
- spin_unlock(&pool->lock);
+ spin_unlock_irqrestore(&pool->lock, flags);
return -EINVAL;
}
@@ -488,15 +489,16 @@ static void d40_lcla_id_put(struct d40_chan *d40c,
struct d40_lcla_pool *pool,
int id)
{
+ unsigned long flags;
if (id < 0)
return;
d40c->lcla.src_id = -1;
d40c->lcla.dst_id = -1;
- spin_lock(&pool->lock);
+ spin_lock_irqsave(&pool->lock, flags);
pool->alloc_map[d40c->phy_chan->num] &= (~(0x1 << id));
- spin_unlock(&pool->lock);
+ spin_unlock_irqrestore(&pool->lock, flags);
}
static int d40_channel_execute_command(struct d40_chan *d40c,
@@ -1990,8 +1992,6 @@ static struct dma_async_tx_descriptor *d40_prep_slave_sg(struct dma_chan *chan,
if (d40d == NULL)
return NULL;
- memset(d40d, 0, sizeof(struct d40_desc));
-
if (d40c->log_num != D40_PHY_CHAN)
err = d40_prep_slave_sg_log(d40d, d40c, sgl, sg_len,
direction, dma_flags);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 08/18] DMAENGINE: ste_dma40: lock fix
2010-06-01 12:21 [PATCH 08/18] DMAENGINE: ste_dma40: lock fix Linus Walleij
@ 2010-06-11 22:38 ` Dan Williams
2010-06-14 16:41 ` Linus WALLEIJ
0 siblings, 1 reply; 4+ messages in thread
From: Dan Williams @ 2010-06-11 22:38 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-kernel, Jonas Aaberg
On Tue, Jun 1, 2010 at 5:21 AM, Linus Walleij
<linus.walleij@stericsson.com> wrote:
> From: Jonas Aaberg <jonas.aberg@stericsson.com>
>
> Fix up some locking issues found by enabling lock debugging.
>
> Signed-off-by: Jonas Aaberg <jonas.aberg@stericsson.com>
> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
> drivers/dma/ste_dma40.c | 14 +++++++-------
> 1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index aba86bb..c4b0d37 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
[..]
> static int d40_channel_execute_command(struct d40_chan *d40c,
> @@ -1990,8 +1992,6 @@ static struct dma_async_tx_descriptor *d40_prep_slave_sg(struct dma_chan *chan,
> if (d40d == NULL)
> return NULL;
>
> - memset(d40d, 0, sizeof(struct d40_desc));
> -
This one isn't a locking bug fix.
--
Dan
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH 08/18] DMAENGINE: ste_dma40: lock fix
2010-06-11 22:38 ` Dan Williams
@ 2010-06-14 16:41 ` Linus WALLEIJ
2010-06-15 4:53 ` Dan Williams
0 siblings, 1 reply; 4+ messages in thread
From: Linus WALLEIJ @ 2010-06-14 16:41 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-kernel, Jonas ABERG
On 06/12/2010 12:38 AM, Dan Williams wrote:
> On Tue, Jun 1, 2010 at 5:21 AM, Linus Walleij
> <linus.walleij@stericsson.com> wrote:
>> From: Jonas Aaberg <jonas.aberg@stericsson.com>
>>
>> Fix up some locking issues found by enabling lock debugging.
>>
>> Signed-off-by: Jonas Aaberg <jonas.aberg@stericsson.com>
>> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
>> ---
>> drivers/dma/ste_dma40.c | 14 +++++++-------
>> 1 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c index
>> aba86bb..c4b0d37 100644
>> --- a/drivers/dma/ste_dma40.c
>> +++ b/drivers/dma/ste_dma40.c
> [..]
>> static int d40_channel_execute_command(struct d40_chan *d40c, @@
>> -1990,8 +1992,6 @@ static struct dma_async_tx_descriptor *d40_prep_slave_sg(struct dma_chan *chan,
>> if (d40d == NULL)
>> return NULL;
>>
>> - memset(d40d, 0, sizeof(struct d40_desc));
>> -
>
> This one isn't a locking bug fix.
Well, actually it was overwriting a spinlock, so I would considering
a locking bug area atleast.
But if you prefer we split this one-liner into a separate patch.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 08/18] DMAENGINE: ste_dma40: lock fix
2010-06-14 16:41 ` Linus WALLEIJ
@ 2010-06-15 4:53 ` Dan Williams
0 siblings, 0 replies; 4+ messages in thread
From: Dan Williams @ 2010-06-15 4:53 UTC (permalink / raw)
To: Linus WALLEIJ; +Cc: linux-kernel, Jonas ABERG
On Mon, Jun 14, 2010 at 9:41 AM, Linus WALLEIJ
<linus.walleij@stericsson.com> wrote:
> On 06/12/2010 12:38 AM, Dan Williams wrote:
>
>> On Tue, Jun 1, 2010 at 5:21 AM, Linus Walleij
>> <linus.walleij@stericsson.com> wrote:
>>> From: Jonas Aaberg <jonas.aberg@stericsson.com>
>>>
>>> Fix up some locking issues found by enabling lock debugging.
>>>
>>> Signed-off-by: Jonas Aaberg <jonas.aberg@stericsson.com>
>>> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
>>> ---
>>> drivers/dma/ste_dma40.c | 14 +++++++-------
>>> 1 files changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c index
>>> aba86bb..c4b0d37 100644
>>> --- a/drivers/dma/ste_dma40.c
>>> +++ b/drivers/dma/ste_dma40.c
>> [..]
>>> static int d40_channel_execute_command(struct d40_chan *d40c, @@
>>> -1990,8 +1992,6 @@ static struct dma_async_tx_descriptor *d40_prep_slave_sg(struct dma_chan *chan,
>>> if (d40d == NULL)
>>> return NULL;
>>>
>>> - memset(d40d, 0, sizeof(struct d40_desc));
>>> -
>>
>> This one isn't a locking bug fix.
>
> Well, actually it was overwriting a spinlock, so I would considering
> a locking bug area atleast.
Ah, I missed that you call dma_async_tx_descriptor_init() every prep.
It may be worthwhile to select ASYNC_TX_DISABLE_CHANNEL_SWITCH to get
rid of the unused fields for async_tx channel switching *.
> But if you prefer we split this one-liner into a separate patch.
You never take the embedded lock so there was not a problem, but you
can leave it in as an innocuous cleanup.
Thanks,
Dan
* I'm now thinking that channel switching should be an opt-in request
rather than opt-out, because I can foresee a driver relying on the
size reductions it affords. With the logic flipped we can disable the
driver that has this dependence.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-06-15 4:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-01 12:21 [PATCH 08/18] DMAENGINE: ste_dma40: lock fix Linus Walleij
2010-06-11 22:38 ` Dan Williams
2010-06-14 16:41 ` Linus WALLEIJ
2010-06-15 4:53 ` Dan Williams
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.