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

[Asias He <asias.hejun@gmail.com>]

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
> 


-- 
Best Regards,
Asias He