[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] ide/atapi: make PIO read requests async
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] ide/atapi: make PIO read requests async |
Date: |
Mon, 2 Nov 2015 19:48:58 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 10/12/2015 08:27 AM, Peter Lieven wrote:
> PIO read requests on the ATAPI interface used to be sync blk requests.
> This has two significant drawbacks. First the main loop hangs util an
> I/O request is completed and secondly if the I/O request does not
> complete (e.g. due to an unresponsive storage) Qemu hangs completely.
>
> Signed-off-by: Peter Lieven <address@hidden>
> ---
> hw/ide/atapi.c | 93
> ++++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 84 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 747f466..2271ea2 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -105,11 +105,16 @@ static void cd_data_to_raw(uint8_t *buf, int lba)
> memset(buf, 0, 288);
> }
>
> -static int cd_read_sector(IDEState *s, int lba, uint8_t *buf, int
> sector_size)
> +static int
> +cd_read_sector_sync(IDEState *s, int lba, uint8_t *buf)
> {
> int ret;
>
> - switch(sector_size) {
> +#ifdef DEBUG_IDE_ATAPI
> + printf("cd_read_sector_sync: lba=%d\n", lba);
> +#endif
> +
> + switch (s->cd_sector_size) {
> case 2048:
> block_acct_start(blk_get_stats(s->blk), &s->acct,
> 4 * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
> @@ -129,9 +134,71 @@ static int cd_read_sector(IDEState *s, int lba, uint8_t
> *buf, int sector_size)
> ret = -EIO;
> break;
> }
> +
> + if (!ret) {
> + s->lba++;
> + s->io_buffer_index = 0;
> + }
> +
> return ret;
> }
>
> +static void cd_read_sector_cb(void *opaque, int ret)
> +{
> + IDEState *s = opaque;
> +
> + block_acct_done(blk_get_stats(s->blk), &s->acct);
> +
> +#ifdef DEBUG_IDE_ATAPI
> + printf("cd_read_sector_cb: lba=%d ret=%d\n", s->lba, ret);
> +#endif
> +
> + if (ret < 0) {
> + ide_atapi_io_error(s, ret);
> + return;
> + }
> +
> + if (s->cd_sector_size == 2352) {
> + cd_data_to_raw(s->io_buffer, s->lba);
> + }
> +
> + s->lba++;
> + s->io_buffer_index = 0;
> + s->status &= ~BUSY_STAT;
> +
> + ide_atapi_cmd_reply_end(s);
> +}
> +
> +static int cd_read_sector(IDEState *s, int lba, void *buf)
> +{
> + if (s->cd_sector_size != 2048 && s->cd_sector_size != 2352) {
> + return -EINVAL;
> + }
> +
> + s->iov.iov_base = buf;
> + if (s->cd_sector_size == 2352) {
> + buf += 16;
> + }
> +
> + s->iov.iov_len = 4 * BDRV_SECTOR_SIZE;
> + qemu_iovec_init_external(&s->qiov, &s->iov, 1);
> +
> +#ifdef DEBUG_IDE_ATAPI
> + printf("cd_read_sector: lba=%d\n", lba);
> +#endif
> +
> + if (blk_aio_readv(s->blk, (int64_t)lba << 2, &s->qiov, 4,
> + cd_read_sector_cb, s) == NULL) {
> + return -EIO;
> + }
> +
> + block_acct_start(blk_get_stats(s->blk), &s->acct,
> + 4 * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
> +
> + s->status |= BUSY_STAT;
> + return 0;
> +}
> +
> void ide_atapi_cmd_ok(IDEState *s)
> {
> s->error = 0;
> @@ -182,18 +249,27 @@ void ide_atapi_cmd_reply_end(IDEState *s)
> ide_atapi_cmd_ok(s);
> ide_set_irq(s->bus);
> #ifdef DEBUG_IDE_ATAPI
> - printf("status=0x%x\n", s->status);
> + printf("end of transfer, status=0x%x\n", s->status);
> #endif
> } else {
> /* see if a new sector must be read */
> if (s->lba != -1 && s->io_buffer_index >= s->cd_sector_size) {
> - ret = cd_read_sector(s, s->lba, s->io_buffer, s->cd_sector_size);
> - if (ret < 0) {
> - ide_atapi_io_error(s, ret);
> + if (!s->elementary_transfer_size) {
> + ret = cd_read_sector(s, s->lba, s->io_buffer);
> + if (ret < 0) {
> + ide_atapi_io_error(s, ret);
> + }
> return;
> + } else {
> + /* rebuffering within an elementary transfer is
> + * only possible with a sync request because we
> + * end up with a race condition otherwise */
> + ret = cd_read_sector_sync(s, s->lba, s->io_buffer);
> + if (ret < 0) {
> + ide_atapi_io_error(s, ret);
> + return;
> + }
> }
> - s->lba++;
> - s->io_buffer_index = 0;
> }
> if (s->elementary_transfer_size > 0) {
> /* there are some data left to transmit in this elementary
> @@ -275,7 +351,6 @@ static void ide_atapi_cmd_read_pio(IDEState *s, int lba,
> int nb_sectors,
> s->io_buffer_index = sector_size;
> s->cd_sector_size = sector_size;
>
> - s->status = READY_STAT | SEEK_STAT;
> ide_atapi_cmd_reply_end(s);
> }
>
>
This patch looks good to me, apart from Stefan's other comments. Will
you be sending a V3? I can try to merge it for this week before the hard
freeze hits.
--js
- Re: [Qemu-devel] [PATCH 1/4] ide/atapi: make PIO read requests async,
John Snow <=