Re: [Qemu-devel] [RFC PATCH 0/5] atapi: change unlimited recursion to while loop
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. --js
Re: [Qemu-devel] [RFC PATCH 0/5] atapi: change unlimited recursion to while loop
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(-) > ACK receipt, I will get to this soon, sorry!
[Qemu-devel] [RFC PATCH 0/5] atapi: change unlimited recursion to while loop
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(-) -- 2.14.3
