[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] dma: Avoid reentrancy in DMA transfer handlers
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH] dma: Avoid reentrancy in DMA transfer handlers |
Date: |
Mon, 31 Oct 2011 15:46:01 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20110927 Thunderbird/7.0 |
Am 28.10.2011 18:35, schrieb Paolo Bonzini:
> On 10/28/2011 04:18 PM, Kevin Wolf wrote:
>> With the conversion of the block layer to coroutines, bdrv_read/write
>> have changed to run a nested event loop that calls qemu_bh_poll.
>> Consequently a scheduled BH can be called while a DMA transfer handler
>> runs and this means that DMA_run becomes reentrant.
>>
>> Devices haven't been designed to cope with that, so instead of running a
>> nested transfer handler just wait for the next invocation of the BH from the
>> main loop.
>>
>> This fixes some problems with the floppy device.
>>
>> Signed-off-by: Kevin Wolf<address@hidden>
>> ---
>> hw/dma.c | 9 +++++++++
>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/dma.c b/hw/dma.c
>> index 8a7302a..e8d6341 100644
>> --- a/hw/dma.c
>> +++ b/hw/dma.c
>> @@ -358,6 +358,13 @@ static void DMA_run (void)
>> struct dma_cont *d;
>> int icont, ichan;
>> int rearm = 0;
>> + static int running = 0;
>> +
>> + if (running) {
>> + goto out;
>> + } else {
>> + running = 1;
>> + }
>>
>> d = dma_controllers;
>>
>> @@ -374,6 +381,8 @@ static void DMA_run (void)
>> }
>> }
>>
>> +out:
>> + running = 0;
>> if (rearm)
>> qemu_bh_schedule_idle(dma_bh);
>> }
>
> Hmm, I think you should set rearm = 1 to ensure the BH is run when
> ultimately you leave the sync read. Sorry for not spotting this before.
I was about to agree, but in fact adding a rearm = 1; line leads to
crashes, whereas in the version I posted it just works. So it looks like
something is wrong with doing it, even though it seemed to make perfect
sense at the first sight.
Kevin