Re: [PATCH v2] ARM: pl330: Fix a race condition

[Javi Merino <javi.merino@arm.com>]

On 11/12/11 17:10, Jassi Brar wrote:
> On 11 December 2011 20:39, Javi Merino <javi.merino@arm.com> wrote:
>>>>
>>>> What about properly tracking what we have sent to the DMA?  Something
>>>> like the following (warning *ugly* and untested code ahead, may eat your
>>>> kitten):
>>>>
>>> Yeah, this is like I said 'marker' method. Though we can clean it up a bit.
>>> 1) Pack req_running and lstenq together. Make lsteng return invalid value
>>> should there be no buff programmed, otherwise 0 or 1.
>>
>> This can lead to starvation.  If lstenq is -1 when the DMA hasn't been
>> programmed yet, in _trigger() you don't know which buffer is the
>> "oldest", so you may end up always starting the new buffer and
>> forgetting about the other one.  lstenq as it is right now prevents that.
>>
> Sorry I don't understand. If lstenq is -1 that means there's no req programmed
> so trigger need not do anything. I didn't say we don't need to do anything else.

Currently lstenq tracks the last request that pl330_submit_req() has
enqueued.  I think we should add a req_running (or any other name) that
tracks what _trigger() has sent to the DMA.  If we pack them together we
lose some information and I don't see a way of packing them safely.

> Though it's just an idea I haven't implemented and it may not work out.

I was trying to implement it and got stuck in _trigger() thinking "ok,
so which buffer am I supposed to trigger now..."

> Just please give it a try if you can. Thanks.

I have a patch that seems to be working.  Let me test it a little bit
more and I'll send it to the list.

Cheers,
Javi