diff mbox

[4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path

Message ID 1304822300-14299-1-git-send-email-asias.hejun@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Asias He May 8, 2011, 2:38 a.m. UTC
As virtio spec says:

"""
 Because this is high importance and low bandwidth, the current Linux
 implementation polls for the buffer to be used, rather than waiting
 for an interrupt, simplifying the implementation signicantly.
"""

drivers/char/virtio_console.c
 send_buf() {
 ...
	/* Tell Host to go! */
	virtqueue_kick(out_vq);
 ...
        while (!virtqueue_get_buf(out_vq, &len))
                cpu_relax();
 ...
 }

The console hangs can simply be reproduced by yes command which
gives tremendous console IOs and IRQs.

[   16.786440] irq 4: nobody cared (try booting with the "irqpoll" option)
[   16.786440] Pid: 1437, comm: yes Tainted: G        W 2.6.39-rc6+ #56
[   16.786440] Call Trace:
[   16.786440]  [<c16578eb>] __report_bad_irq+0x30/0x89
[   16.786440]  [<c10980e6>] note_interrupt+0x118/0x17a
[   16.786440]  [<c1096e7d>] handle_irq_event_percpu+0x168/0x179
[   16.786440]  [<c1096eba>] handle_irq_event+0x2c/0x46
[   16.786440]  [<c1098516>] ? unmask_irq+0x1e/0x1e
[   16.786440]  [<c1098566>] handle_level_irq+0x50/0x6e
[   16.786440]  <IRQ>  [<c102fa69>] ? do_IRQ+0x35/0x7f
[   16.786440]  [<c1665ea9>] ? common_interrupt+0x29/0x30
[   16.786440]  [<c16610d6>] ? _raw_spin_unlock_irqrestore+0x7/0x28
[   16.786440]  [<c1364f65>] ? hvc_write+0x88/0x9e
[   16.786440]  [<c1355500>] ? do_output_char+0x88/0x18a
[   16.786440]  [<c1355631>] ? process_output+0x2f/0x42
[   16.786440]  [<c1355af6>] ? n_tty_write+0x211/0x2dc
[   16.786440]  [<c1059d77>] ? try_to_wake_up+0x226/0x226
[   16.786440]  [<c13534a4>] ? tty_write+0x15e/0x1d1
[   16.786440]  [<c12c1644>] ? security_file_permission+0x22/0x26
[   16.786440]  [<c13558e5>] ? process_echoes+0x241/0x241
[   16.786440]  [<c10dd9d2>] ? vfs_write+0x84/0xd7
[   16.786440]  [<c1353346>] ? tty_write_lock+0x3d/0x3d
[   16.786440]  [<c10ddb92>] ? sys_write+0x3b/0x5d
[   16.786440]  [<c166594c>] ? sysenter_do_call+0x12/0x22
[   16.786440] handlers:
[   16.786440] [<c1351397>] (vp_interrupt+0x0/0x3a)
[   16.786440] Disabling IRQ #4

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/virtio/console.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

Comments

Ingo Molnar May 8, 2011, 6:22 a.m. UTC | #1
* Asias He <asias.hejun@gmail.com> wrote:

> As virtio spec says:
> 
> """
>  Because this is high importance and low bandwidth, the current Linux
>  implementation polls for the buffer to be used, rather than waiting
>  for an interrupt, simplifying the implementation signicantly.
> """
> 
> drivers/char/virtio_console.c
>  send_buf() {
>  ...
> 	/* Tell Host to go! */
> 	virtqueue_kick(out_vq);
>  ...
>         while (!virtqueue_get_buf(out_vq, &len))
>                 cpu_relax();
>  ...
>  }
> 
> The console hangs can simply be reproduced by yes command which
> gives tremendous console IOs and IRQs.
> 
> [   16.786440] irq 4: nobody cared (try booting with the "irqpoll" option)
> [   16.786440] Pid: 1437, comm: yes Tainted: G        W 2.6.39-rc6+ #56
> [   16.786440] Call Trace:
> [   16.786440]  [<c16578eb>] __report_bad_irq+0x30/0x89
> [   16.786440]  [<c10980e6>] note_interrupt+0x118/0x17a
> [   16.786440]  [<c1096e7d>] handle_irq_event_percpu+0x168/0x179
> [   16.786440]  [<c1096eba>] handle_irq_event+0x2c/0x46
> [   16.786440]  [<c1098516>] ? unmask_irq+0x1e/0x1e
> [   16.786440]  [<c1098566>] handle_level_irq+0x50/0x6e
> [   16.786440]  <IRQ>  [<c102fa69>] ? do_IRQ+0x35/0x7f
> [   16.786440]  [<c1665ea9>] ? common_interrupt+0x29/0x30
> [   16.786440]  [<c16610d6>] ? _raw_spin_unlock_irqrestore+0x7/0x28
> [   16.786440]  [<c1364f65>] ? hvc_write+0x88/0x9e
> [   16.786440]  [<c1355500>] ? do_output_char+0x88/0x18a
> [   16.786440]  [<c1355631>] ? process_output+0x2f/0x42
> [   16.786440]  [<c1355af6>] ? n_tty_write+0x211/0x2dc
> [   16.786440]  [<c1059d77>] ? try_to_wake_up+0x226/0x226
> [   16.786440]  [<c13534a4>] ? tty_write+0x15e/0x1d1
> [   16.786440]  [<c12c1644>] ? security_file_permission+0x22/0x26
> [   16.786440]  [<c13558e5>] ? process_echoes+0x241/0x241
> [   16.786440]  [<c10dd9d2>] ? vfs_write+0x84/0xd7
> [   16.786440]  [<c1353346>] ? tty_write_lock+0x3d/0x3d
> [   16.786440]  [<c10ddb92>] ? sys_write+0x3b/0x5d
> [   16.786440]  [<c166594c>] ? sysenter_do_call+0x12/0x22
> [   16.786440] handlers:
> [   16.786440] [<c1351397>] (vp_interrupt+0x0/0x3a)
> [   16.786440] Disabling IRQ #4

Hm, why is irq #4 active if the guest-side virtio console driver does not 
handle it?

> Signed-off-by: Asias He <asias.hejun@gmail.com>
> ---
>  tools/kvm/virtio/console.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
> index f5449ba..1fecf37 100644
> --- a/tools/kvm/virtio/console.c
> +++ b/tools/kvm/virtio/console.c
> @@ -171,8 +171,6 @@ static void virtio_console_handle_callback(struct kvm *self, void *param)
>  		len = term_putc_iov(CONSOLE_VIRTIO, iov, out);
>  		virt_queue__set_used_elem(vq, head, len);
>  	}
> -
> -	virt_queue__trigger_irq(vq, virtio_console_pci_device.irq_line, &cdev.isr, self);
>  }

I think this at least requires a comment at that place, that we intentionally 
skip notifying the guest, because Linux guests do not use the console IRQ.

Does the guest-side virtio driver *ever* use the irq?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Asias He May 8, 2011, 12:59 p.m. UTC | #2
On 05/08/2011 02:22 PM, Ingo Molnar wrote:
> 
> * Asias He <asias.hejun@gmail.com> wrote:
> 
>> As virtio spec says:
>>
>> """
>>  Because this is high importance and low bandwidth, the current Linux
>>  implementation polls for the buffer to be used, rather than waiting
>>  for an interrupt, simplifying the implementation signicantly.
>> """
>>
>> drivers/char/virtio_console.c
>>  send_buf() {
>>  ...
>> 	/* Tell Host to go! */
>> 	virtqueue_kick(out_vq);
>>  ...
>>         while (!virtqueue_get_buf(out_vq, &len))
>>                 cpu_relax();
>>  ...
>>  }
>>
>> The console hangs can simply be reproduced by yes command which
>> gives tremendous console IOs and IRQs.
>>
>> [   16.786440] irq 4: nobody cared (try booting with the "irqpoll" option)
>> [   16.786440] Pid: 1437, comm: yes Tainted: G        W 2.6.39-rc6+ #56
>> [   16.786440] Call Trace:
>> [   16.786440]  [<c16578eb>] __report_bad_irq+0x30/0x89
>> [   16.786440]  [<c10980e6>] note_interrupt+0x118/0x17a
>> [   16.786440]  [<c1096e7d>] handle_irq_event_percpu+0x168/0x179
>> [   16.786440]  [<c1096eba>] handle_irq_event+0x2c/0x46
>> [   16.786440]  [<c1098516>] ? unmask_irq+0x1e/0x1e
>> [   16.786440]  [<c1098566>] handle_level_irq+0x50/0x6e
>> [   16.786440]  <IRQ>  [<c102fa69>] ? do_IRQ+0x35/0x7f
>> [   16.786440]  [<c1665ea9>] ? common_interrupt+0x29/0x30
>> [   16.786440]  [<c16610d6>] ? _raw_spin_unlock_irqrestore+0x7/0x28
>> [   16.786440]  [<c1364f65>] ? hvc_write+0x88/0x9e
>> [   16.786440]  [<c1355500>] ? do_output_char+0x88/0x18a
>> [   16.786440]  [<c1355631>] ? process_output+0x2f/0x42
>> [   16.786440]  [<c1355af6>] ? n_tty_write+0x211/0x2dc
>> [   16.786440]  [<c1059d77>] ? try_to_wake_up+0x226/0x226
>> [   16.786440]  [<c13534a4>] ? tty_write+0x15e/0x1d1
>> [   16.786440]  [<c12c1644>] ? security_file_permission+0x22/0x26
>> [   16.786440]  [<c13558e5>] ? process_echoes+0x241/0x241
>> [   16.786440]  [<c10dd9d2>] ? vfs_write+0x84/0xd7
>> [   16.786440]  [<c1353346>] ? tty_write_lock+0x3d/0x3d
>> [   16.786440]  [<c10ddb92>] ? sys_write+0x3b/0x5d
>> [   16.786440]  [<c166594c>] ? sysenter_do_call+0x12/0x22
>> [   16.786440] handlers:
>> [   16.786440] [<c1351397>] (vp_interrupt+0x0/0x3a)
>> [   16.786440] Disabling IRQ #4
> 
> Hm, why is irq #4 active if the guest-side virtio console driver does not 
> handle it?
> 
>> Signed-off-by: Asias He <asias.hejun@gmail.com>
>> ---
>>  tools/kvm/virtio/console.c |    2 --
>>  1 files changed, 0 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
>> index f5449ba..1fecf37 100644
>> --- a/tools/kvm/virtio/console.c
>> +++ b/tools/kvm/virtio/console.c
>> @@ -171,8 +171,6 @@ static void virtio_console_handle_callback(struct kvm *self, void *param)
>>  		len = term_putc_iov(CONSOLE_VIRTIO, iov, out);
>>  		virt_queue__set_used_elem(vq, head, len);
>>  	}
>> -
>> -	virt_queue__trigger_irq(vq, virtio_console_pci_device.irq_line, &cdev.isr, self);
>>  }
> 
> I think this at least requires a comment at that place, that we intentionally 
> skip notifying the guest, because Linux guests do not use the console IRQ.

Will do.

> Does the guest-side virtio driver *ever* use the irq?

Yes. They use IRQ at least for the RX path.

> Thanks,
> 
> 	Ingo
>
diff mbox

Patch

diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
index f5449ba..1fecf37 100644
--- a/tools/kvm/virtio/console.c
+++ b/tools/kvm/virtio/console.c
@@ -171,8 +171,6 @@  static void virtio_console_handle_callback(struct kvm *self, void *param)
 		len = term_putc_iov(CONSOLE_VIRTIO, iov, out);
 		virt_queue__set_used_elem(vq, head, len);
 	}
-
-	virt_queue__trigger_irq(vq, virtio_console_pci_device.irq_line, &cdev.isr, self);
 }
 
 static bool virtio_console_pci_io_out(struct kvm *self, u16 port, void *data, int size, u32 count)