[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [RFC PATCH 0/5] atapi: change unlimited re

From: Paolo Bonzini
Subject: Re: [Qemu-block] [Qemu-devel] [RFC PATCH 0/5] atapi: change unlimited recursion to while loop
Date: Fri, 23 Mar 2018 21:17:44 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

On 23/03/2018 21:08, John Snow wrote:
> On 02/23/2018 10:26 AM, Paolo Bonzini wrote:
>> Real hardware doesn't have an unlimited stack, so the unlimited
>> recursion in the ATAPI code smells a bit.  In fact, the call to
>> ide_transfer_start easily becomes a tail call with a small change
>> to the code (patch 4).  The remaining four patches move code around
>> so as to the turn the call back to ide_atapi_cmd_reply_end into
>> another tail call, and then convert the (double) tail recursion into
>> a while loop.
>> I'm not sure how this can be tested, apart from adding a READ CD
>> test to ahci-test (which I don't really have time for now, hence
>> the RFC tag).  The existing AHCI tests still pass, so patches 1-3
>> aren't complete crap.
>> Paolo
>> Paolo Bonzini (5):
>>   ide: push call to end_transfer_func out of start_transfer callback
>>   ide: push end_transfer callback to ide_transfer_halt
>>   ide: make ide_transfer_stop idempotent
>>   atapi: call ide_set_irq before ide_transfer_start
>>   ide: introduce ide_transfer_start_norecurse
>>  hw/ide/ahci.c             | 12 +++++++-----
>>  hw/ide/atapi.c            | 37 ++++++++++++++++++++-----------------
>>  hw/ide/core.c             | 37 +++++++++++++++++++++++--------------
>>  include/hw/ide/internal.h |  3 +++
>>  4 files changed, 53 insertions(+), 36 deletions(-)
> LGTM; only comments wound up being naming.

The "PIO setup" FIS though should be sent at the *beginning* of data
transfer according to the spec.  And if that is fixed a bunch of things
are simpler (no end_transfer callback!).  I'll test and send next week.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]