* Re: Bug inkvm_set_irq
From: Jean-Philippe Menil @ 2011-02-28 10:40 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20110228101139.GD28006@redhat.com>
Le 28/02/2011 11:11, Michael S. Tsirkin a écrit :
> On Mon, Feb 28, 2011 at 09:56:46AM +0100, Jean-Philippe Menil wrote:
>> Le 27/02/2011 18:00, Michael S. Tsirkin a écrit :
>>> On Fri, Feb 25, 2011 at 10:07:22AM +0100, Jean-Philippe Menil wrote:
>>>> Hi,
>>>>
>>>> Each time i try tou use vhost_net, i'm facing a kernel bug.
>>>> I do a "modprobe vhost_net", and start guest whith vhost=on.
>>>>
>>>> Following is a trace with a kernel 2.6.37, but i had the same
>>>> problem with 2.6.36 (cf https://lkml.org/lkml/2010/11/30/29).
>>> 2.6.36 had a theorectical race that could explain this,
>>> but it should be ok in 2.6.37.
>>>
>>>> The bug only occurs whith vhost_net charged, so i don't know if this
>>>> is a bug in kvm module code or in the vhost_net code.
>>> It could be a bug in eventfd which is the interface
>>> used by both kvm and vhost_net.
>>> Just for fun, you can try 3.6.38 - eventfd code has been changed
>>> a lot in 2.6.38 and if it does not trigger there
>>> it's a hint that irqfd is the reason.
>>>
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.243100] BUG: unable to handle kernel paging request at
>>>> 0000000000002458
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.243250] IP: [<ffffffffa041aa8a>] kvm_set_irq+0x2a/0x130 [kvm]
>>> Could you run markup_oops/ ksymoops on this please?
>>> As far as I can see kvm_set_irq can only get a wrong
>>> kvm pointer. Unless there's some general memory corruption,
>>> I'd guess
>>>
>>> You can also try comparing the irqfd->kvm pointer in
>>> kvm_irqfd_assign irqfd_wakeup and kvm_set_irq in
>>> virt/kvm/eventfd.c.
>>>
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.243378] PGD 45d363067 PUD 45e77a067 PMD 0
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.243556] Oops: 0000 [#1] SMP
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.243692] last sysfs file:
>>>> /sys/devices/pci0000:00/0000:00:0d.0/0000:05:00.0/0000:06:00.0/irq
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [ 685.243777] CPU 0
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.243820] Modules linked in: vhost_net macvtap macvlan tun
>>>> powernow_k8 mperf cpufreq_userspace cpufreq_stats cpufreq_powersave
>>>> cpufreq_ondemand fre
>>>> q_table cpufreq_conservative fuse xt_physdev ip6t_LOG
>>>> ip6table_filter ip6_tables ipt_LOG xt_multiport xt_limit xt_tcpudp
>>>> xt_state iptable_filter ip_tables x_tables nf_conntrack_tftp
>>>> nf_conntrack_ftp nf_connt
>>>> rack_ipv4 nf_defrag_ipv4 8021q bridge stp ext2 mbcache
>>>> dm_round_robin dm_multipath nf_conntrack_ipv6 nf_conntrack
>>>> nf_defrag_ipv6 kvm_amd kvm ipv6 snd_pcm snd_timer snd soundcore
>>>> snd_page_alloc tpm_tis tpm ps
>>>> mouse dcdbas tpm_bios processor i2c_nforce2 shpchp pcspkr ghes
>>>> serio_raw joydev evdev pci_hotplug i2c_core hed button thermal_sys
>>>> xfs exportfs dm_mod sg sr_mod cdrom usbhid hid usb_storage ses
>>>> sd_mod enclosu
>>>> re megaraid_sas ohci_hcd lpfc scsi_transport_fc scsi_tgt bnx2
>>>> scsi_mod ehci_hcd [last unloaded: scsi_wait_scan]
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [ 685.246123]
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] Pid: 10, comm: kworker/0:1 Not tainted
>>>> 2.6.37-dsiun-110105 #17 0K543T/PowerEdge M605
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] RIP: 0010:[<ffffffffa041aa8a>] [<ffffffffa041aa8a>]
>>>> kvm_set_irq+0x2a/0x130 [kvm]
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] RSP: 0018:ffff88045fc89d30 EFLAGS: 00010246
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] RAX: 0000000000000000 RBX: 000000000000001a RCX:
>>>> 0000000000000001
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
>>>> 0000000000000000
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] RBP: 0000000000000000 R08: 0000000000000001 R09:
>>>> ffff880856a91e48
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] R10: 0000000000000000 R11: 00000000ffffffff R12:
>>>> 0000000000000000
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] R13: 0000000000000001 R14: 0000000000000000 R15:
>>>> 0000000000000000
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] FS: 00007f617986c710(0000) GS:ffff88007f800000(0000)
>>>> knlGS:0000000000000000
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] CR2: 0000000000002458 CR3: 000000045d197000 CR4:
>>>> 00000000000006f0
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
>>>> 0000000000000000
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
>>>> 0000000000000400
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] Process kworker/0:1 (pid: 10, threadinfo
>>>> ffff88045fc88000, task ffff88085fc53c30)
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [ 685.246123] Stack:
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] ffff88045fc89fd8 00000000000119c0 ffff88045fc88010
>>>> ffff88085fc53ee8
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] ffff88045fc89fd8 ffff88085fc53ee0 ffff88085fc53c30
>>>> 00000000000119c0
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] 00000000000119c0 ffffffff8137f7ce ffff88007f80df40
>>>> 00000000ffffffff
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] Call Trace:
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffff8137f7ce>] ? common_interrupt+0xe/0x13
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffffa041bc30>] ? irqfd_inject+0x0/0x50 [kvm]
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffffa041bc57>] ? irqfd_inject+0x27/0x50 [kvm]
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffffa041bc30>] ? irqfd_inject+0x0/0x50 [kvm]
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffff8106b6f2>] ? process_one_work+0x112/0x460
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffff8106be25>] ? worker_thread+0x145/0x410
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffff8103a3d0>] ? __wake_up_common+0x50/0x80
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffff8106bce0>] ? worker_thread+0x0/0x410
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffff8106bce0>] ? worker_thread+0x0/0x410
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffff8106f786>] ? kthread+0x96/0xa0
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffff81003ce4>] ? kernel_thread_helper+0x4/0x10
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffff8106f6f0>] ? kthread+0x0/0xa0
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] [<ffffffff81003ce0>] ? kernel_thread_helper+0x0/0x10
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] Code: ff 41 57 41 89 f7 41 56 41 55 41 89 cd 41 54 49 89
>>>> fc 55 53 89 d3 48 81 ec 98 00 00 00 8b 15 c6 79 03 00 85 d2 0f 85 c4
>>>> 00 00 00<4
>>>> 9> 8b 84 24 58 24 00 00 3b 98 28 01 00 00 73 5e 89 db 48 8b 84
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] RIP [<ffffffffa041aa8a>] kvm_set_irq+0x2a/0x130 [kvm]
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] RSP<ffff88045fc89d30>
>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>> 685.246123] CR2: 0000000000002458
>>>>
>>>>
>>>> If someone can help me, on how to solve this.
>>>>
>>>> Regards.
>>>> _______________________________________________
>>>> Virtualization mailing list
>>>> Virtualization@lists.linux-foundation.org
>>>> https://lists.linux-foundation.org/mailman/listinfo/virtualization
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Hi,
>>
>> thanks for your response.
>>
>> This is what markup_oops.pl return me:
>> "No matching code found"
> Well, let's try to understand what's there.
>
> Do objdumop -ldS kvm.ko
> look for<kvm_set_irq>
>
> and paste the content from start of that function
> to offset 0x2a and a bit beyond.
>
> You can also upload your kvm.ko somewhere, I'll try to take a look.
>
>
>> So this is not a vhost_net bug, or my oops is incomplete and
>> markup_oops can't find the good vma offset.
>>
>> I will try to compare the pointers you indicate me, even it could be
>> a little difficult for me.
> Hmm you know how to add printk to code and rebuild, right?
>
>> Maybe i will try a 2.6.38, will wait a response from the kvm team.
>>
>> Regards.
>>
>> --
>> Jean-Philippe Menil - Pôle réseau Service IRTS
>> DSI Université de Nantes
>> jean-philippe.menil@univ-nantes.fr
>> Tel : 02.53.48.49.27 - Fax : 02.53.48.49.09
So, here is the result for the objdump against the kvm.ko (the
kvm_set_irq part) :
0000000000006a60 <kvm_set_irq>:
kvm_set_irq():
6a60: 41 57 push %r15
6a62: 41 89 f7 mov %esi,%r15d
6a65: 41 56 push %r14
6a67: 41 55 push %r13
6a69: 41 89 cd mov %ecx,%r13d
6a6c: 41 54 push %r12
6a6e: 49 89 fc mov %rdi,%r12
6a71: 55 push %rbp
6a72: 53 push %rbx
6a73: 89 d3 mov %edx,%ebx
6a75: 48 81 ec 98 00 00 00 sub $0x98,%rsp
6a7c: 8b 15 00 00 00 00 mov 0x0(%rip),%edx #
6a82 <kvm_set_irq+0x22>
6a82: 85 d2 test %edx,%edx
6a84: 0f 85 c4 00 00 00 jne 6b4e <kvm_set_irq+0xee>
6a8a: 49 8b 84 24 58 24 00 mov 0x2458(%r12),%rax
6a91: 00
6a92: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx
6a98: 73 5e jae 6af8 <kvm_set_irq+0x98>
6a9a: 89 db mov %ebx,%ebx
6a9c: 48 8b 84 d8 30 01 00 mov 0x130(%rax,%rbx,8),%rax
6aa3: 00
6aa4: 48 85 c0 test %rax,%rax
6aa7: 74 4f je 6af8 <kvm_set_irq+0x98>
6aa9: 48 89 e2 mov %rsp,%rdx
6aac: 31 db xor %ebx,%ebx
6aae: 48 8b 08 mov (%rax),%rcx
6ab1: 83 c3 01 add $0x1,%ebx
6ab4: 0f 18 09 prefetcht0 (%rcx)
6ab7: 48 8b 48 e0 mov -0x20(%rax),%rcx
6abb: 48 89 0a mov %rcx,(%rdx)
6abe: 48 8b 48 e8 mov -0x18(%rax),%rcx
6ac2: 48 89 4a 08 mov %rcx,0x8(%rdx)
6ac6: 48 8b 48 f0 mov -0x10(%rax),%rcx
6aca: 48 89 4a 10 mov %rcx,0x10(%rdx)
6ace: 48 8b 48 f8 mov -0x8(%rax),%rcx
6ad2: 48 89 4a 18 mov %rcx,0x18(%rdx)
6ad6: 48 8b 08 mov (%rax),%rcx
6ad9: 48 89 4a 20 mov %rcx,0x20(%rdx)
6add: 48 8b 48 08 mov 0x8(%rax),%rcx
6ae1: 48 89 4a 28 mov %rcx,0x28(%rdx)
6ae5: 48 8b 00 mov (%rax),%rax
6ae8: 48 83 c2 30 add $0x30,%rdx
6aec: 48 85 c0 test %rax,%rax
6aef: 75 bd jne 6aae <kvm_set_irq+0x4e>
6af1: eb 07 jmp 6afa <kvm_set_irq+0x9a>
6af3: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
6af8: 31 db xor %ebx,%ebx
6afa: bd ff ff ff ff mov $0xffffffff,%ebp
6aff: 49 89 e6 mov %rsp,%r14
6b02: 85 db test %ebx,%ebx
6b04: 74 34 je 6b3a <kvm_set_irq+0xda>
6b06: 83 eb 01 sub $0x1,%ebx
6b09: 44 89 e9 mov %r13d,%ecx
6b0c: 44 89 fa mov %r15d,%edx
6b0f: 48 63 c3 movslq %ebx,%rax
6b12: 4c 89 e6 mov %r12,%rsi
6b15: 48 8d 04 40 lea (%rax,%rax,2),%rax
6b19: 48 c1 e0 04 shl $0x4,%rax
6b1d: 49 8d 3c 06 lea (%r14,%rax,1),%rdi
6b21: ff 54 04 08 callq *0x8(%rsp,%rax,1)
6b25: 85 c0 test %eax,%eax
6b27: 78 d9 js 6b02 <kvm_set_irq+0xa2>
6b29: 85 ed test %ebp,%ebp
6b2b: ba 00 00 00 00 mov $0x0,%edx
6b30: 0f 48 ea cmovs %edx,%ebp
6b33: 85 db test %ebx,%ebx
6b35: 8d 2c 28 lea (%rax,%rbp,1),%ebp
6b38: 75 cc jne 6b06 <kvm_set_irq+0xa6>
6b3a: 48 81 c4 98 00 00 00 add $0x98,%rsp
6b41: 89 e8 mov %ebp,%eax
6b43: 5b pop %rbx
6b44: 5d pop %rbp
6b45: 41 5c pop %r12
6b47: 41 5d pop %r13
6b49: 41 5e pop %r14
6b4b: 41 5f pop %r15
6b4d: c3 retq
6b4e: 48 8b 2d 00 00 00 00 mov 0x0(%rip),%rbp #
6b55 <kvm_set_irq+0xf5>
6b55: 48 85 ed test %rbp,%rbp
6b58: 0f 84 2c ff ff ff je 6a8a <kvm_set_irq+0x2a>
6b5e: 48 8b 45 00 mov 0x0(%rbp),%rax
6b62: 48 8b 7d 08 mov 0x8(%rbp),%rdi
6b66: 48 83 c5 10 add $0x10,%rbp
6b6a: 44 89 f9 mov %r15d,%ecx
6b6d: 44 89 ea mov %r13d,%edx
6b70: 89 de mov %ebx,%esi
6b72: ff d0 callq *%rax
6b74: 48 8b 45 00 mov 0x0(%rbp),%rax
6b78: 48 85 c0 test %rax,%rax
6b7b: 75 e5 jne 6b62 <kvm_set_irq+0x102>
6b7d: e9 08 ff ff ff jmpq 6a8a <kvm_set_irq+0x2a>
6b82: 66 66 66 66 66 2e 0f nopw %cs:0x0(%rax,%rax,1)
6b89: 1f 84 00 00 00 00 00
I admit that this analysis is too complicated for me.
I, effectively, can rebuild a kernel with more printk, and program a reboot.
The kvm.ko is available through the following address:
http://filex.univ-nantes.fr/get?k=k1jKhQghdcHLz12Z50H
Regards.
--
Jean-Philippe Menil - Pôle réseau Service IRTS
DSI Université de Nantes
jean-philippe.menil@univ-nantes.fr
Tel : 02.53.48.49.27 - Fax : 02.53.48.49.09
^ permalink raw reply
* Re: Bug inkvm_set_irq
From: Michael S. Tsirkin @ 2011-02-28 11:39 UTC (permalink / raw)
To: Jean-Philippe Menil; +Cc: netdev, kvm, virtualization
In-Reply-To: <4D6B7BAB.9070907@univ-nantes.fr>
On Mon, Feb 28, 2011 at 11:40:43AM +0100, Jean-Philippe Menil wrote:
> Le 28/02/2011 11:11, Michael S. Tsirkin a écrit :
> >On Mon, Feb 28, 2011 at 09:56:46AM +0100, Jean-Philippe Menil wrote:
> >>Le 27/02/2011 18:00, Michael S. Tsirkin a écrit :
> >>>On Fri, Feb 25, 2011 at 10:07:22AM +0100, Jean-Philippe Menil wrote:
> >>>>Hi,
> >>>>
> >>>>Each time i try tou use vhost_net, i'm facing a kernel bug.
> >>>>I do a "modprobe vhost_net", and start guest whith vhost=on.
> >>>>
> >>>>Following is a trace with a kernel 2.6.37, but i had the same
> >>>>problem with 2.6.36 (cf https://lkml.org/lkml/2010/11/30/29).
> >>>2.6.36 had a theorectical race that could explain this,
> >>>but it should be ok in 2.6.37.
> >>>
> >>>>The bug only occurs whith vhost_net charged, so i don't know if this
> >>>>is a bug in kvm module code or in the vhost_net code.
> >>>It could be a bug in eventfd which is the interface
> >>>used by both kvm and vhost_net.
> >>>Just for fun, you can try 3.6.38 - eventfd code has been changed
> >>>a lot in 2.6.38 and if it does not trigger there
> >>>it's a hint that irqfd is the reason.
> >>>
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.243100] BUG: unable to handle kernel paging request at
> >>>>0000000000002458
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.243250] IP: [<ffffffffa041aa8a>] kvm_set_irq+0x2a/0x130 [kvm]
> >>>Could you run markup_oops/ ksymoops on this please?
> >>>As far as I can see kvm_set_irq can only get a wrong
> >>>kvm pointer. Unless there's some general memory corruption,
> >>>I'd guess
> >>>
> >>>You can also try comparing the irqfd->kvm pointer in
> >>>kvm_irqfd_assign irqfd_wakeup and kvm_set_irq in
> >>>virt/kvm/eventfd.c.
> >>>
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.243378] PGD 45d363067 PUD 45e77a067 PMD 0
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.243556] Oops: 0000 [#1] SMP
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.243692] last sysfs file:
> >>>>/sys/devices/pci0000:00/0000:00:0d.0/0000:05:00.0/0000:06:00.0/irq
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [ 685.243777] CPU 0
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.243820] Modules linked in: vhost_net macvtap macvlan tun
> >>>>powernow_k8 mperf cpufreq_userspace cpufreq_stats cpufreq_powersave
> >>>>cpufreq_ondemand fre
> >>>>q_table cpufreq_conservative fuse xt_physdev ip6t_LOG
> >>>>ip6table_filter ip6_tables ipt_LOG xt_multiport xt_limit xt_tcpudp
> >>>>xt_state iptable_filter ip_tables x_tables nf_conntrack_tftp
> >>>>nf_conntrack_ftp nf_connt
> >>>>rack_ipv4 nf_defrag_ipv4 8021q bridge stp ext2 mbcache
> >>>>dm_round_robin dm_multipath nf_conntrack_ipv6 nf_conntrack
> >>>>nf_defrag_ipv6 kvm_amd kvm ipv6 snd_pcm snd_timer snd soundcore
> >>>>snd_page_alloc tpm_tis tpm ps
> >>>>mouse dcdbas tpm_bios processor i2c_nforce2 shpchp pcspkr ghes
> >>>>serio_raw joydev evdev pci_hotplug i2c_core hed button thermal_sys
> >>>>xfs exportfs dm_mod sg sr_mod cdrom usbhid hid usb_storage ses
> >>>>sd_mod enclosu
> >>>>re megaraid_sas ohci_hcd lpfc scsi_transport_fc scsi_tgt bnx2
> >>>>scsi_mod ehci_hcd [last unloaded: scsi_wait_scan]
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [ 685.246123]
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] Pid: 10, comm: kworker/0:1 Not tainted
> >>>>2.6.37-dsiun-110105 #17 0K543T/PowerEdge M605
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] RIP: 0010:[<ffffffffa041aa8a>] [<ffffffffa041aa8a>]
> >>>>kvm_set_irq+0x2a/0x130 [kvm]
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] RSP: 0018:ffff88045fc89d30 EFLAGS: 00010246
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] RAX: 0000000000000000 RBX: 000000000000001a RCX:
> >>>>0000000000000001
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
> >>>>0000000000000000
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] RBP: 0000000000000000 R08: 0000000000000001 R09:
> >>>>ffff880856a91e48
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] R10: 0000000000000000 R11: 00000000ffffffff R12:
> >>>>0000000000000000
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] R13: 0000000000000001 R14: 0000000000000000 R15:
> >>>>0000000000000000
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] FS: 00007f617986c710(0000) GS:ffff88007f800000(0000)
> >>>>knlGS:0000000000000000
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] CR2: 0000000000002458 CR3: 000000045d197000 CR4:
> >>>>00000000000006f0
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> >>>>0000000000000000
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
> >>>>0000000000000400
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] Process kworker/0:1 (pid: 10, threadinfo
> >>>>ffff88045fc88000, task ffff88085fc53c30)
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [ 685.246123] Stack:
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] ffff88045fc89fd8 00000000000119c0 ffff88045fc88010
> >>>>ffff88085fc53ee8
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] ffff88045fc89fd8 ffff88085fc53ee0 ffff88085fc53c30
> >>>>00000000000119c0
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] 00000000000119c0 ffffffff8137f7ce ffff88007f80df40
> >>>>00000000ffffffff
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] Call Trace:
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffff8137f7ce>] ? common_interrupt+0xe/0x13
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffffa041bc30>] ? irqfd_inject+0x0/0x50 [kvm]
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffffa041bc57>] ? irqfd_inject+0x27/0x50 [kvm]
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffffa041bc30>] ? irqfd_inject+0x0/0x50 [kvm]
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffff8106b6f2>] ? process_one_work+0x112/0x460
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffff8106be25>] ? worker_thread+0x145/0x410
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffff8103a3d0>] ? __wake_up_common+0x50/0x80
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffff8106bce0>] ? worker_thread+0x0/0x410
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffff8106bce0>] ? worker_thread+0x0/0x410
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffff8106f786>] ? kthread+0x96/0xa0
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffff81003ce4>] ? kernel_thread_helper+0x4/0x10
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffff8106f6f0>] ? kthread+0x0/0xa0
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] [<ffffffff81003ce0>] ? kernel_thread_helper+0x0/0x10
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] Code: ff 41 57 41 89 f7 41 56 41 55 41 89 cd 41 54 49 89
> >>>>fc 55 53 89 d3 48 81 ec 98 00 00 00 8b 15 c6 79 03 00 85 d2 0f 85 c4
> >>>>00 00 00<4
> >>>>9> 8b 84 24 58 24 00 00 3b 98 28 01 00 00 73 5e 89 db 48 8b 84
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] RIP [<ffffffffa041aa8a>] kvm_set_irq+0x2a/0x130 [kvm]
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] RSP<ffff88045fc89d30>
> >>>>Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
> >>>>685.246123] CR2: 0000000000002458
> >>>>
> >>>>
> >>>>If someone can help me, on how to solve this.
> >>>>
> >>>>Regards.
> >>>>_______________________________________________
> >>>>Virtualization mailing list
> >>>>Virtualization@lists.linux-foundation.org
> >>>>https://lists.linux-foundation.org/mailman/listinfo/virtualization
> >>>--
> >>>To unsubscribe from this list: send the line "unsubscribe netdev" in
> >>>the body of a message to majordomo@vger.kernel.org
> >>>More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>Hi,
> >>
> >>thanks for your response.
> >>
> >>This is what markup_oops.pl return me:
> >>"No matching code found"
> >Well, let's try to understand what's there.
> >
> >Do objdumop -ldS kvm.ko
> >look for<kvm_set_irq>
> >
> >and paste the content from start of that function
> >to offset 0x2a and a bit beyond.
> >
> >You can also upload your kvm.ko somewhere, I'll try to take a look.
> >
> >
> >>So this is not a vhost_net bug, or my oops is incomplete and
> >>markup_oops can't find the good vma offset.
> >>
> >>I will try to compare the pointers you indicate me, even it could be
> >>a little difficult for me.
> >Hmm you know how to add printk to code and rebuild, right?
> >
> >>Maybe i will try a 2.6.38, will wait a response from the kvm team.
> >>
> >>Regards.
> >>
> >>--
> >>Jean-Philippe Menil - Pôle réseau Service IRTS
> >>DSI Université de Nantes
> >>jean-philippe.menil@univ-nantes.fr
> >>Tel : 02.53.48.49.27 - Fax : 02.53.48.49.09
> So, here is the result for the objdump against the kvm.ko (the
> kvm_set_irq part) :
Can you try building with -g and adding -l and -S to objdump
please? I'd rather make the tool do the legwork than
do it manually.
>
> 0000000000006a60 <kvm_set_irq>:
> kvm_set_irq():
> 6a60: 41 57 push %r15
> 6a62: 41 89 f7 mov %esi,%r15d
> 6a65: 41 56 push %r14
> 6a67: 41 55 push %r13
> 6a69: 41 89 cd mov %ecx,%r13d
> 6a6c: 41 54 push %r12
> 6a6e: 49 89 fc mov %rdi,%r12
> 6a71: 55 push %rbp
> 6a72: 53 push %rbx
> 6a73: 89 d3 mov %edx,%ebx
> 6a75: 48 81 ec 98 00 00 00 sub $0x98,%rsp
> 6a7c: 8b 15 00 00 00 00 mov 0x0(%rip),%edx
> # 6a82 <kvm_set_irq+0x22>
> 6a82: 85 d2 test %edx,%edx
> 6a84: 0f 85 c4 00 00 00 jne 6b4e <kvm_set_irq+0xee>
> 6a8a: 49 8b 84 24 58 24 00 mov 0x2458(%r12),%rax
OK, 0x6a8a is the offset.
After you build with -g, try
addr2line kvm.ko 0x6a8a
and see which line this points to.
> 6a91: 00
> 6a92: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx
> 6a98: 73 5e jae 6af8 <kvm_set_irq+0x98>
> 6a9a: 89 db mov %ebx,%ebx
> 6a9c: 48 8b 84 d8 30 01 00 mov 0x130(%rax,%rbx,8),%rax
> 6aa3: 00
> 6aa4: 48 85 c0 test %rax,%rax
> 6aa7: 74 4f je 6af8 <kvm_set_irq+0x98>
> 6aa9: 48 89 e2 mov %rsp,%rdx
> 6aac: 31 db xor %ebx,%ebx
> 6aae: 48 8b 08 mov (%rax),%rcx
> 6ab1: 83 c3 01 add $0x1,%ebx
> 6ab4: 0f 18 09 prefetcht0 (%rcx)
> 6ab7: 48 8b 48 e0 mov -0x20(%rax),%rcx
> 6abb: 48 89 0a mov %rcx,(%rdx)
> 6abe: 48 8b 48 e8 mov -0x18(%rax),%rcx
> 6ac2: 48 89 4a 08 mov %rcx,0x8(%rdx)
> 6ac6: 48 8b 48 f0 mov -0x10(%rax),%rcx
> 6aca: 48 89 4a 10 mov %rcx,0x10(%rdx)
> 6ace: 48 8b 48 f8 mov -0x8(%rax),%rcx
> 6ad2: 48 89 4a 18 mov %rcx,0x18(%rdx)
> 6ad6: 48 8b 08 mov (%rax),%rcx
> 6ad9: 48 89 4a 20 mov %rcx,0x20(%rdx)
> 6add: 48 8b 48 08 mov 0x8(%rax),%rcx
> 6ae1: 48 89 4a 28 mov %rcx,0x28(%rdx)
> 6ae5: 48 8b 00 mov (%rax),%rax
> 6ae8: 48 83 c2 30 add $0x30,%rdx
> 6aec: 48 85 c0 test %rax,%rax
> 6aef: 75 bd jne 6aae <kvm_set_irq+0x4e>
> 6af1: eb 07 jmp 6afa <kvm_set_irq+0x9a>
> 6af3: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
> 6af8: 31 db xor %ebx,%ebx
> 6afa: bd ff ff ff ff mov $0xffffffff,%ebp
> 6aff: 49 89 e6 mov %rsp,%r14
> 6b02: 85 db test %ebx,%ebx
> 6b04: 74 34 je 6b3a <kvm_set_irq+0xda>
> 6b06: 83 eb 01 sub $0x1,%ebx
> 6b09: 44 89 e9 mov %r13d,%ecx
> 6b0c: 44 89 fa mov %r15d,%edx
> 6b0f: 48 63 c3 movslq %ebx,%rax
> 6b12: 4c 89 e6 mov %r12,%rsi
> 6b15: 48 8d 04 40 lea (%rax,%rax,2),%rax
> 6b19: 48 c1 e0 04 shl $0x4,%rax
> 6b1d: 49 8d 3c 06 lea (%r14,%rax,1),%rdi
> 6b21: ff 54 04 08 callq *0x8(%rsp,%rax,1)
> 6b25: 85 c0 test %eax,%eax
> 6b27: 78 d9 js 6b02 <kvm_set_irq+0xa2>
> 6b29: 85 ed test %ebp,%ebp
> 6b2b: ba 00 00 00 00 mov $0x0,%edx
> 6b30: 0f 48 ea cmovs %edx,%ebp
> 6b33: 85 db test %ebx,%ebx
> 6b35: 8d 2c 28 lea (%rax,%rbp,1),%ebp
> 6b38: 75 cc jne 6b06 <kvm_set_irq+0xa6>
> 6b3a: 48 81 c4 98 00 00 00 add $0x98,%rsp
> 6b41: 89 e8 mov %ebp,%eax
> 6b43: 5b pop %rbx
> 6b44: 5d pop %rbp
> 6b45: 41 5c pop %r12
> 6b47: 41 5d pop %r13
> 6b49: 41 5e pop %r14
> 6b4b: 41 5f pop %r15
> 6b4d: c3 retq
> 6b4e: 48 8b 2d 00 00 00 00 mov 0x0(%rip),%rbp
> # 6b55 <kvm_set_irq+0xf5>
> 6b55: 48 85 ed test %rbp,%rbp
> 6b58: 0f 84 2c ff ff ff je 6a8a <kvm_set_irq+0x2a>
> 6b5e: 48 8b 45 00 mov 0x0(%rbp),%rax
> 6b62: 48 8b 7d 08 mov 0x8(%rbp),%rdi
> 6b66: 48 83 c5 10 add $0x10,%rbp
> 6b6a: 44 89 f9 mov %r15d,%ecx
> 6b6d: 44 89 ea mov %r13d,%edx
> 6b70: 89 de mov %ebx,%esi
> 6b72: ff d0 callq *%rax
> 6b74: 48 8b 45 00 mov 0x0(%rbp),%rax
> 6b78: 48 85 c0 test %rax,%rax
> 6b7b: 75 e5 jne 6b62 <kvm_set_irq+0x102>
> 6b7d: e9 08 ff ff ff jmpq 6a8a <kvm_set_irq+0x2a>
> 6b82: 66 66 66 66 66 2e 0f nopw %cs:0x0(%rax,%rax,1)
> 6b89: 1f 84 00 00 00 00 00
>
> I admit that this analysis is too complicated for me.
> I, effectively, can rebuild a kernel with more printk, and program a reboot.
>
> The kvm.ko is available through the following address:
> http://filex.univ-nantes.fr/get?k=k1jKhQghdcHLz12Z50H
>
> Regards.
This has no debug data. Can you rebuild with -g please?
BTW if you want to rerun and get more reliable backtrace,
tyr enabling frame pointers (do you know how to?). But this will change code
so backtrace will no longer be val we will need
a new one.
> --
> Jean-Philippe Menil - Pôle réseau Service IRTS
> DSI Université de Nantes
> jean-philippe.menil@univ-nantes.fr
> Tel : 02.53.48.49.27 - Fax : 02.53.48.49.09
^ permalink raw reply
* Re: Bug inkvm_set_irq
From: Jean-Philippe Menil @ 2011-02-28 15:13 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20110228113939.GH28006@redhat.com>
Le 28/02/2011 12:39, Michael S. Tsirkin a écrit :
> On Mon, Feb 28, 2011 at 11:40:43AM +0100, Jean-Philippe Menil wrote:
>> Le 28/02/2011 11:11, Michael S. Tsirkin a écrit :
>>> On Mon, Feb 28, 2011 at 09:56:46AM +0100, Jean-Philippe Menil wrote:
>>>> Le 27/02/2011 18:00, Michael S. Tsirkin a écrit :
>>>>> On Fri, Feb 25, 2011 at 10:07:22AM +0100, Jean-Philippe Menil wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Each time i try tou use vhost_net, i'm facing a kernel bug.
>>>>>> I do a "modprobe vhost_net", and start guest whith vhost=on.
>>>>>>
>>>>>> Following is a trace with a kernel 2.6.37, but i had the same
>>>>>> problem with 2.6.36 (cf https://lkml.org/lkml/2010/11/30/29).
>>>>> 2.6.36 had a theorectical race that could explain this,
>>>>> but it should be ok in 2.6.37.
>>>>>
>>>>>> The bug only occurs whith vhost_net charged, so i don't know if this
>>>>>> is a bug in kvm module code or in the vhost_net code.
>>>>> It could be a bug in eventfd which is the interface
>>>>> used by both kvm and vhost_net.
>>>>> Just for fun, you can try 3.6.38 - eventfd code has been changed
>>>>> a lot in 2.6.38 and if it does not trigger there
>>>>> it's a hint that irqfd is the reason.
>>>>>
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.243100] BUG: unable to handle kernel paging request at
>>>>>> 0000000000002458
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.243250] IP: [<ffffffffa041aa8a>] kvm_set_irq+0x2a/0x130 [kvm]
>>>>> Could you run markup_oops/ ksymoops on this please?
>>>>> As far as I can see kvm_set_irq can only get a wrong
>>>>> kvm pointer. Unless there's some general memory corruption,
>>>>> I'd guess
>>>>>
>>>>> You can also try comparing the irqfd->kvm pointer in
>>>>> kvm_irqfd_assign irqfd_wakeup and kvm_set_irq in
>>>>> virt/kvm/eventfd.c.
>>>>>
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.243378] PGD 45d363067 PUD 45e77a067 PMD 0
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.243556] Oops: 0000 [#1] SMP
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.243692] last sysfs file:
>>>>>> /sys/devices/pci0000:00/0000:00:0d.0/0000:05:00.0/0000:06:00.0/irq
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [ 685.243777] CPU 0
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.243820] Modules linked in: vhost_net macvtap macvlan tun
>>>>>> powernow_k8 mperf cpufreq_userspace cpufreq_stats cpufreq_powersave
>>>>>> cpufreq_ondemand fre
>>>>>> q_table cpufreq_conservative fuse xt_physdev ip6t_LOG
>>>>>> ip6table_filter ip6_tables ipt_LOG xt_multiport xt_limit xt_tcpudp
>>>>>> xt_state iptable_filter ip_tables x_tables nf_conntrack_tftp
>>>>>> nf_conntrack_ftp nf_connt
>>>>>> rack_ipv4 nf_defrag_ipv4 8021q bridge stp ext2 mbcache
>>>>>> dm_round_robin dm_multipath nf_conntrack_ipv6 nf_conntrack
>>>>>> nf_defrag_ipv6 kvm_amd kvm ipv6 snd_pcm snd_timer snd soundcore
>>>>>> snd_page_alloc tpm_tis tpm ps
>>>>>> mouse dcdbas tpm_bios processor i2c_nforce2 shpchp pcspkr ghes
>>>>>> serio_raw joydev evdev pci_hotplug i2c_core hed button thermal_sys
>>>>>> xfs exportfs dm_mod sg sr_mod cdrom usbhid hid usb_storage ses
>>>>>> sd_mod enclosu
>>>>>> re megaraid_sas ohci_hcd lpfc scsi_transport_fc scsi_tgt bnx2
>>>>>> scsi_mod ehci_hcd [last unloaded: scsi_wait_scan]
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [ 685.246123]
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] Pid: 10, comm: kworker/0:1 Not tainted
>>>>>> 2.6.37-dsiun-110105 #17 0K543T/PowerEdge M605
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] RIP: 0010:[<ffffffffa041aa8a>] [<ffffffffa041aa8a>]
>>>>>> kvm_set_irq+0x2a/0x130 [kvm]
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] RSP: 0018:ffff88045fc89d30 EFLAGS: 00010246
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] RAX: 0000000000000000 RBX: 000000000000001a RCX:
>>>>>> 0000000000000001
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
>>>>>> 0000000000000000
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] RBP: 0000000000000000 R08: 0000000000000001 R09:
>>>>>> ffff880856a91e48
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] R10: 0000000000000000 R11: 00000000ffffffff R12:
>>>>>> 0000000000000000
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] R13: 0000000000000001 R14: 0000000000000000 R15:
>>>>>> 0000000000000000
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] FS: 00007f617986c710(0000) GS:ffff88007f800000(0000)
>>>>>> knlGS:0000000000000000
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] CR2: 0000000000002458 CR3: 000000045d197000 CR4:
>>>>>> 00000000000006f0
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
>>>>>> 0000000000000000
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
>>>>>> 0000000000000400
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] Process kworker/0:1 (pid: 10, threadinfo
>>>>>> ffff88045fc88000, task ffff88085fc53c30)
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [ 685.246123] Stack:
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] ffff88045fc89fd8 00000000000119c0 ffff88045fc88010
>>>>>> ffff88085fc53ee8
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] ffff88045fc89fd8 ffff88085fc53ee0 ffff88085fc53c30
>>>>>> 00000000000119c0
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] 00000000000119c0 ffffffff8137f7ce ffff88007f80df40
>>>>>> 00000000ffffffff
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] Call Trace:
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffff8137f7ce>] ? common_interrupt+0xe/0x13
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffffa041bc30>] ? irqfd_inject+0x0/0x50 [kvm]
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffffa041bc57>] ? irqfd_inject+0x27/0x50 [kvm]
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffffa041bc30>] ? irqfd_inject+0x0/0x50 [kvm]
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffff8106b6f2>] ? process_one_work+0x112/0x460
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffff8106be25>] ? worker_thread+0x145/0x410
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffff8103a3d0>] ? __wake_up_common+0x50/0x80
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffff8106bce0>] ? worker_thread+0x0/0x410
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffff8106bce0>] ? worker_thread+0x0/0x410
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffff8106f786>] ? kthread+0x96/0xa0
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffff81003ce4>] ? kernel_thread_helper+0x4/0x10
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffff8106f6f0>] ? kthread+0x0/0xa0
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] [<ffffffff81003ce0>] ? kernel_thread_helper+0x0/0x10
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] Code: ff 41 57 41 89 f7 41 56 41 55 41 89 cd 41 54 49 89
>>>>>> fc 55 53 89 d3 48 81 ec 98 00 00 00 8b 15 c6 79 03 00 85 d2 0f 85 c4
>>>>>> 00 00 00<4
>>>>>> 9> 8b 84 24 58 24 00 00 3b 98 28 01 00 00 73 5e 89 db 48 8b 84
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] RIP [<ffffffffa041aa8a>] kvm_set_irq+0x2a/0x130 [kvm]
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] RSP<ffff88045fc89d30>
>>>>>> Feb 23 13:56:19 ayrshire.u06.univ-nantes.prive kernel: [
>>>>>> 685.246123] CR2: 0000000000002458
>>>>>>
>>>>>>
>>>>>> If someone can help me, on how to solve this.
>>>>>>
>>>>>> Regards.
>>>>>> _______________________________________________
>>>>>> Virtualization mailing list
>>>>>> Virtualization@lists.linux-foundation.org
>>>>>> https://lists.linux-foundation.org/mailman/listinfo/virtualization
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>> Hi,
>>>>
>>>> thanks for your response.
>>>>
>>>> This is what markup_oops.pl return me:
>>>> "No matching code found"
>>> Well, let's try to understand what's there.
>>>
>>> Do objdumop -ldS kvm.ko
>>> look for<kvm_set_irq>
>>>
>>> and paste the content from start of that function
>>> to offset 0x2a and a bit beyond.
>>>
>>> You can also upload your kvm.ko somewhere, I'll try to take a look.
>>>
>>>
>>>> So this is not a vhost_net bug, or my oops is incomplete and
>>>> markup_oops can't find the good vma offset.
>>>>
>>>> I will try to compare the pointers you indicate me, even it could be
>>>> a little difficult for me.
>>> Hmm you know how to add printk to code and rebuild, right?
>>>
>>>> Maybe i will try a 2.6.38, will wait a response from the kvm team.
>>>>
>>>> Regards.
>>>>
>>>> --
>>>> Jean-Philippe Menil - Pôle réseau Service IRTS
>>>> DSI Université de Nantes
>>>> jean-philippe.menil@univ-nantes.fr
>>>> Tel : 02.53.48.49.27 - Fax : 02.53.48.49.09
>> So, here is the result for the objdump against the kvm.ko (the
>> kvm_set_irq part) :
> Can you try building with -g and adding -l and -S to objdump
> please? I'd rather make the tool do the legwork than
> do it manually.
>
>> 0000000000006a60<kvm_set_irq>:
>> kvm_set_irq():
>> 6a60: 41 57 push %r15
>> 6a62: 41 89 f7 mov %esi,%r15d
>> 6a65: 41 56 push %r14
>> 6a67: 41 55 push %r13
>> 6a69: 41 89 cd mov %ecx,%r13d
>> 6a6c: 41 54 push %r12
>> 6a6e: 49 89 fc mov %rdi,%r12
>> 6a71: 55 push %rbp
>> 6a72: 53 push %rbx
>> 6a73: 89 d3 mov %edx,%ebx
>> 6a75: 48 81 ec 98 00 00 00 sub $0x98,%rsp
>> 6a7c: 8b 15 00 00 00 00 mov 0x0(%rip),%edx
>> # 6a82<kvm_set_irq+0x22>
>> 6a82: 85 d2 test %edx,%edx
>> 6a84: 0f 85 c4 00 00 00 jne 6b4e<kvm_set_irq+0xee>
>> 6a8a: 49 8b 84 24 58 24 00 mov 0x2458(%r12),%rax
> OK, 0x6a8a is the offset.
> After you build with -g, try
>
> addr2line kvm.ko 0x6a8a
>
> and see which line this points to.
>
>
>> 6a91: 00
>> 6a92: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx
>> 6a98: 73 5e jae 6af8<kvm_set_irq+0x98>
>> 6a9a: 89 db mov %ebx,%ebx
>> 6a9c: 48 8b 84 d8 30 01 00 mov 0x130(%rax,%rbx,8),%rax
>> 6aa3: 00
>> 6aa4: 48 85 c0 test %rax,%rax
>> 6aa7: 74 4f je 6af8<kvm_set_irq+0x98>
>> 6aa9: 48 89 e2 mov %rsp,%rdx
>> 6aac: 31 db xor %ebx,%ebx
>> 6aae: 48 8b 08 mov (%rax),%rcx
>> 6ab1: 83 c3 01 add $0x1,%ebx
>> 6ab4: 0f 18 09 prefetcht0 (%rcx)
>> 6ab7: 48 8b 48 e0 mov -0x20(%rax),%rcx
>> 6abb: 48 89 0a mov %rcx,(%rdx)
>> 6abe: 48 8b 48 e8 mov -0x18(%rax),%rcx
>> 6ac2: 48 89 4a 08 mov %rcx,0x8(%rdx)
>> 6ac6: 48 8b 48 f0 mov -0x10(%rax),%rcx
>> 6aca: 48 89 4a 10 mov %rcx,0x10(%rdx)
>> 6ace: 48 8b 48 f8 mov -0x8(%rax),%rcx
>> 6ad2: 48 89 4a 18 mov %rcx,0x18(%rdx)
>> 6ad6: 48 8b 08 mov (%rax),%rcx
>> 6ad9: 48 89 4a 20 mov %rcx,0x20(%rdx)
>> 6add: 48 8b 48 08 mov 0x8(%rax),%rcx
>> 6ae1: 48 89 4a 28 mov %rcx,0x28(%rdx)
>> 6ae5: 48 8b 00 mov (%rax),%rax
>> 6ae8: 48 83 c2 30 add $0x30,%rdx
>> 6aec: 48 85 c0 test %rax,%rax
>> 6aef: 75 bd jne 6aae<kvm_set_irq+0x4e>
>> 6af1: eb 07 jmp 6afa<kvm_set_irq+0x9a>
>> 6af3: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
>> 6af8: 31 db xor %ebx,%ebx
>> 6afa: bd ff ff ff ff mov $0xffffffff,%ebp
>> 6aff: 49 89 e6 mov %rsp,%r14
>> 6b02: 85 db test %ebx,%ebx
>> 6b04: 74 34 je 6b3a<kvm_set_irq+0xda>
>> 6b06: 83 eb 01 sub $0x1,%ebx
>> 6b09: 44 89 e9 mov %r13d,%ecx
>> 6b0c: 44 89 fa mov %r15d,%edx
>> 6b0f: 48 63 c3 movslq %ebx,%rax
>> 6b12: 4c 89 e6 mov %r12,%rsi
>> 6b15: 48 8d 04 40 lea (%rax,%rax,2),%rax
>> 6b19: 48 c1 e0 04 shl $0x4,%rax
>> 6b1d: 49 8d 3c 06 lea (%r14,%rax,1),%rdi
>> 6b21: ff 54 04 08 callq *0x8(%rsp,%rax,1)
>> 6b25: 85 c0 test %eax,%eax
>> 6b27: 78 d9 js 6b02<kvm_set_irq+0xa2>
>> 6b29: 85 ed test %ebp,%ebp
>> 6b2b: ba 00 00 00 00 mov $0x0,%edx
>> 6b30: 0f 48 ea cmovs %edx,%ebp
>> 6b33: 85 db test %ebx,%ebx
>> 6b35: 8d 2c 28 lea (%rax,%rbp,1),%ebp
>> 6b38: 75 cc jne 6b06<kvm_set_irq+0xa6>
>> 6b3a: 48 81 c4 98 00 00 00 add $0x98,%rsp
>> 6b41: 89 e8 mov %ebp,%eax
>> 6b43: 5b pop %rbx
>> 6b44: 5d pop %rbp
>> 6b45: 41 5c pop %r12
>> 6b47: 41 5d pop %r13
>> 6b49: 41 5e pop %r14
>> 6b4b: 41 5f pop %r15
>> 6b4d: c3 retq
>> 6b4e: 48 8b 2d 00 00 00 00 mov 0x0(%rip),%rbp
>> # 6b55<kvm_set_irq+0xf5>
>> 6b55: 48 85 ed test %rbp,%rbp
>> 6b58: 0f 84 2c ff ff ff je 6a8a<kvm_set_irq+0x2a>
>> 6b5e: 48 8b 45 00 mov 0x0(%rbp),%rax
>> 6b62: 48 8b 7d 08 mov 0x8(%rbp),%rdi
>> 6b66: 48 83 c5 10 add $0x10,%rbp
>> 6b6a: 44 89 f9 mov %r15d,%ecx
>> 6b6d: 44 89 ea mov %r13d,%edx
>> 6b70: 89 de mov %ebx,%esi
>> 6b72: ff d0 callq *%rax
>> 6b74: 48 8b 45 00 mov 0x0(%rbp),%rax
>> 6b78: 48 85 c0 test %rax,%rax
>> 6b7b: 75 e5 jne 6b62<kvm_set_irq+0x102>
>> 6b7d: e9 08 ff ff ff jmpq 6a8a<kvm_set_irq+0x2a>
>> 6b82: 66 66 66 66 66 2e 0f nopw %cs:0x0(%rax,%rax,1)
>> 6b89: 1f 84 00 00 00 00 00
>>
>> I admit that this analysis is too complicated for me.
>> I, effectively, can rebuild a kernel with more printk, and program a reboot.
>>
>> The kvm.ko is available through the following address:
>> http://filex.univ-nantes.fr/get?k=k1jKhQghdcHLz12Z50H
>>
>> Regards.
> This has no debug data. Can you rebuild with -g please?
>
> BTW if you want to rerun and get more reliable backtrace,
> tyr enabling frame pointers (do you know how to?). But this will change code
> so backtrace will no longer be val we will need
> a new one.
>
>> --
>> Jean-Philippe Menil - Pôle réseau Service IRTS
>> DSI Université de Nantes
>> jean-philippe.menil@univ-nantes.fr
>> Tel : 02.53.48.49.27 - Fax : 02.53.48.49.09
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Host reboot with his new kernel (2.6.37.2), i modprobe vhost_net, start
three kvm guests.
Host hang in a half an hour.
This time i get a general protection fault:
[ 2380.381225] general protection fault: 0000 [#1] SMP
[ 2380.381261] last sysfs file:
/sys/devices/system/cpu/cpu11/cache/index2/shared_cpu_map
[ 2380.381309] CPU 0
[ 2380.381316] Modules linked in: vhost_net macvtap macvlan tun veth
powernow_k8 mperf cpufreq_userspace cpufreq_stats cpufreq_powersave
cpufreq_ondemand freq_table cpufreq_conservative fuse xt_physdev
ip6t_LOG ip6table_filter ip6_tables ipt_LOG xt_multiport xt_limit
xt_tcpudp xt_state iptable_filter ip_tables x_tables nf_conntrack_tftp
nf_conntrack_ftp nf_conntrack_ipv4 nf_defrag_ipv4 8021q bridge stp ext2
mbcache dm_round_robin dm_multipath nf_conntrack_ipv6 nf_conntrack
nf_defrag_ipv6 kvm_amd kvm ipv6 snd_pcm snd_timer snd soundcore
snd_page_alloc shpchp i2c_nforce2 pci_hotplug psmouse tpm_tis joydev
pcspkr tpm evdev i2c_core dcdbas tpm_bios serio_raw processor ghes
button hed thermal_sys xfs exportfs dm_mod sg sr_mod cdrom usbhid hid
usb_storage sd_mod ses enclosure megaraid_sas lpfc ohci_hcd
scsi_transport_fc scsi_tgt scsi_mod bnx2 ehci_hcd [last unloaded:
scsi_wait_scan]
Feb 28 15:28:09 ayrshire.u06.univ-nantes.prive kernel: Feb 28 15:28:09
ayrshire.u06.univ-nantes.prive kernel: [ 2380.381839] Pid: 10, comm:
kworker/0:1 Not tainted 2.6.37.2-dsiun-110105+ #2 Dell Inc. PowerEdge
M605/0K543T
[ 2380.381902] RIP: 0010:[<ffffffffa037e877>] [<ffffffffa037e877>]
kvm_set_irq+0x37/0x140 [kvm]
[ 2380.381973] RSP: 0018:ffff88045fc85d00 EFLAGS: 00010246
[ 2380.382002] RAX: 000200740000029c RBX: 000000000000001a RCX:
0000000000000001
[ 2380.382035] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
ffff88045dbb7440
[ 2380.382068] RBP: ffff88045fc85dd0 R08: ffff88045fc84000 R09:
000000000000000c
[ 2380.382101] R10: 0000000000000036 R11: 00000000ffffffff R12:
ffff88045dbb7440
[ 2380.382134] R13: ffff88045dbb7440 R14: ffffffffa037faa0 R15:
0000000000000001
[ 2380.382168] FS: 00007f0c97165720(0000) GS:ffff88007f800000(0000)
knlGS:0000000000000000
[ 2380.382216] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 2380.382246] CR2: 00007f13bcc80b40 CR3: 000000045e96c000 CR4:
00000000000006f0
[ 2380.382279] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 2380.382312] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
0000000000000400
[ 2380.382347] Process kworker/0:1 (pid: 10, threadinfo
ffff88045fc84000, task ffff88085fc53c30)
[ 2380.382395] Stack:
[ 2380.382416] 00000000000119c0 00000000000119c0 00000000000119c0
ffff88085fc53c30
[ 2380.382466] ffff88085fc53ee0 ffff88045fc85fd8 ffff88085fc53ee8
ffff88045fc84010
[ 2380.382516] 00000000000119c0 ffff88045fc85fd8 00000000000119c0
00000000000119c0
[ 2380.382566] Call Trace:
[ 2380.382600] [<ffffffff813818ce>] ? common_interrupt+0xe/0x13
[ 2380.382648] [<ffffffffa037faa0>] ? irqfd_inject+0x0/0x50 [kvm]
[ 2380.382694] [<ffffffffa037faca>] irqfd_inject+0x2a/0x50 [kvm]
[ 2380.382729] [<ffffffff8106b7bb>] process_one_work+0x11b/0x450
[ 2380.382762] [<ffffffff8106bf37>] worker_thread+0x157/0x410
[ 2380.382796] [<ffffffff8103a569>] ? __wake_up_common+0x59/0x90
[ 2380.382828] [<ffffffff8106bde0>] ? worker_thread+0x0/0x410
[ 2380.382861] [<ffffffff8106f996>] kthread+0x96/0xa0
[ 2380.382894] [<ffffffff81003c64>] kernel_thread_helper+0x4/0x10
[ 2380.382927] [<ffffffff8106f900>] ? kthread+0x0/0xa0
[ 2380.382958] [<ffffffff81003c60>] ? kernel_thread_helper+0x0/0x10
[ 2380.382987] Code: 55 49 89 fd 41 54 53 89 d3 48 81 ec a8 00 00 00 8b
15 a6 75 03 00 89 b5 3c ff ff ff 85 d2 0f 85 d5 00 00 00 49 8b 85 58 24
00 00 <3b> 98 28 01 00 00 73 61 89 db 48 8b 84 d8 30 01 00 00 48 85 c0
[ 2380.383185] RIP [<ffffffffa037e877>] kvm_set_irq+0x37/0x140 [kvm]
[ 2380.383231] RSP <ffff88045fc85d00>
Running markup_oops.pl give me the following:
vmaoffset = 18446744072102576128 ffffffffa037e841: 48 89
e5 mov %rsp,%rbp
ffffffffa037e844: 41 57 push %r15
ffffffffa037e846: 41 89 cf mov %ecx,%r15d |
%r15 => 1 %ecx = 1
ffffffffa037e849: 41 56 push %r14 |
%r14 => ffffffffa037faa0
ffffffffa037e84b: 41 55 push %r13
ffffffffa037e84d: 49 89 fd mov %rdi,%r13 |
%edi = ffff88045dbb7440 %r13 => ffff88045dbb7440
ffffffffa037e850: 41 54 push %r12 |
%r12 => ffff88045dbb7440
ffffffffa037e852: 53 push %rbx
ffffffffa037e853: 89 d3 mov %edx,%ebx |
%ebx => 1a
ffffffffa037e855: 48 81 ec a8 00 00 00 sub $0xa8,%rsp
ffffffffa037e85c: 8b 15 00 00 00 00 mov
0x0(%rip),%edx # ffffffffa037e862 <kvm_set_irq+0x22>
ffffffffa037e862: 89 b5 3c ff ff ff mov %esi,-0xc4(%rbp)
| %esi = 0
ffffffffa037e868: 85 d2 test %edx,%edx |
%edx => 0
ffffffffa037e86a: 0f 85 d5 00 00 00 jne ffffffffa037e945
<kvm_set_irq+0x105>
ffffffffa037e870: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax
| %eax => 200740000029c %r13 = ffff88045dbb7440
*ffffffffa037e877: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx
| %eax = 200740000029c %ebx = 1a <--- faulting instruction
ffffffffa037e87d: 73 61 jae ffffffffa037e8e0
<kvm_set_irq+0xa0>
ffffffffa037e87f: 89 db mov %ebx,%ebx
ffffffffa037e881: 48 8b 84 d8 30 01 00 mov
0x130(%rax,%rbx,8),%rax
ffffffffa037e888: 00
ffffffffa037e889: 48 85 c0 test %rax,%rax
ffffffffa037e88c: 74 52 je ffffffffa037e8e0
<kvm_set_irq+0xa0>
ffffffffa037e88e: 48 8d 95 40 ff ff ff lea -0xc0(%rbp),%rdx
ffffffffa037e895: 31 db xor %ebx,%ebx
ffffffffa037e897: 48 8b 08 mov (%rax),%rcx
ffffffffa037e89a: 83 c3 01 add $0x1,%ebx
ffffffffa037e89d: 0f 18 09 prefetcht0 (%rcx)
ffffffffa037e8a0: 48 8b 48 e0 mov -0x20(%rax),%rcx
ffffffffa037e8a4: 48 89 0a mov %rcx,(%rdx)
ffffffffa037e8a7: 48 8b 48 e8 mov -0x18(%rax),%rcx
ffffffffa037e8ab: 48 89 4a 08 mov %rcx,0x8(%rdx)
ffffffffa037e8af: 48 8b 48 f0 mov -0x10(%rax),%rcx
ffffffffa037e8b3: 48 89 4a 10 mov %rcx,0x10(%rdx)
ffffffffa037e8b7: 48 8b 48 f8 mov -0x8(%rax),%rcx
ffffffffa037e8bb: 48 89 4a 18 mov %rcx,0x18(%rdx)
ffffffffa037e8bf: 48 8b 08 mov (%rax),%rcx
I've re-run markup_oops on the first oops (2.6.37.1) (on the right
module this time, sorry for that), it give me the following:
vmaoffset = 18446744072103215104 ffffffffa041aa62: 41 89
f7 mov %esi,%r15d | %r15 => 0 %esi = 0
ffffffffa041aa65: 41 56 push %r14 |
%r14 => 0
ffffffffa041aa67: 41 55 push %r13
ffffffffa041aa69: 41 89 cd mov %ecx,%r13d |
%ecx = 1 %r13 => 1
ffffffffa041aa6c: 41 54 push %r12
ffffffffa041aa6e: 49 89 fc mov %rdi,%r12 |
%edi = 0 %r12 => 0
ffffffffa041aa71: 55 push %rbp
ffffffffa041aa72: 53 push %rbx
ffffffffa041aa73: 89 d3 mov %edx,%ebx |
%ebx => 1a
ffffffffa041aa75: 48 81 ec 98 00 00 00 sub $0x98,%rsp
ffffffffa041aa7c: 8b 15 00 00 00 00 mov
0x0(%rip),%edx # ffffffffa041aa82 <kvm_set_irq+0x22>
ffffffffa041aa82: 85 d2 test %edx,%edx |
%edx => 0
ffffffffa041aa84: 0f 85 c4 00 00 00 jne ffffffffa041ab4e
<kvm_set_irq+0xee>
*ffffffffa041aa8a: 49 8b 84 24 58 24 00 mov 0x2458(%r12),%rax
| %eax = 0 %r12 = 0 <--- faulting instruction
ffffffffa041aa91: 00
ffffffffa041aa92: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx
ffffffffa041aa98: 73 5e jae ffffffffa041aaf8
<kvm_set_irq+0x98>
ffffffffa041aa9a: 89 db mov %ebx,%ebx
ffffffffa041aa9c: 48 8b 84 d8 30 01 00 mov
0x130(%rax,%rbx,8),%rax
ffffffffa041aaa3: 00
ffffffffa041aaa4: 48 85 c0 test %rax,%rax
ffffffffa041aaa7: 74 4f je ffffffffa041aaf8
<kvm_set_irq+0x98>
ffffffffa041aaa9: 48 89 e2 mov %rsp,%rdx
ffffffffa041aaac: 31 db xor %ebx,%ebx
ffffffffa041aaae: 48 8b 08 mov (%rax),%rcx
ffffffffa041aab1: 83 c3 01 add $0x1,%ebx
ffffffffa041aab4: 0f 18 09 prefetcht0 (%rcx)
ffffffffa041aab7: 48 8b 48 e0 mov -0x20(%rax),%rcx
ffffffffa041aabb: 48 89 0a mov %rcx,(%rdx)
ffffffffa041aabe: 48 8b 48 e8 mov -0x18(%rax),%rcx
ffffffffa041aac2: 48 89 4a 08 mov %rcx,0x8(%rdx)
ffffffffa041aac6: 48 8b 48 f0 mov -0x10(%rax),%rcx
ffffffffa041aaca: 48 89 4a 10 mov %rcx,0x10(%rdx)
ffffffffa041aace: 48 8b 48 f8 mov -0x8(%rax),%rcx
It's appear that the kernel i recompiled (make-pkg) with the debug
options for kvm module, doesn't have the debug!
addr2line give me an "??:0"
I will retent with the good options.
Regards.
--
Jean-Philippe Menil - Pôle réseau Service IRTS
DSI Université de Nantes
jean-philippe.menil@univ-nantes.fr
Tel : 02.53.48.49.27 - Fax : 02.53.48.49.09
^ permalink raw reply
* Re: Bug inkvm_set_irq
From: Jean-Philippe Menil @ 2011-02-28 22:34 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <4D6BBB88.5060709@univ-nantes.fr>
[-- Attachment #1: Type: text/plain, Size: 14653 bytes --]
Hi,
here is another trace with kvm.ko compiled with debug flags.
the bug:
[12099.503414] BUG: unable to handle kernel paging request at
000000000b6635e9
[12099.503462] IP: [<ffffffffa03ee877>] kvm_set_irq+0x37/0x140 [kvm]
[12099.503521] PGD 45d8d2067 PUD 45d58e067 PMD 0
[12099.503560] Oops: 0000 [#1] SMP
[12099.503591] last sysfs file:
/sys/devices/system/cpu/cpu11/cache/index2/shared_cpu_map
[12099.503641] CPU 0
[12099.503648] Modules linked in: netconsole configfs vhost_net macvtap
macvlan tun veth powernow_k8 mperf cpufreq_userspace cpufreq_stats
cpufreq_powersave cpufreq_ondemand freq_table cpufreq_conservative fuse
xt_physdev ip6t_LOG ip6table_filter ip6_tables ipt_LOG xt_multiport
xt_limit xt_tcpudp xt_state iptable_filter ip_tables x_tables
nf_conntrack_tftp nf_conntrack_ftp nf_conntrack_ipv4 nf_defrag_ipv4
8021q bridge stp ext2 mbcache dm_round_robin dm_multipath
nf_conntrack_ipv6 nf_conntrack nf_defrag_ipv6 kvm_amd kvm ipv6 snd_pcm
snd_timer snd soundcore snd_page_alloc shpchp pci_hotplug tpm_tis
i2c_nforce2 tpm i2c_core pcspkr evdev psmouse joydev tpm_bios processor
ghes dcdbas hed button serio_raw thermal_sys xfs exportfs dm_mod sg
sr_mod cdrom usbhid hid usb_storage ses sd_mod enclosure megaraid_sas
ohci_hcd lpfc scsi_transport_fc bnx2 scsi_tgt scsi_mod ehci_hcd [last
unloaded: scsi_wait_scan]
[12099.504277]
[12099.504302] Pid: 1742, comm: kworker/0:2 Not tainted
2.6.37.2-dsiun-110105+ #2 Dell Inc. PowerEdge M605/0K543T
[12099.504373] RIP: 0010:[<ffffffffa03ee877>] [<ffffffffa03ee877>]
kvm_set_irq+0x37/0x140 [kvm]
[12099.504444] RSP: 0018:ffff88045e013d00 EFLAGS: 00010246
[12099.504474] RAX: 000000000b6634c1 RBX: 0000000000000018 RCX:
0000000000000001
[12099.504508] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
ffff880419b600c0
[12099.504541] RBP: ffff88045e013dd0 R08: ffff88045e012000 R09:
0000000000000000
[12099.504575] R10: 0000000000000000 R11: 00000000ffffffff R12:
ffff880419b600c0
[12099.504609] R13: ffff880419b600c0 R14: ffffffffa03efaa0 R15:
0000000000000001
[12099.504643] FS: 00007f3abaa05710(0000) GS:ffff88007f800000(0000)
knlGS:0000000000000000
[12099.504693] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[12099.504724] CR2: 000000000b6635e9 CR3: 000000045e2bc000 CR4:
00000000000006f0
[12099.504757] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[12099.504791] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
0000000000000400
[12099.504825] Process kworker/0:2 (pid: 1742, threadinfo
ffkvm_set_irqff88045e012000, task ffff88045ffb0d60)
[12099.504874] Stack:
[12099.504897] 00000000000119c0 00000000000119c0 00000000000119c0
ffff88045ffb0d60
[12099.504953] ffff88045ffb1010 ffff88045e013fd8 ffff88045ffb1018
ffff88045e012010
[12099.505009] 00000000000119c0 ffff88045e013fd8 00000000000119c0
00000000000119c0
[12099.505065] Call Trace:
[12099.505099] [<ffffffff813818ce>] ? common_interrupt+0xe/0x13
[12099.505145] [<ffffffffa03efaa0>] ? irqfd_inject+0x0/0x50 [kvm]
[12099.505145] [<ffffffffa03efaca>] irqfd_inject+0x2a/0x50 [kvm]
[12099.505145] [<ffffffff8106b7bb>] process_one_work+0x11b/0x450
[12099.505145] [<ffffffff8106bf37>] worker_thread+0x157/0x410
[12099.505145] [<ffffffff8103a569>] ? __wake_up_common+0x59/0x90
[12099.505145] [<ffffffff8106bde0>] ? worker_thread+0x0/0x410
[12099.505145] [<ffffffff8106f996>] kthread+0x96/0xa0
[12099.505145] [<ffffffff81003c64>] kernel_thread_helper+0x4/0x10
[12099.505145] [<ffffffff8106f900>] ? kthread+0x0/0xa0
[12099.505145] [<ffffffff81003c60>] ? kernel_thread_helper+0x0/0x10
[12099.505145] Code: 55 49 89 fd 41 54 53 89 d3 48 81 ec a8 00 00 00 8b
15 a6 75 03 00 89 b5 3c ff ff ff 85 d2 0f 85 d5 00 00 00 49 8b 85 58 24
00 00 <3b> 98 28 01 00 00 73 61 89 db 48 8b 84 d8 30 01 00 00 48 85 c0
[12099.505145] RIP [<ffffffffa03ee877>] kvm_set_irq+0x37/0x140 [kvm]
[12099.505145] RSP <ffff88045e013d00>
[12099.505145] CR2: 000000000b6635e9
markup_oops result:
root@ayrshire:~# cat bug.txt | perl markup_oops.pl -m
/lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko
/boot/vmlinuz-2.6.37.2-dsiun-110105+
vmaoffset = 18446744072103034880 ffffffffa03ee841: 48 89 e5
mov %rsp,%rbp
ffffffffa03ee844: 41 57 push %r15
ffffffffa03ee846: 41 89 cf mov %ecx,%r15d | %r15 =>
1 %ecx = 1
ffffffffa03ee849: 41 56 push %r14 | %r14 =>
ffffffffa03efaa0
ffffffffa03ee84b: 41 55 push %r13
ffffffffa03ee84d: 49 89 fd mov %rdi,%r13 | %edi =
ffff880419b600c0 %r13 => ffff880419b600c0
ffffffffa03ee850: 41 54 push %r12 | %r12 =>
ffff880419b600c0
ffffffffa03ee852: 53 push %rbx
ffffffffa03ee853: 89 d3 mov %edx,%ebx | %ebx => 18
ffffffffa03ee855: 48 81 ec a8 00 00 00 sub $0xa8,%rsp
ffffffffa03ee85c: 8b 15 00 00 00 00 mov 0x0(%rip),%edx #
ffffffffa03ee862 <kvm_set_irq+0x22>
ffffffffa03ee862: 89 b5 3c ff ff ff mov %esi,-0xc4(%rbp) |
%esi = 0
ffffffffa03ee868: 85 d2 test %edx,%edx | %edx => 0
ffffffffa03ee86a: 0f 85 d5 00 00 00 jne ffffffffa03ee945
<kvm_set_irq+0x105>
ffffffffa03ee870: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax |
%eax => b6634c1 %r13 = ffff880419b600c0
*ffffffffa03ee877: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx | %eax
= b6634c1 %ebx = 18 <--- faulting instruction
ffffffffa03ee87d: 73 61 jae ffffffffa03ee8e0
<kvm_set_irq+0xa0>
ffffffffa03ee87f: 89 db mov %ebx,%ebx
ffffffffa03ee881: 48 8b 84 d8 30 01 00 mov 0x130(%rax,%rbx,8),%rax
ffffffffa03ee888: 00
ffffffffa03ee889: 48 85 c0 test %rax,%rax
ffffffffa03ee88c: 74 52 je ffffffffa03ee8e0
<kvm_set_irq+0xa0>
ffffffffa03ee88e: 48 8d 95 40 ff ff ff lea -0xc0(%rbp),%rdx
ffffffffa03ee895: 31 db xor %ebx,%ebx
ffffffffa03ee897: 48 8b 08 mov (%rax),%rcx
ffffffffa03ee89a: 83 c3 01 add $0x1,%ebx
ffffffffa03ee89d: 0f 18 09 prefetcht0 (%rcx)
ffffffffa03ee8a0: 48 8b 48 e0 mov -0x20(%rax),%rcx
ffffffffa03ee8a4: 48 89 0a mov %rcx,(%rdx)
ffffffffa03ee8a7: 48 8b 48 e8 mov -0x18(%rax),%rcx
ffffffffa03ee8ab: 48 89 4a 08 mov %rcx,0x8(%rdx)
ffffffffa03ee8af: 48 8b 48 f0 mov -0x10(%rax),%rcx
ffffffffa03ee8b3: 48 89 4a 10 mov %rcx,0x10(%rdx)
ffffffffa03ee8b7: 48 8b 48 f8 mov -0x8(%rax),%rcx
ffffffffa03ee8bb: 48 89 4a 18 mov %rcx,0x18(%rdx)
ffffffffa03ee8bf: 48 8b 08 mov (%rax),%rcx
The relvant part of objdump for kvm_set_irq:
root@ayrshire:~# objdump -ldS
/lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko > dump.txt
0000000000006840 <kvm_set_irq>:
kvm_set_irq():
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:148
6840: 55 push %rbp
6841: 48 89 e5 mov %rsp,%rbp
6844: 41 57 push %r15
6846: 41 89 cf mov %ecx,%r15d
6849: 41 56 push %r14
684b: 41 55 push %r13
684d: 49 89 fd mov %rdi,%r13
6850: 41 54 push %r12
6852: 53 push %rbx
6853: 89 d3 mov %edx,%ebx
6855: 48 81 ec a8 00 00 00 sub $0xa8,%rsp
trace_kvm_set_irq():
/usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
685c: 8b 15 00 00 00 00 mov 0x0(%rip),%edx #
6862 <kvm_set_irq+0x22>
kvm_set_irq():
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:148
6862: 89 b5 3c ff ff ff mov %esi,-0xc4(%rbp)
trace_kvm_set_irq():
/usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
6868: 85 d2 test %edx,%edx
686a: 0f 85 d5 00 00 00 jne 6945 <kvm_set_irq+0x105>
kvm_set_irq():
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:161
6870: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
6877: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx
687d: 73 61 jae 68e0 <kvm_set_irq+0xa0>
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
687f: 89 db mov %ebx,%ebx
6881: 48 8b 84 d8 30 01 00 mov 0x130(%rax,%rbx,8),%rax
6888: 00
6889: 48 85 c0 test %rax,%rax
688c: 74 52 je 68e0 <kvm_set_irq+0xa0>
688e: 48 8d 95 40 ff ff ff lea -0xc0(%rbp),%rdx
6895: 31 db xor %ebx,%ebx
6897: 48 8b 08 mov (%rax),%rcx
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:164
689a: 83 c3 01 add $0x1,%ebx
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
689d: 0f 18 09 prefetcht0 (%rcx)
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:164
68a0: 48 8b 48 e0 mov -0x20(%rax),%rcx
68a4: 48 89 0a mov %rcx,(%rdx)
68a7: 48 8b 48 e8 mov -0x18(%rax),%rcx
68ab: 48 89 4a 08 mov %rcx,0x8(%rdx)
68af: 48 8b 48 f0 mov -0x10(%rax),%rcx
68b3: 48 89 4a 10 mov %rcx,0x10(%rdx)
68b7: 48 8b 48 f8 mov -0x8(%rax),%rcx
68bb: 48 89 4a 18 mov %rcx,0x18(%rdx)
68bf: 48 8b 08 mov (%rax),%rcx
68c2: 48 89 4a 20 mov %rcx,0x20(%rdx)
68c6: 48 8b 48 08 mov 0x8(%rax),%rcx
68ca: 48 89 4a 28 mov %rcx,0x28(%rdx)
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
68ce: 48 8b 00 mov (%rax),%rax
68d1: 48 83 c2 30 add $0x30,%rdx
68d5: 48 85 c0 test %rax,%rax
68d8: 75 bd jne 6897 <kvm_set_irq+0x57>
68da: eb 06 jmp 68e2 <kvm_set_irq+0xa2>
68dc: 0f 1f 40 00 nopl 0x0(%rax)
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
68e0: 31 db xor %ebx,%ebx
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:169
68e2: 4c 8d b5 40 ff ff ff lea -0xc0(%rbp),%r14
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
68e9: 41 bc ff ff ff ff mov $0xffffffff,%r12d
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
68ef: 85 db test %ebx,%ebx
68f1: 74 3d je 6930 <kvm_set_irq+0xf0>
68f3: 83 eb 01 sub $0x1,%ebx
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:169
68f6: 44 89 f9 mov %r15d,%ecx
68f9: 8b 95 3c ff ff ff mov -0xc4(%rbp),%edx
68ff: 48 63 c3 movslq %ebx,%rax
6902: 4c 89 ee mov %r13,%rsi
6905: 48 8d 04 40 lea (%rax,%rax,2),%rax
6909: 48 c1 e0 04 shl $0x4,%rax
690d: 49 8d 3c 06 lea (%r14,%rax,1),%rdi
6911: ff 94 05 48 ff ff ff callq *-0xb8(%rbp,%rax,1)
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:170
6918: 85 c0 test %eax,%eax
691a: 78 d3 js 68ef <kvm_set_irq+0xaf>
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:173
691c: 45 85 e4 test %r12d,%r12d
691f: ba 00 00 00 00 mov $0x0,%edx
6924: 44 0f 48 e2 cmovs %edx,%r12d
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
6928: 85 db test %ebx,%ebx
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:173
692a: 46 8d 24 20 lea (%rax,%r12,1),%r12d
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
692e: 75 c3 jne 68f3 <kvm_set_irq+0xb3>
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:177
6930: 48 81 c4 a8 00 00 00 add $0xa8,%rsp
6937: 44 89 e0 mov %r12d,%eax
693a: 5b pop %rbx
693b: 41 5c pop %r12
693d: 41 5d pop %r13
693f: 41 5e pop %r14
6941: 41 5f pop %r15
6943: c9 leaveq
6944: c3 retq
trace_kvm_set_irq():
/usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
6945: 4c 8b 25 00 00 00 00 mov 0x0(%rip),%r12 #
694c <kvm_set_irq+0x10c>
694c: 4d 85 e4 test %r12,%r12
694f: 0f 84 1b ff ff ff je 6870 <kvm_set_irq+0x30>
6955: 49 8b 04 24 mov (%r12),%rax
6959: 49 8b 7c 24 08 mov 0x8(%r12),%rdi
695e: 49 83 c4 10 add $0x10,%r12
6962: 8b 8d 3c ff ff ff mov -0xc4(%rbp),%ecx
6968: 44 89 fa mov %r15d,%edx
696b: 89 de mov %ebx,%esi
696d: ff d0 callq *%rax
696f: 49 8b 04 24 mov (%r12),%rax
6973: 48 85 c0 test %rax,%rax
6976: 75 e1 jne 6959 <kvm_set_irq+0x119>
6978: e9 f3 fe ff ff jmpq 6870 <kvm_set_irq+0x30>
kvm_set_irq():
697d: 0f 1f 00 nopl (%rax)
So, if i've read correctly, the offset is 0x6877 ?
root@ayrshire:~# addr2line -e
/lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko 0x6877
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
Is it the correct way to analyse this?
Regards.
[-- Attachment #2: jean-philippe_menil.vcf --]
[-- Type: text/x-vcard, Size: 361 bytes --]
begin:vcard
fn:Jean-Philippe Menil
n:Menil;Jean-Philippe
org;quoted-printable:Universit=C3=A9 de Nantes;IRTS - DSI
adr;quoted-printable:;;2 rue de la Houssini=C3=A8re;Nantes;Loire-Atlantique;44382;France
email;internet:jean-philippe.menil@univ-nantes.fr
title;quoted-printable:Administrateur R=C3=A9seau
url:http://www.cri.univ-nantes.fr
version:2.1
end:vcard
[-- Attachment #3: Type: text/plain, Size: 184 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 1/6] Staging: hv: Unify hyper-v device abstractions
From: Greg KH @ 2011-03-01 2:28 UTC (permalink / raw)
To: KY Srinivasan
Cc: Steven Rostedt, r@home.goodmis.org, gregkh@suse.de,
linux-kernel@vger.kernel.org, devel@linuxdriverproject.org,
virtualization@lists.osdl.org, Haiyang Zhang, Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E048015786@TK5EX14MBXC128.redmond.corp.microsoft.com>
On Sat, Feb 26, 2011 at 05:13:34PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Steven Rostedt [mailto:rostedt@goodmis.org]
> > Sent: Friday, February 25, 2011 10:21 PM
> > To: KY Srinivasan; r@home.goodmis.org
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> > Janssen
> > Subject: Re: [PATCH 1/6] Staging: hv: Unify hyper-v device abstractions
> >
> > Hi!
> >
> > Just an FYI,
> >
> > When sending out patch series, could you send out a [PATCH 0/6] first,
> > and have all other patches a reply to that patch. Both git and quilt
> > have ways to do this.
>
> My mistake; I numbered the patches 1 through 6 as opposed to 0 through 5.
> Sorry for the confusion.
Wait, what?
So, what should I apply here, only patches 2-6? What is patch 1 for
then?
still totally confused,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/6] Staging: hv: Rename vm_device to hyperv_device
From: Greg KH @ 2011-03-01 2:32 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, virtualization, devel
In-Reply-To: <1298685963-30831-1-git-send-email-kys@microsoft.com>
On Fri, Feb 25, 2011 at 06:06:03PM -0800, K. Y. Srinivasan wrote:
> Rename the vm_device abstraction as hyperv_device.
That's a nice name, but it's the first one with a "hyperv_" prefix in
this subsystem. Ok, second, but hyperv_service_context doesn't really
count as it's not really used.
Everything else is named "hv_" here. Which is it going to be, "hv_" or
"hyperv_"?
Either is fine, just be aware of the work involved if you pick
"hyperv_"...
You also rename vm_device_info to hyperv_device_info here, doing more
than one thing in a single patch.
Please don't do that, this should be 2 patches.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
>
> ---
> drivers/staging/hv/blkvsc.c | 4 +-
> drivers/staging/hv/blkvsc_drv.c | 8 ++--
> drivers/staging/hv/channel_mgmt.h | 2 +-
> drivers/staging/hv/netvsc.c | 56 +++++++++++++++++++----------------
> drivers/staging/hv/netvsc.h | 2 +-
> drivers/staging/hv/netvsc_api.h | 12 ++++----
> drivers/staging/hv/netvsc_drv.c | 14 ++++----
> drivers/staging/hv/rndis_filter.c | 18 ++++++------
> drivers/staging/hv/storvsc.c | 36 ++++++++++++-----------
> drivers/staging/hv/storvsc_api.h | 4 +-
> drivers/staging/hv/storvsc_drv.c | 10 +++---
> drivers/staging/hv/vmbus.h | 6 ++--
> drivers/staging/hv/vmbus_api.h | 8 ++--
> drivers/staging/hv/vmbus_drv.c | 50 ++++++++++++++++----------------
> drivers/staging/hv/vmbus_private.h | 12 ++++----
> 15 files changed, 124 insertions(+), 118 deletions(-)
>
> diff --git a/drivers/staging/hv/blkvsc.c b/drivers/staging/hv/blkvsc.c
> index ecface3..47ccec2 100644
> --- a/drivers/staging/hv/blkvsc.c
> +++ b/drivers/staging/hv/blkvsc.c
> @@ -35,8 +35,8 @@ static const struct hv_guid g_blk_device_type = {
> }
> };
>
> -static int blk_vsc_on_device_add(struct vm_device *device,
> - void *additional_info)
> +static int
> +blk_vsc_on_device_add(struct hyperv_device *device, void *additional_info)
Ick, why break the formatting like this? Please keep the return type on
the same line as the function name.
You do that in a number of places, please don't.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/6] Staging: hv: Unify hyper-v device abstractions
From: Greg KH @ 2011-03-01 2:34 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Hank Janssen
In-Reply-To: <1298685930-30806-1-git-send-email-kys@microsoft.com>
On Fri, Feb 25, 2011 at 06:05:30PM -0800, K. Y. Srinivasan wrote:
> Hyper-V drivers have supported two device abstractions.
> This patch implements a single device abstraction.
"This patch" or "This patch series"?
> This simplifies the code and avoids duplication
> of state.
This patch is confusing, you are renaming structures (from hv_ to vm_)
which I didn't think you wanted to do.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
>
> ---
> drivers/staging/hv/blkvsc.c | 17 ++++---
> drivers/staging/hv/blkvsc_drv.c | 14 +++---
> drivers/staging/hv/channel_mgmt.c | 1 +
> drivers/staging/hv/channel_mgmt.h | 2 +-
> drivers/staging/hv/netvsc.c | 55 ++++++++++++-----------
> drivers/staging/hv/netvsc.h | 2 +-
> drivers/staging/hv/netvsc_api.h | 12 +++---
> drivers/staging/hv/netvsc_drv.c | 28 +++++-------
> drivers/staging/hv/rndis_filter.c | 19 ++++----
> drivers/staging/hv/storvsc.c | 37 ++++++++--------
> drivers/staging/hv/storvsc_api.h | 4 +-
> drivers/staging/hv/storvsc_drv.c | 21 ++++-----
> drivers/staging/hv/vmbus.h | 13 +++---
> drivers/staging/hv/vmbus_api.h | 29 ++----------
> drivers/staging/hv/vmbus_drv.c | 84 +++++++++++++++---------------------
> drivers/staging/hv/vmbus_private.h | 12 +++---
> 16 files changed, 157 insertions(+), 193 deletions(-)
>
> diff --git a/drivers/staging/hv/blkvsc.c b/drivers/staging/hv/blkvsc.c
> index 7c8729b..ecface3 100644
> --- a/drivers/staging/hv/blkvsc.c
> +++ b/drivers/staging/hv/blkvsc.c
> @@ -35,7 +35,8 @@ static const struct hv_guid g_blk_device_type = {
> }
> };
>
> -static int blk_vsc_on_device_add(struct hv_device *device, void *additional_info)
> +static int blk_vsc_on_device_add(struct vm_device *device,
> + void *additional_info)
Huh? What was this change for? 80 column issues for function
definitions is not a big deal, if any, and should not be burried in a
patch that claims to do something else.
Still totally confused,
greg k-h
^ permalink raw reply
* Re: [PATCH 3/6] Staging: hv: Cleanup hyperv_device variable names
From: Greg KH @ 2011-03-01 2:44 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Hank Janssen
In-Reply-To: <1298685992-30864-1-git-send-email-kys@microsoft.com>
On Fri, Feb 25, 2011 at 06:06:32PM -0800, K. Y. Srinivasan wrote:
> Cleanup the names of variables that refer to the
> hyperv_device abstraction.
Clean them up to be what? Shorter? Nice? Full of rounded edges so
that when we bump into them in the dark they don't poke us and cause us
to shreak in pain?
>
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Sweet, you cloned yourself, I thought only Alan Cox had achieved that
goal...
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
>
> ---
> drivers/staging/hv/blkvsc_drv.c | 12 ++--
> drivers/staging/hv/netvsc.c | 4 +-
> drivers/staging/hv/netvsc_drv.c | 36 ++++----
> drivers/staging/hv/storvsc_drv.c | 44 +++++-----
> drivers/staging/hv/vmbus_drv.c | 164 +++++++++++++++++++-------------------
> 5 files changed, 130 insertions(+), 130 deletions(-)
>
> diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
> index 58ab0e8..305a665 100644
> --- a/drivers/staging/hv/blkvsc_drv.c
> +++ b/drivers/staging/hv/blkvsc_drv.c
> @@ -95,7 +95,7 @@ struct blkvsc_request {
> /* Per device structure */
> struct block_device_context {
> /* point back to our device context */
> - struct hyperv_device *device_ctx;
> + struct hyperv_device *device_obj;
Hey, I was right, it does have more rounded edges, nicely done.
> -static int netvsc_device_add(struct hyperv_device *device,
> - void *additional_info);
> +static int
> +netvsc_device_add(struct hyperv_device *device, void *additional_info);
Again with the function return value hiding. Please don't.
> --- a/drivers/staging/hv/storvsc_drv.c
> +++ b/drivers/staging/hv/storvsc_drv.c
> @@ -43,7 +43,7 @@ struct host_device_context {
> /* must be 1st field
> * FIXME this is a bug */
> /* point back to our device context */
> - struct hyperv_device *device_ctx;
> + struct hyperv_device *device_obj;
I really don't understand this change at all. "obj" is just as vapid
and clueless as "ctx" is, and it seems very gratuitous to change this.
And I should know, I have made a lot of gratuitous renames in my time in
the kernel...
> static int vmbus_uevent(struct device *device, struct kobj_uevent_env *env)
> {
> - struct hyperv_device *device_ctx = device_to_hyperv_device(device);
> + struct hyperv_device *device_obj = device_to_hyperv_device(device);
> int ret;
>
> DPRINT_INFO(VMBUS_DRV, "generating uevent - VMBUS_DEVICE_CLASS_GUID={"
> "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> "%02x%02x%02x%02x%02x%02x%02x%02x}",
> - device_ctx->class_id.data[3], device_ctx->class_id.data[2],
> - device_ctx->class_id.data[1], device_ctx->class_id.data[0],
> - device_ctx->class_id.data[5], device_ctx->class_id.data[4],
> - device_ctx->class_id.data[7], device_ctx->class_id.data[6],
> - device_ctx->class_id.data[8], device_ctx->class_id.data[9],
> - device_ctx->class_id.data[10],
> - device_ctx->class_id.data[11],
> - device_ctx->class_id.data[12],
> - device_ctx->class_id.data[13],
> - device_ctx->class_id.data[14],
> - device_ctx->class_id.data[15]);
> + device_obj->class_id.data[3], device_obj->class_id.data[2],
> + device_obj->class_id.data[1], device_obj->class_id.data[0],
> + device_obj->class_id.data[5], device_obj->class_id.data[4],
> + device_obj->class_id.data[7], device_obj->class_id.data[6],
> + device_obj->class_id.data[8], device_obj->class_id.data[9],
> + device_obj->class_id.data[10],
> + device_obj->class_id.data[11],
> + device_obj->class_id.data[12],
> + device_obj->class_id.data[13],
> + device_obj->class_id.data[14],
> + device_obj->class_id.data[15]);
After seeing this stuff so many times I'm waiting for a helper function
to be added for it in this subsystem. I'm sure you really don't want to
edit GUID printk-like functions ever again, right?
> static void vmbus_device_release(struct device *device)
> {
> - struct hyperv_device *device_ctx = device_to_hyperv_device(device);
> + struct hyperv_device *device_obj = device_to_hyperv_device(device);
>
> - kfree(device_ctx);
> + kfree(device_obj);
>
> - /* !!DO NOT REFERENCE device_ctx anymore at this point!! */
> + /* !!DO NOT REFERENCE device_obj anymore at this point!! */
> }
I think by virtue of the kfree() right above this comment, it should be
deleted. Especially as it is at the end of the function, making it
pretty much impossible to make any sense here...
Come on, global search-and-replace needs to be done in a sane manner,
other wise you can just send me a vi macro to run on the code, it would
be the same thing in the end (hint, don't do that, only one person has
ever gotten away with doing that in the history of the kernel, in an act
never to be ever repeated again.)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
From: Greg KH @ 2011-03-01 2:52 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Hank Janssen
In-Reply-To: <1298686023-30892-1-git-send-email-kys@microsoft.com>
On Fri, Feb 25, 2011 at 06:07:03PM -0800, K. Y. Srinivasan wrote:
> This patch combines the two driver abstractions into
> a single driver abstraction.
Ah, how sweet. Unfortunatly you don't say "how" you did this.
Nor do you describe _what_ those two driver abstractions were. Are we
talking i2c and usb abstractions? gpio and spi? Driver core and
platform? We want to know exactly what is going on here.
Think of writing something that when you look back, in 3 years, while
staring at a Linux hyperv driver originally written for the 2.6.9
kernel, that somehow never got forward ported and you are tasked with
doing this, that you can just do a simple 'git log drivers/staging/hv/'
and instantly know just from the changelog comments exactly what you
need to do to your driver to clean it up and properly get it to work on
the new 8.2.2 kernel release.
This changelog entry, would require you to go and dig through the guts
of the patch itself, trying to figure out what abstractions you are
talking about, and exactly how they were combined, all the while
wondering _why_ they were combined.
Please, think of your future self, you will thank him in the years to
come by doing this properly. Not to mention making other's lives easier
if you happen to have escaped this dire task by then.
Oh, you have an extra space up there in the subject, please fix it next
time.
> -int blk_vsc_initialize(struct hv_driver *driver)
> +int blk_vsc_initialize(struct driver_context *driver)
"struct driver_context"? Oh please no.
I realize that you are hopefully going to later rename this to something
else, but remember, a few patches back you thought that the "ctx" name
wasn't nice. And here you go resuscitating it from the graveyard of
pointy bits.
And what happens if your future patch is rejected? You are stuck with a
"driver_context" structure in a subsystem? That's a pretty big abuse of
the global namespace, don't you think? It sounds like something that
should go into include/linux/device.h
Please be careful about names, they mean things, even when you think
they don't.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 5/6] Staging: hv: Rename driver_context to hyperv_driver
From: Greg KH @ 2011-03-01 2:56 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Hank Janssen
In-Reply-To: <1298686058-30918-1-git-send-email-kys@microsoft.com>
On Fri, Feb 25, 2011 at 06:07:38PM -0800, K. Y. Srinivasan wrote:
> The title says it all.
No it doesn't. You should have done this on the last patch to keep me
from complaining about that name.
Oh, and the extra space, drop it please.
> --- a/drivers/staging/hv/blkvsc_drv.c
> +++ b/drivers/staging/hv/blkvsc_drv.c
> @@ -116,10 +116,10 @@ struct block_device_context {
> };
>
> /* Per driver */
> -struct blkvsc_driver_context {
> +struct blkvsc_hyperv_driver {
> /* !! These must be the first 2 fields !! */
> /* FIXME this is a bug! */
> - struct driver_context drv_ctx;
> + struct hyperv_driver drv_ctx;
> struct storvsc_driver_object drv_obj;
> };
>
Hey look, that Subject: and changelog body actually lied. It didn't say
it all. In fact, it didn't say enough, or you just renamed a different
structure because it felt nice.
{sigh}
One thing per patch, with full description. I'm getting tired of
repeating Documentation/CodingStyle for this subsystem when patches are
submitted.
It's as if no one even listens to me...
{sniff}
greg k-h
^ permalink raw reply
* Re: [PATCH 6/6] Staging: hv: Cleanup hyperv_driver variable names
From: Greg KH @ 2011-03-01 2:58 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Hank Janssen
In-Reply-To: <1298686078-30945-1-git-send-email-kys@microsoft.com>
On Fri, Feb 25, 2011 at 06:07:58PM -0800, K. Y. Srinivasan wrote:
> The title says it all.
That's a horrible changelog comment. So bad that I would rather see an
empty message than this one. Seriously, it give no description, and
makes us think that the whole patch is obvious, when it really isn't.
What did you change them to?
What did you change them from?
What was your motivation in changing them?
How were you feeling when the names changed?
Ok, maybe not the last one, but you get the idea.
thanks,
greg k-h
^ permalink raw reply
* Re: Bug inkvm_set_irq
From: Michael S. Tsirkin @ 2011-03-01 7:03 UTC (permalink / raw)
To: Jean-Philippe Menil; +Cc: netdev, kvm, virtualization
In-Reply-To: <4D6C22E8.2020007@univ-nantes.fr>
On Mon, Feb 28, 2011 at 11:34:16PM +0100, Jean-Philippe Menil wrote:
> Hi,
>
> here is another trace with kvm.ko compiled with debug flags.
>
> the bug:
> [12099.503414] BUG: unable to handle kernel paging request at
> 000000000b6635e9
> [12099.503462] IP: [<ffffffffa03ee877>] kvm_set_irq+0x37/0x140 [kvm]
> [12099.503521] PGD 45d8d2067 PUD 45d58e067 PMD 0
> [12099.503560] Oops: 0000 [#1] SMP
> [12099.503591] last sysfs file:
> /sys/devices/system/cpu/cpu11/cache/index2/shared_cpu_map
> [12099.503641] CPU 0
> [12099.503648] Modules linked in: netconsole configfs vhost_net
> macvtap macvlan tun veth powernow_k8 mperf cpufreq_userspace
> cpufreq_stats cpufreq_powersave cpufreq_ondemand freq_table
> cpufreq_conservative fuse xt_physdev ip6t_LOG ip6table_filter
> ip6_tables ipt_LOG xt_multiport xt_limit xt_tcpudp xt_state
> iptable_filter ip_tables x_tables nf_conntrack_tftp nf_conntrack_ftp
> nf_conntrack_ipv4 nf_defrag_ipv4 8021q bridge stp ext2 mbcache
> dm_round_robin dm_multipath nf_conntrack_ipv6 nf_conntrack
> nf_defrag_ipv6 kvm_amd kvm ipv6 snd_pcm snd_timer snd soundcore
> snd_page_alloc shpchp pci_hotplug tpm_tis i2c_nforce2 tpm i2c_core
> pcspkr evdev psmouse joydev tpm_bios processor ghes dcdbas hed
> button serio_raw thermal_sys xfs exportfs dm_mod sg sr_mod cdrom
> usbhid hid usb_storage ses sd_mod enclosure megaraid_sas ohci_hcd
> lpfc scsi_transport_fc bnx2 scsi_tgt scsi_mod ehci_hcd [last
> unloaded: scsi_wait_scan]
> [12099.504277]
> [12099.504302] Pid: 1742, comm: kworker/0:2 Not tainted
> 2.6.37.2-dsiun-110105+ #2 Dell Inc. PowerEdge M605/0K543T
> [12099.504373] RIP: 0010:[<ffffffffa03ee877>] [<ffffffffa03ee877>]
> kvm_set_irq+0x37/0x140 [kvm]
> [12099.504444] RSP: 0018:ffff88045e013d00 EFLAGS: 00010246
> [12099.504474] RAX: 000000000b6634c1 RBX: 0000000000000018 RCX:
> 0000000000000001
> [12099.504508] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
> ffff880419b600c0
> [12099.504541] RBP: ffff88045e013dd0 R08: ffff88045e012000 R09:
> 0000000000000000
> [12099.504575] R10: 0000000000000000 R11: 00000000ffffffff R12:
> ffff880419b600c0
> [12099.504609] R13: ffff880419b600c0 R14: ffffffffa03efaa0 R15:
> 0000000000000001
> [12099.504643] FS: 00007f3abaa05710(0000) GS:ffff88007f800000(0000)
> knlGS:0000000000000000
> [12099.504693] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [12099.504724] CR2: 000000000b6635e9 CR3: 000000045e2bc000 CR4:
> 00000000000006f0
> [12099.504757] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [12099.504791] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
> 0000000000000400
> [12099.504825] Process kworker/0:2 (pid: 1742, threadinfo
> ffkvm_set_irqff88045e012000, task ffff88045ffb0d60)
> [12099.504874] Stack:
> [12099.504897] 00000000000119c0 00000000000119c0 00000000000119c0
> ffff88045ffb0d60
> [12099.504953] ffff88045ffb1010 ffff88045e013fd8 ffff88045ffb1018
> ffff88045e012010
> [12099.505009] 00000000000119c0 ffff88045e013fd8 00000000000119c0
> 00000000000119c0
> [12099.505065] Call Trace:
> [12099.505099] [<ffffffff813818ce>] ? common_interrupt+0xe/0x13
> [12099.505145] [<ffffffffa03efaa0>] ? irqfd_inject+0x0/0x50 [kvm]
> [12099.505145] [<ffffffffa03efaca>] irqfd_inject+0x2a/0x50 [kvm]
> [12099.505145] [<ffffffff8106b7bb>] process_one_work+0x11b/0x450
> [12099.505145] [<ffffffff8106bf37>] worker_thread+0x157/0x410
> [12099.505145] [<ffffffff8103a569>] ? __wake_up_common+0x59/0x90
> [12099.505145] [<ffffffff8106bde0>] ? worker_thread+0x0/0x410
> [12099.505145] [<ffffffff8106f996>] kthread+0x96/0xa0
> [12099.505145] [<ffffffff81003c64>] kernel_thread_helper+0x4/0x10
> [12099.505145] [<ffffffff8106f900>] ? kthread+0x0/0xa0
> [12099.505145] [<ffffffff81003c60>] ? kernel_thread_helper+0x0/0x10
> [12099.505145] Code: 55 49 89 fd 41 54 53 89 d3 48 81 ec a8 00 00 00
> 8b 15 a6 75 03 00 89 b5 3c ff ff ff 85 d2 0f 85 d5 00 00 00 49 8b 85
> 58 24 00 00 <3b> 98 28 01 00 00 73 61 89 db 48 8b 84 d8 30 01 00 00
> 48 85 c0
> [12099.505145] RIP [<ffffffffa03ee877>] kvm_set_irq+0x37/0x140 [kvm]
> [12099.505145] RSP <ffff88045e013d00>
> [12099.505145] CR2: 000000000b6635e9
>
>
> markup_oops result:
>
> root@ayrshire:~# cat bug.txt | perl markup_oops.pl -m
> /lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko
> /boot/vmlinuz-2.6.37.2-dsiun-110105+
> vmaoffset = 18446744072103034880 ffffffffa03ee841: 48 89 e5 mov
> %rsp,%rbp
> ffffffffa03ee844: 41 57 push %r15
> ffffffffa03ee846: 41 89 cf mov %ecx,%r15d | %r15
> => 1 %ecx = 1
> ffffffffa03ee849: 41 56 push %r14 | %r14
> => ffffffffa03efaa0
> ffffffffa03ee84b: 41 55 push %r13
> ffffffffa03ee84d: 49 89 fd mov %rdi,%r13 | %edi
> = ffff880419b600c0 %r13 => ffff880419b600c0
> ffffffffa03ee850: 41 54 push %r12 | %r12
> => ffff880419b600c0
> ffffffffa03ee852: 53 push %rbx
> ffffffffa03ee853: 89 d3 mov %edx,%ebx | %ebx => 18
> ffffffffa03ee855: 48 81 ec a8 00 00 00 sub $0xa8,%rsp
> ffffffffa03ee85c: 8b 15 00 00 00 00 mov 0x0(%rip),%edx
> # ffffffffa03ee862 <kvm_set_irq+0x22>
> ffffffffa03ee862: 89 b5 3c ff ff ff mov %esi,-0xc4(%rbp) |
> %esi = 0
> ffffffffa03ee868: 85 d2 test %edx,%edx | %edx => 0
> ffffffffa03ee86a: 0f 85 d5 00 00 00 jne ffffffffa03ee945
> <kvm_set_irq+0x105>
> ffffffffa03ee870: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax |
> %eax => b6634c1 %r13 = ffff880419b600c0
> *ffffffffa03ee877: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx |
> %eax = b6634c1 %ebx = 18 <--- faulting instruction
> ffffffffa03ee87d: 73 61 jae ffffffffa03ee8e0
> <kvm_set_irq+0xa0>
> ffffffffa03ee87f: 89 db mov %ebx,%ebx
> ffffffffa03ee881: 48 8b 84 d8 30 01 00 mov 0x130(%rax,%rbx,8),%rax
> ffffffffa03ee888: 00
> ffffffffa03ee889: 48 85 c0 test %rax,%rax
> ffffffffa03ee88c: 74 52 je ffffffffa03ee8e0
> <kvm_set_irq+0xa0>
> ffffffffa03ee88e: 48 8d 95 40 ff ff ff lea -0xc0(%rbp),%rdx
> ffffffffa03ee895: 31 db xor %ebx,%ebx
> ffffffffa03ee897: 48 8b 08 mov (%rax),%rcx
> ffffffffa03ee89a: 83 c3 01 add $0x1,%ebx
> ffffffffa03ee89d: 0f 18 09 prefetcht0 (%rcx)
> ffffffffa03ee8a0: 48 8b 48 e0 mov -0x20(%rax),%rcx
> ffffffffa03ee8a4: 48 89 0a mov %rcx,(%rdx)
> ffffffffa03ee8a7: 48 8b 48 e8 mov -0x18(%rax),%rcx
> ffffffffa03ee8ab: 48 89 4a 08 mov %rcx,0x8(%rdx)
> ffffffffa03ee8af: 48 8b 48 f0 mov -0x10(%rax),%rcx
> ffffffffa03ee8b3: 48 89 4a 10 mov %rcx,0x10(%rdx)
> ffffffffa03ee8b7: 48 8b 48 f8 mov -0x8(%rax),%rcx
> ffffffffa03ee8bb: 48 89 4a 18 mov %rcx,0x18(%rdx)
> ffffffffa03ee8bf: 48 8b 08 mov (%rax),%rcx
>
> The relvant part of objdump for kvm_set_irq:
> root@ayrshire:~# objdump -ldS
> /lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko >
> dump.txt
>
> 0000000000006840 <kvm_set_irq>:
> kvm_set_irq():
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:148
> 6840: 55 push %rbp
> 6841: 48 89 e5 mov %rsp,%rbp
> 6844: 41 57 push %r15
> 6846: 41 89 cf mov %ecx,%r15d
> 6849: 41 56 push %r14
> 684b: 41 55 push %r13
> 684d: 49 89 fd mov %rdi,%r13
> 6850: 41 54 push %r12
> 6852: 53 push %rbx
> 6853: 89 d3 mov %edx,%ebx
> 6855: 48 81 ec a8 00 00 00 sub $0xa8,%rsp
> trace_kvm_set_irq():
> /usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
> 685c: 8b 15 00 00 00 00 mov 0x0(%rip),%edx
> # 6862 <kvm_set_irq+0x22>
> kvm_set_irq():
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:148
> 6862: 89 b5 3c ff ff ff mov %esi,-0xc4(%rbp)
> trace_kvm_set_irq():
> /usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
> 6868: 85 d2 test %edx,%edx
> 686a: 0f 85 d5 00 00 00 jne 6945 <kvm_set_irq+0x105>
> kvm_set_irq():
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:161
> 6870: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
> 6877: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx
> 687d: 73 61 jae 68e0 <kvm_set_irq+0xa0>
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
> 687f: 89 db mov %ebx,%ebx
> 6881: 48 8b 84 d8 30 01 00 mov 0x130(%rax,%rbx,8),%rax
> 6888: 00
> 6889: 48 85 c0 test %rax,%rax
> 688c: 74 52 je 68e0 <kvm_set_irq+0xa0>
> 688e: 48 8d 95 40 ff ff ff lea -0xc0(%rbp),%rdx
> 6895: 31 db xor %ebx,%ebx
> 6897: 48 8b 08 mov (%rax),%rcx
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:164
> 689a: 83 c3 01 add $0x1,%ebx
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
> 689d: 0f 18 09 prefetcht0 (%rcx)
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:164
> 68a0: 48 8b 48 e0 mov -0x20(%rax),%rcx
> 68a4: 48 89 0a mov %rcx,(%rdx)
> 68a7: 48 8b 48 e8 mov -0x18(%rax),%rcx
> 68ab: 48 89 4a 08 mov %rcx,0x8(%rdx)
> 68af: 48 8b 48 f0 mov -0x10(%rax),%rcx
> 68b3: 48 89 4a 10 mov %rcx,0x10(%rdx)
> 68b7: 48 8b 48 f8 mov -0x8(%rax),%rcx
> 68bb: 48 89 4a 18 mov %rcx,0x18(%rdx)
> 68bf: 48 8b 08 mov (%rax),%rcx
> 68c2: 48 89 4a 20 mov %rcx,0x20(%rdx)
> 68c6: 48 8b 48 08 mov 0x8(%rax),%rcx
> 68ca: 48 89 4a 28 mov %rcx,0x28(%rdx)
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
> 68ce: 48 8b 00 mov (%rax),%rax
> 68d1: 48 83 c2 30 add $0x30,%rdx
> 68d5: 48 85 c0 test %rax,%rax
> 68d8: 75 bd jne 6897 <kvm_set_irq+0x57>
> 68da: eb 06 jmp 68e2 <kvm_set_irq+0xa2>
> 68dc: 0f 1f 40 00 nopl 0x0(%rax)
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
> 68e0: 31 db xor %ebx,%ebx
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:169
> 68e2: 4c 8d b5 40 ff ff ff lea -0xc0(%rbp),%r14
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
> 68e9: 41 bc ff ff ff ff mov $0xffffffff,%r12d
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
> 68ef: 85 db test %ebx,%ebx
> 68f1: 74 3d je 6930 <kvm_set_irq+0xf0>
> 68f3: 83 eb 01 sub $0x1,%ebx
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:169
> 68f6: 44 89 f9 mov %r15d,%ecx
> 68f9: 8b 95 3c ff ff ff mov -0xc4(%rbp),%edx
> 68ff: 48 63 c3 movslq %ebx,%rax
> 6902: 4c 89 ee mov %r13,%rsi
> 6905: 48 8d 04 40 lea (%rax,%rax,2),%rax
> 6909: 48 c1 e0 04 shl $0x4,%rax
> 690d: 49 8d 3c 06 lea (%r14,%rax,1),%rdi
> 6911: ff 94 05 48 ff ff ff callq *-0xb8(%rbp,%rax,1)
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:170
> 6918: 85 c0 test %eax,%eax
> 691a: 78 d3 js 68ef <kvm_set_irq+0xaf>
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:173
> 691c: 45 85 e4 test %r12d,%r12d
> 691f: ba 00 00 00 00 mov $0x0,%edx
> 6924: 44 0f 48 e2 cmovs %edx,%r12d
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
> 6928: 85 db test %ebx,%ebx
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:173
> 692a: 46 8d 24 20 lea (%rax,%r12,1),%r12d
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
> 692e: 75 c3 jne 68f3 <kvm_set_irq+0xb3>
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:177
> 6930: 48 81 c4 a8 00 00 00 add $0xa8,%rsp
> 6937: 44 89 e0 mov %r12d,%eax
> 693a: 5b pop %rbx
> 693b: 41 5c pop %r12
> 693d: 41 5d pop %r13
> 693f: 41 5e pop %r14
> 6941: 41 5f pop %r15
> 6943: c9 leaveq
> 6944: c3 retq
> trace_kvm_set_irq():
> /usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
> 6945: 4c 8b 25 00 00 00 00 mov 0x0(%rip),%r12
> # 694c <kvm_set_irq+0x10c>
> 694c: 4d 85 e4 test %r12,%r12
> 694f: 0f 84 1b ff ff ff je 6870 <kvm_set_irq+0x30>
> 6955: 49 8b 04 24 mov (%r12),%rax
> 6959: 49 8b 7c 24 08 mov 0x8(%r12),%rdi
> 695e: 49 83 c4 10 add $0x10,%r12
> 6962: 8b 8d 3c ff ff ff mov -0xc4(%rbp),%ecx
> 6968: 44 89 fa mov %r15d,%edx
> 696b: 89 de mov %ebx,%esi
> 696d: ff d0 callq *%rax
> 696f: 49 8b 04 24 mov (%r12),%rax
> 6973: 48 85 c0 test %rax,%rax
> 6976: 75 e1 jne 6959 <kvm_set_irq+0x119>
> 6978: e9 f3 fe ff ff jmpq 6870 <kvm_set_irq+0x30>
> kvm_set_irq():
> 697d: 0f 1f 00 nopl (%rax)
>
> So, if i've read correctly, the offset is 0x6877 ?
>
> root@ayrshire:~# addr2line -e
> /lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko
> 0x6877
> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
>
>
> Is it the correct way to analyse this?
>
> Regards.
Yes. So we have:
irq_rt = rcu_dereference(kvm->irq_routing);
> ffffffffa03ee870: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax |
> %eax => b6634c1 %r13 = ffff880419b600c0
if (irq < irq_rt->nr_rt_entries)
> *ffffffffa03ee877: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx |
> %eax = b6634c1 %ebx = 18 <--- faulting instruction
The problem then is that while the kvm pointer is
ffff880419b600c0 which looks sane,
the value we read from kvm->irq_routing is b6634c1 which
does not make sense. When we dereference that, kaboom.
Is the kvm pointer wrong or the memory corrupted?
Try printing the kvm pointer during
initialization, e.g. in kvm_vm_ioctl_create_vcpu,
then and compare to markup_oops.
--
MST
^ permalink raw reply
* Re: Bug inkvm_set_irq
From: Jean-Philippe Menil @ 2011-03-01 14:39 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20110301070333.GA5972@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 20060 bytes --]
Le 01/03/2011 08:03, Michael S. Tsirkin a écrit :
> On Mon, Feb 28, 2011 at 11:34:16PM +0100, Jean-Philippe Menil wrote:
>> Hi,
>>
>> here is another trace with kvm.ko compiled with debug flags.
>>
>> the bug:
>> [12099.503414] BUG: unable to handle kernel paging request at
>> 000000000b6635e9
>> [12099.503462] IP: [<ffffffffa03ee877>] kvm_set_irq+0x37/0x140 [kvm]
>> [12099.503521] PGD 45d8d2067 PUD 45d58e067 PMD 0
>> [12099.503560] Oops: 0000 [#1] SMP
>> [12099.503591] last sysfs file:
>> /sys/devices/system/cpu/cpu11/cache/index2/shared_cpu_map
>> [12099.503641] CPU 0
>> [12099.503648] Modules linked in: netconsole configfs vhost_net
>> macvtap macvlan tun veth powernow_k8 mperf cpufreq_userspace
>> cpufreq_stats cpufreq_powersave cpufreq_ondemand freq_table
>> cpufreq_conservative fuse xt_physdev ip6t_LOG ip6table_filter
>> ip6_tables ipt_LOG xt_multiport xt_limit xt_tcpudp xt_state
>> iptable_filter ip_tables x_tables nf_conntrack_tftp nf_conntrack_ftp
>> nf_conntrack_ipv4 nf_defrag_ipv4 8021q bridge stp ext2 mbcache
>> dm_round_robin dm_multipath nf_conntrack_ipv6 nf_conntrack
>> nf_defrag_ipv6 kvm_amd kvm ipv6 snd_pcm snd_timer snd soundcore
>> snd_page_alloc shpchp pci_hotplug tpm_tis i2c_nforce2 tpm i2c_core
>> pcspkr evdev psmouse joydev tpm_bios processor ghes dcdbas hed
>> button serio_raw thermal_sys xfs exportfs dm_mod sg sr_mod cdrom
>> usbhid hid usb_storage ses sd_mod enclosure megaraid_sas ohci_hcd
>> lpfc scsi_transport_fc bnx2 scsi_tgt scsi_mod ehci_hcd [last
>> unloaded: scsi_wait_scan]
>> [12099.504277]
>> [12099.504302] Pid: 1742, comm: kworker/0:2 Not tainted
>> 2.6.37.2-dsiun-110105+ #2 Dell Inc. PowerEdge M605/0K543T
>> [12099.504373] RIP: 0010:[<ffffffffa03ee877>] [<ffffffffa03ee877>]
>> kvm_set_irq+0x37/0x140 [kvm]
>> [12099.504444] RSP: 0018:ffff88045e013d00 EFLAGS: 00010246
>> [12099.504474] RAX: 000000000b6634c1 RBX: 0000000000000018 RCX:
>> 0000000000000001
>> [12099.504508] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
>> ffff880419b600c0
>> [12099.504541] RBP: ffff88045e013dd0 R08: ffff88045e012000 R09:
>> 0000000000000000
>> [12099.504575] R10: 0000000000000000 R11: 00000000ffffffff R12:
>> ffff880419b600c0
>> [12099.504609] R13: ffff880419b600c0 R14: ffffffffa03efaa0 R15:
>> 0000000000000001
>> [12099.504643] FS: 00007f3abaa05710(0000) GS:ffff88007f800000(0000)
>> knlGS:0000000000000000
>> [12099.504693] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>> [12099.504724] CR2: 000000000b6635e9 CR3: 000000045e2bc000 CR4:
>> 00000000000006f0
>> [12099.504757] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
>> 0000000000000000
>> [12099.504791] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
>> 0000000000000400
>> [12099.504825] Process kworker/0:2 (pid: 1742, threadinfo
>> ffkvm_set_irqff88045e012000, task ffff88045ffb0d60)
>> [12099.504874] Stack:
>> [12099.504897] 00000000000119c0 00000000000119c0 00000000000119c0
>> ffff88045ffb0d60
>> [12099.504953] ffff88045ffb1010 ffff88045e013fd8 ffff88045ffb1018
>> ffff88045e012010
>> [12099.505009] 00000000000119c0 ffff88045e013fd8 00000000000119c0
>> 00000000000119c0
>> [12099.505065] Call Trace:
>> [12099.505099] [<ffffffff813818ce>] ? common_interrupt+0xe/0x13
>> [12099.505145] [<ffffffffa03efaa0>] ? irqfd_inject+0x0/0x50 [kvm]
>> [12099.505145] [<ffffffffa03efaca>] irqfd_inject+0x2a/0x50 [kvm]
>> [12099.505145] [<ffffffff8106b7bb>] process_one_work+0x11b/0x450
>> [12099.505145] [<ffffffff8106bf37>] worker_thread+0x157/0x410
>> [12099.505145] [<ffffffff8103a569>] ? __wake_up_common+0x59/0x90
>> [12099.505145] [<ffffffff8106bde0>] ? worker_thread+0x0/0x410
>> [12099.505145] [<ffffffff8106f996>] kthread+0x96/0xa0
>> [12099.505145] [<ffffffff81003c64>] kernel_thread_helper+0x4/0x10
>> [12099.505145] [<ffffffff8106f900>] ? kthread+0x0/0xa0
>> [12099.505145] [<ffffffff81003c60>] ? kernel_thread_helper+0x0/0x10
>> [12099.505145] Code: 55 49 89 fd 41 54 53 89 d3 48 81 ec a8 00 00 00
>> 8b 15 a6 75 03 00 89 b5 3c ff ff ff 85 d2 0f 85 d5 00 00 00 49 8b 85
>> 58 24 00 00<3b> 98 28 01 00 00 73 61 89 db 48 8b 84 d8 30 01 00 00
>> 48 85 c0
>> [12099.505145] RIP [<ffffffffa03ee877>] kvm_set_irq+0x37/0x140 [kvm]
>> [12099.505145] RSP<ffff88045e013d00>
>> [12099.505145] CR2: 000000000b6635e9
>>
>>
>> markup_oops result:
>>
>> root@ayrshire:~# cat bug.txt | perl markup_oops.pl -m
>> /lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko
>> /boot/vmlinuz-2.6.37.2-dsiun-110105+
>> vmaoffset = 18446744072103034880 ffffffffa03ee841: 48 89 e5 mov
>> %rsp,%rbp
>> ffffffffa03ee844: 41 57 push %r15
>> ffffffffa03ee846: 41 89 cf mov %ecx,%r15d | %r15
>> => 1 %ecx = 1
>> ffffffffa03ee849: 41 56 push %r14 | %r14
>> => ffffffffa03efaa0
>> ffffffffa03ee84b: 41 55 push %r13
>> ffffffffa03ee84d: 49 89 fd mov %rdi,%r13 | %edi
>> = ffff880419b600c0 %r13 => ffff880419b600c0
>> ffffffffa03ee850: 41 54 push %r12 | %r12
>> => ffff880419b600c0
>> ffffffffa03ee852: 53 push %rbx
>> ffffffffa03ee853: 89 d3 mov %edx,%ebx | %ebx => 18
>> ffffffffa03ee855: 48 81 ec a8 00 00 00 sub $0xa8,%rsp
>> ffffffffa03ee85c: 8b 15 00 00 00 00 mov 0x0(%rip),%edx
>> # ffffffffa03ee862<kvm_set_irq+0x22>
>> ffffffffa03ee862: 89 b5 3c ff ff ff mov %esi,-0xc4(%rbp) |
>> %esi = 0
>> ffffffffa03ee868: 85 d2 test %edx,%edx | %edx => 0
>> ffffffffa03ee86a: 0f 85 d5 00 00 00 jne ffffffffa03ee945
>> <kvm_set_irq+0x105>
>> ffffffffa03ee870: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax |
>> %eax => b6634c1 %r13 = ffff880419b600c0
>> *ffffffffa03ee877: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx |
>> %eax = b6634c1 %ebx = 18<--- faulting instruction
>> ffffffffa03ee87d: 73 61 jae ffffffffa03ee8e0
>> <kvm_set_irq+0xa0>
>> ffffffffa03ee87f: 89 db mov %ebx,%ebx
>> ffffffffa03ee881: 48 8b 84 d8 30 01 00 mov 0x130(%rax,%rbx,8),%rax
>> ffffffffa03ee888: 00
>> ffffffffa03ee889: 48 85 c0 test %rax,%rax
>> ffffffffa03ee88c: 74 52 je ffffffffa03ee8e0
>> <kvm_set_irq+0xa0>
>> ffffffffa03ee88e: 48 8d 95 40 ff ff ff lea -0xc0(%rbp),%rdx
>> ffffffffa03ee895: 31 db xor %ebx,%ebx
>> ffffffffa03ee897: 48 8b 08 mov (%rax),%rcx
>> ffffffffa03ee89a: 83 c3 01 add $0x1,%ebx
>> ffffffffa03ee89d: 0f 18 09 prefetcht0 (%rcx)
>> ffffffffa03ee8a0: 48 8b 48 e0 mov -0x20(%rax),%rcx
>> ffffffffa03ee8a4: 48 89 0a mov %rcx,(%rdx)
>> ffffffffa03ee8a7: 48 8b 48 e8 mov -0x18(%rax),%rcx
>> ffffffffa03ee8ab: 48 89 4a 08 mov %rcx,0x8(%rdx)
>> ffffffffa03ee8af: 48 8b 48 f0 mov -0x10(%rax),%rcx
>> ffffffffa03ee8b3: 48 89 4a 10 mov %rcx,0x10(%rdx)
>> ffffffffa03ee8b7: 48 8b 48 f8 mov -0x8(%rax),%rcx
>> ffffffffa03ee8bb: 48 89 4a 18 mov %rcx,0x18(%rdx)
>> ffffffffa03ee8bf: 48 8b 08 mov (%rax),%rcx
>>
>> The relvant part of objdump for kvm_set_irq:
>> root@ayrshire:~# objdump -ldS
>> /lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko>
>> dump.txt
>>
>> 0000000000006840<kvm_set_irq>:
>> kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:148
>> 6840: 55 push %rbp
>> 6841: 48 89 e5 mov %rsp,%rbp
>> 6844: 41 57 push %r15
>> 6846: 41 89 cf mov %ecx,%r15d
>> 6849: 41 56 push %r14
>> 684b: 41 55 push %r13
>> 684d: 49 89 fd mov %rdi,%r13
>> 6850: 41 54 push %r12
>> 6852: 53 push %rbx
>> 6853: 89 d3 mov %edx,%ebx
>> 6855: 48 81 ec a8 00 00 00 sub $0xa8,%rsp
>> trace_kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
>> 685c: 8b 15 00 00 00 00 mov 0x0(%rip),%edx
>> # 6862<kvm_set_irq+0x22>
>> kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:148
>> 6862: 89 b5 3c ff ff ff mov %esi,-0xc4(%rbp)
>> trace_kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
>> 6868: 85 d2 test %edx,%edx
>> 686a: 0f 85 d5 00 00 00 jne 6945<kvm_set_irq+0x105>
>> kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:161
>> 6870: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
>> 6877: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx
>> 687d: 73 61 jae 68e0<kvm_set_irq+0xa0>
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
>> 687f: 89 db mov %ebx,%ebx
>> 6881: 48 8b 84 d8 30 01 00 mov 0x130(%rax,%rbx,8),%rax
>> 6888: 00
>> 6889: 48 85 c0 test %rax,%rax
>> 688c: 74 52 je 68e0<kvm_set_irq+0xa0>
>> 688e: 48 8d 95 40 ff ff ff lea -0xc0(%rbp),%rdx
>> 6895: 31 db xor %ebx,%ebx
>> 6897: 48 8b 08 mov (%rax),%rcx
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:164
>> 689a: 83 c3 01 add $0x1,%ebx
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
>> 689d: 0f 18 09 prefetcht0 (%rcx)
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:164
>> 68a0: 48 8b 48 e0 mov -0x20(%rax),%rcx
>> 68a4: 48 89 0a mov %rcx,(%rdx)
>> 68a7: 48 8b 48 e8 mov -0x18(%rax),%rcx
>> 68ab: 48 89 4a 08 mov %rcx,0x8(%rdx)
>> 68af: 48 8b 48 f0 mov -0x10(%rax),%rcx
>> 68b3: 48 89 4a 10 mov %rcx,0x10(%rdx)
>> 68b7: 48 8b 48 f8 mov -0x8(%rax),%rcx
>> 68bb: 48 89 4a 18 mov %rcx,0x18(%rdx)
>> 68bf: 48 8b 08 mov (%rax),%rcx
>> 68c2: 48 89 4a 20 mov %rcx,0x20(%rdx)
>> 68c6: 48 8b 48 08 mov 0x8(%rax),%rcx
>> 68ca: 48 89 4a 28 mov %rcx,0x28(%rdx)
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
>> 68ce: 48 8b 00 mov (%rax),%rax
>> 68d1: 48 83 c2 30 add $0x30,%rdx
>> 68d5: 48 85 c0 test %rax,%rax
>> 68d8: 75 bd jne 6897<kvm_set_irq+0x57>
>> 68da: eb 06 jmp 68e2<kvm_set_irq+0xa2>
>> 68dc: 0f 1f 40 00 nopl 0x0(%rax)
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
>> 68e0: 31 db xor %ebx,%ebx
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:169
>> 68e2: 4c 8d b5 40 ff ff ff lea -0xc0(%rbp),%r14
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
>> 68e9: 41 bc ff ff ff ff mov $0xffffffff,%r12d
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
>> 68ef: 85 db test %ebx,%ebx
>> 68f1: 74 3d je 6930<kvm_set_irq+0xf0>
>> 68f3: 83 eb 01 sub $0x1,%ebx
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:169
>> 68f6: 44 89 f9 mov %r15d,%ecx
>> 68f9: 8b 95 3c ff ff ff mov -0xc4(%rbp),%edx
>> 68ff: 48 63 c3 movslq %ebx,%rax
>> 6902: 4c 89 ee mov %r13,%rsi
>> 6905: 48 8d 04 40 lea (%rax,%rax,2),%rax
>> 6909: 48 c1 e0 04 shl $0x4,%rax
>> 690d: 49 8d 3c 06 lea (%r14,%rax,1),%rdi
>> 6911: ff 94 05 48 ff ff ff callq *-0xb8(%rbp,%rax,1)
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:170
>> 6918: 85 c0 test %eax,%eax
>> 691a: 78 d3 js 68ef<kvm_set_irq+0xaf>
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:173
>> 691c: 45 85 e4 test %r12d,%r12d
>> 691f: ba 00 00 00 00 mov $0x0,%edx
>> 6924: 44 0f 48 e2 cmovs %edx,%r12d
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
>> 6928: 85 db test %ebx,%ebx
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:173
>> 692a: 46 8d 24 20 lea (%rax,%r12,1),%r12d
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
>> 692e: 75 c3 jne 68f3<kvm_set_irq+0xb3>
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:177
>> 6930: 48 81 c4 a8 00 00 00 add $0xa8,%rsp
>> 6937: 44 89 e0 mov %r12d,%eax
>> 693a: 5b pop %rbx
>> 693b: 41 5c pop %r12
>> 693d: 41 5d pop %r13
>> 693f: 41 5e pop %r14
>> 6941: 41 5f pop %r15
>> 6943: c9 leaveq
>> 6944: c3 retq
>> trace_kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
>> 6945: 4c 8b 25 00 00 00 00 mov 0x0(%rip),%r12
>> # 694c<kvm_set_irq+0x10c>
>> 694c: 4d 85 e4 test %r12,%r12
>> 694f: 0f 84 1b ff ff ff je 6870<kvm_set_irq+0x30>
>> 6955: 49 8b 04 24 mov (%r12),%rax
>> 6959: 49 8b 7c 24 08 mov 0x8(%r12),%rdi
>> 695e: 49 83 c4 10 add $0x10,%r12
>> 6962: 8b 8d 3c ff ff ff mov -0xc4(%rbp),%ecx
>> 6968: 44 89 fa mov %r15d,%edx
>> 696b: 89 de mov %ebx,%esi
>> 696d: ff d0 callq *%rax
>> 696f: 49 8b 04 24 mov (%r12),%rax
>> 6973: 48 85 c0 test %rax,%rax
>> 6976: 75 e1 jne 6959<kvm_set_irq+0x119>
>> 6978: e9 f3 fe ff ff jmpq 6870<kvm_set_irq+0x30>
>> kvm_set_irq():
>> 697d: 0f 1f 00 nopl (%rax)
>>
>> So, if i've read correctly, the offset is 0x6877 ?
>>
>> root@ayrshire:~# addr2line -e
>> /lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko
>> 0x6877
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
>>
>>
>> Is it the correct way to analyse this?
>>
>> Regards.
>
> Yes. So we have:
>
> irq_rt = rcu_dereference(kvm->irq_routing);
>
>> ffffffffa03ee870: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax |
>> %eax => b6634c1 %r13 = ffff880419b600c0
>
> if (irq< irq_rt->nr_rt_entries)
>
>> *ffffffffa03ee877: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx |
>> %eax = b6634c1 %ebx = 18<--- faulting instruction
>
> The problem then is that while the kvm pointer is
> ffff880419b600c0 which looks sane,
> the value we read from kvm->irq_routing is b6634c1 which
> does not make sense. When we dereference that, kaboom.
>
> Is the kvm pointer wrong or the memory corrupted?
> Try printing the kvm pointer during
> initialization, e.g. in kvm_vm_ioctl_create_vcpu,
> then and compare to markup_oops.
>
>
Hi,
so this time the bug is:
[17882.612303] BUG: unable to handle kernel paging request at
0000000000002458
[17882.612342] IP: [<ffffffffa03898a0>] kvm_set_irq+0x30/0x140 [kvm]
markup_oops give me this:
root@ayrshire:~# cat bug-0103.txt | perl markup_oops.pl -m
/lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko
/boot/vmlinuz-2.6.37.2-dsiun-110105+
vmaoffset = 18446744072102621184 ffffffffa0389871: 48 89 e5
mov %rsp,%rbp
ffffffffa0389874: 41 57 push %r15
ffffffffa0389876: 41 89 cf mov %ecx,%r15d | %r15 =>
1 %ecx = 1
ffffffffa0389879: 41 56 push %r14 | %r14 =>
ffffffffa038aad0
ffffffffa038987b: 41 55 push %r13
ffffffffa038987d: 49 89 fd mov %rdi,%r13 | %edi = 0
%r13 => 0
ffffffffa0389880: 41 54 push %r12 | %r12 => 0
ffffffffa0389882: 53 push %rbx
ffffffffa0389883: 89 d3 mov %edx,%ebx | %ebx => 1a
ffffffffa0389885: 48 81 ec a8 00 00 00 sub $0xa8,%rsp
ffffffffa038988c: 8b 15 00 00 00 00 mov 0x0(%rip),%edx #
ffffffffa0389892 <kvm_set_irq+0x22>
ffffffffa0389892: 89 b5 3c ff ff ff mov %esi,-0xc4(%rbp) |
%esi = 0
ffffffffa0389898: 85 d2 test %edx,%edx | %edx => 0
ffffffffa038989a: 0f 85 d5 00 00 00 jne ffffffffa0389975
<kvm_set_irq+0x105>
*ffffffffa03898a0: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax |
%eax = 0 %r13 = 0 <--- faulting instruction
ffffffffa03898a7: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx
ffffffffa03898ad: 73 61 jae ffffffffa0389910
<kvm_set_irq+0xa0>
ffffffffa03898af: 89 db mov %ebx,%ebx
ffffffffa03898b1: 48 8b 84 d8 30 01 00 mov 0x130(%rax,%rbx,8),%rax
ffffffffa03898b8: 00
ffffffffa03898b9: 48 85 c0 test %rax,%rax
ffffffffa03898bc: 74 52 je ffffffffa0389910
<kvm_set_irq+0xa0>
ffffffffa03898be: 48 8d 95 40 ff ff ff lea -0xc0(%rbp),%rdx
ffffffffa03898c5: 31 db xor %ebx,%ebx
ffffffffa03898c7: 48 8b 08 mov (%rax),%rcx
ffffffffa03898ca: 83 c3 01 add $0x1,%ebx
ffffffffa03898cd: 0f 18 09 prefetcht0 (%rcx)
ffffffffa03898d0: 48 8b 48 e0 mov -0x20(%rax),%rcx
ffffffffa03898d4: 48 89 0a mov %rcx,(%rdx)
ffffffffa03898d7: 48 8b 48 e8 mov -0x18(%rax),%rcx
ffffffffa03898db: 48 89 4a 08 mov %rcx,0x8(%rdx)
ffffffffa03898df: 48 8b 48 f0 mov -0x10(%rax),%rcx
ffffffffa03898e3: 48 89 4a 10 mov %rcx,0x10(%rdx)
ffffffffa03898e7: 48 8b 48 f8 mov -0x8(%rax),%rcx
ffffffffa03898eb: 48 89 4a 18 mov %rcx,0x18(%rdx)
wich correspond to offset 68a0 (from objdump):
kvm_set_irq():
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:161
68a0: 49 8b 85 58 24 00 00 mov 0x2458(%r13),%rax
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
68a7: 3b 98 28 01 00 00 cmp 0x128(%rax),%ebx
root@ayrshire:~# addr2line -e
/lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko 0x68a0
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:161
So here kvm->irq_routing is null.
How can it be?
Regards.
[-- Attachment #2: jean-philippe_menil.vcf --]
[-- Type: text/x-vcard, Size: 361 bytes --]
begin:vcard
fn:Jean-Philippe Menil
n:Menil;Jean-Philippe
org;quoted-printable:Universit=C3=A9 de Nantes;IRTS - DSI
adr;quoted-printable:;;2 rue de la Houssini=C3=A8re;Nantes;Loire-Atlantique;44382;France
email;internet:jean-philippe.menil@univ-nantes.fr
title;quoted-printable:Administrateur R=C3=A9seau
url:http://www.cri.univ-nantes.fr
version:2.1
end:vcard
[-- Attachment #3: Type: text/plain, Size: 184 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* RE: [PATCH 1/6] Staging: hv: Unify hyper-v device abstractions
From: KY Srinivasan @ 2011-03-02 1:40 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <20110301023449.GC1663@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Monday, February 28, 2011 9:35 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: Re: [PATCH 1/6] Staging: hv: Unify hyper-v device abstractions
>
> On Fri, Feb 25, 2011 at 06:05:30PM -0800, K. Y. Srinivasan wrote:
> > Hyper-V drivers have supported two device abstractions.
> > This patch implements a single device abstraction.
>
> "This patch" or "This patch series"?
Greg, If you recall, last week I had sent a single patch that did what these 6 patches do.
While I agree, the description could have been more informative; I have tried to
follow the guidelines I got in terms of what should go into each patch:
The first set of 3 patches (1 through 3) deal with the device related issues:
Patch 1: Combines the state in hv_device into vm_device to create a
single device abstraction called vm_device.
Patch 2: Changes the name of vm_device to hyperv_device.
Patch 3: Cleanup the names of variables that refer to hyperv_device
Next set of 3 patches (4 through 6) deal with the driver related issues:
Patch 4: Combines the state maintained in hv_driver into driver_context to
Create a single driver abstraction called driver_context.
Patch 5: Renames the driver context to hyperv_driver.
Patch 6: Cleanup the names of variables that refer to hyperv_driver.
I am beginning to wonder if perhaps I chose the wrong granularity in splitting
up these patches.
Regards,
K. Y
>
> > This simplifies the code and avoids duplication
> > of state.
>
> This patch is confusing, you are renaming structures (from hv_ to vm_)
> which I didn't think you wanted to do.
>
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> >
> > ---
> > drivers/staging/hv/blkvsc.c | 17 ++++---
> > drivers/staging/hv/blkvsc_drv.c | 14 +++---
> > drivers/staging/hv/channel_mgmt.c | 1 +
> > drivers/staging/hv/channel_mgmt.h | 2 +-
> > drivers/staging/hv/netvsc.c | 55 ++++++++++++-----------
> > drivers/staging/hv/netvsc.h | 2 +-
> > drivers/staging/hv/netvsc_api.h | 12 +++---
> > drivers/staging/hv/netvsc_drv.c | 28 +++++-------
> > drivers/staging/hv/rndis_filter.c | 19 ++++----
> > drivers/staging/hv/storvsc.c | 37 ++++++++--------
> > drivers/staging/hv/storvsc_api.h | 4 +-
> > drivers/staging/hv/storvsc_drv.c | 21 ++++-----
> > drivers/staging/hv/vmbus.h | 13 +++---
> > drivers/staging/hv/vmbus_api.h | 29 ++----------
> > drivers/staging/hv/vmbus_drv.c | 84 +++++++++++++++---------------------
> > drivers/staging/hv/vmbus_private.h | 12 +++---
> > 16 files changed, 157 insertions(+), 193 deletions(-)
> >
> > diff --git a/drivers/staging/hv/blkvsc.c b/drivers/staging/hv/blkvsc.c
> > index 7c8729b..ecface3 100644
> > --- a/drivers/staging/hv/blkvsc.c
> > +++ b/drivers/staging/hv/blkvsc.c
> > @@ -35,7 +35,8 @@ static const struct hv_guid g_blk_device_type = {
> > }
> > };
> >
> > -static int blk_vsc_on_device_add(struct hv_device *device, void
> *additional_info)
> > +static int blk_vsc_on_device_add(struct vm_device *device,
> > + void *additional_info)
>
> Huh? What was this change for? 80 column issues for function
> definitions is not a big deal, if any, and should not be burried in a
> patch that claims to do something else.
>
> Still totally confused,
>
> greg k-h
^ permalink raw reply
* RE: [PATCH 2/6] Staging: hv: Rename vm_device to hyperv_device
From: KY Srinivasan @ 2011-03-02 1:41 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <20110301023248.GB1663@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Monday, February 28, 2011 9:33 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: Re: [PATCH 2/6] Staging: hv: Rename vm_device to hyperv_device
>
> On Fri, Feb 25, 2011 at 06:06:03PM -0800, K. Y. Srinivasan wrote:
> > Rename the vm_device abstraction as hyperv_device.
>
> That's a nice name, but it's the first one with a "hyperv_" prefix in
> this subsystem. Ok, second, but hyperv_service_context doesn't really
> count as it's not really used.
>
> Everything else is named "hv_" here. Which is it going to be, "hv_" or
> "hyperv_"?
>
> Either is fine, just be aware of the work involved if you pick
> "hyperv_"...
As far as the device state and driver state in this subsystem is concerned;
I thought the consensus was to name them hyperv_*
>
> You also rename vm_device_info to hyperv_device_info here, doing more
> than one thing in a single patch.
>
> Please don't do that, this should be 2 patches.
I changed all device related state to be consistently named. However,
I will break it up into 2 patches.
>
>
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> >
> > ---
> > drivers/staging/hv/blkvsc.c | 4 +-
> > drivers/staging/hv/blkvsc_drv.c | 8 ++--
> > drivers/staging/hv/channel_mgmt.h | 2 +-
> > drivers/staging/hv/netvsc.c | 56 +++++++++++++++++++----------------
> > drivers/staging/hv/netvsc.h | 2 +-
> > drivers/staging/hv/netvsc_api.h | 12 ++++----
> > drivers/staging/hv/netvsc_drv.c | 14 ++++----
> > drivers/staging/hv/rndis_filter.c | 18 ++++++------
> > drivers/staging/hv/storvsc.c | 36 ++++++++++++-----------
> > drivers/staging/hv/storvsc_api.h | 4 +-
> > drivers/staging/hv/storvsc_drv.c | 10 +++---
> > drivers/staging/hv/vmbus.h | 6 ++--
> > drivers/staging/hv/vmbus_api.h | 8 ++--
> > drivers/staging/hv/vmbus_drv.c | 50 ++++++++++++++++----------------
> > drivers/staging/hv/vmbus_private.h | 12 ++++----
> > 15 files changed, 124 insertions(+), 118 deletions(-)
> >
> > diff --git a/drivers/staging/hv/blkvsc.c b/drivers/staging/hv/blkvsc.c
> > index ecface3..47ccec2 100644
> > --- a/drivers/staging/hv/blkvsc.c
> > +++ b/drivers/staging/hv/blkvsc.c
> > @@ -35,8 +35,8 @@ static const struct hv_guid g_blk_device_type = {
> > }
> > };
> >
> > -static int blk_vsc_on_device_add(struct vm_device *device,
> > - void *additional_info)
> > +static int
> > +blk_vsc_on_device_add(struct hyperv_device *device, void *additional_info)
>
> Ick, why break the formatting like this? Please keep the return type on
> the same line as the function name.
>
> You do that in a number of places, please don't.
>
> thanks,
>
> greg k-h
^ permalink raw reply
* RE: [PATCH 3/6] Staging: hv: Cleanup hyperv_device variable names
From: KY Srinivasan @ 2011-03-02 1:42 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <20110301024403.GD1663@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Monday, February 28, 2011 9:44 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: Re: [PATCH 3/6] Staging: hv: Cleanup hyperv_device variable names
>
> On Fri, Feb 25, 2011 at 06:06:32PM -0800, K. Y. Srinivasan wrote:
> > Cleanup the names of variables that refer to the
> > hyperv_device abstraction.
>
> Clean them up to be what? Shorter? Nice? Full of rounded edges so
> that when we bump into them in the dark they don't poke us and cause us
> to shreak in pain?
>
> >
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
>
> Sweet, you cloned yourself, I thought only Alan Cox had achieved that
> goal...
>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> >
> > ---
> > drivers/staging/hv/blkvsc_drv.c | 12 ++--
> > drivers/staging/hv/netvsc.c | 4 +-
> > drivers/staging/hv/netvsc_drv.c | 36 ++++----
> > drivers/staging/hv/storvsc_drv.c | 44 +++++-----
> > drivers/staging/hv/vmbus_drv.c | 164 +++++++++++++++++++-----------------
> --
> > 5 files changed, 130 insertions(+), 130 deletions(-)
> >
> > diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
> > index 58ab0e8..305a665 100644
> > --- a/drivers/staging/hv/blkvsc_drv.c
> > +++ b/drivers/staging/hv/blkvsc_drv.c
> > @@ -95,7 +95,7 @@ struct blkvsc_request {
> > /* Per device structure */
> > struct block_device_context {
> > /* point back to our device context */
> > - struct hyperv_device *device_ctx;
> > + struct hyperv_device *device_obj;
>
> Hey, I was right, it does have more rounded edges, nicely done.
>
>
> > -static int netvsc_device_add(struct hyperv_device *device,
> > - void *additional_info);
> > +static int
> > +netvsc_device_add(struct hyperv_device *device, void *additional_info);
>
> Again with the function return value hiding. Please don't.
>
> > --- a/drivers/staging/hv/storvsc_drv.c
> > +++ b/drivers/staging/hv/storvsc_drv.c
> > @@ -43,7 +43,7 @@ struct host_device_context {
> > /* must be 1st field
> > * FIXME this is a bug */
> > /* point back to our device context */
> > - struct hyperv_device *device_ctx;
> > + struct hyperv_device *device_obj;
>
> I really don't understand this change at all. "obj" is just as vapid
> and clueless as "ctx" is, and it seems very gratuitous to change this.
> And I should know, I have made a lot of gratuitous renames in my time in
> the kernel...
Greg, there are not that many options here. As I recall there was universal
objection to the use of *context/*ctx to refer to device or driver objects.
The name I chose is fairly descriptive of what it represents. If there is consensus
on a better name, I will use it.
>
> > static int vmbus_uevent(struct device *device, struct kobj_uevent_env *env)
> > {
> > - struct hyperv_device *device_ctx = device_to_hyperv_device(device);
> > + struct hyperv_device *device_obj = device_to_hyperv_device(device);
> > int ret;
> >
> > DPRINT_INFO(VMBUS_DRV, "generating uevent -
> VMBUS_DEVICE_CLASS_GUID={"
> > "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> > "%02x%02x%02x%02x%02x%02x%02x%02x}",
> > - device_ctx->class_id.data[3], device_ctx->class_id.data[2],
> > - device_ctx->class_id.data[1], device_ctx->class_id.data[0],
> > - device_ctx->class_id.data[5], device_ctx->class_id.data[4],
> > - device_ctx->class_id.data[7], device_ctx->class_id.data[6],
> > - device_ctx->class_id.data[8], device_ctx->class_id.data[9],
> > - device_ctx->class_id.data[10],
> > - device_ctx->class_id.data[11],
> > - device_ctx->class_id.data[12],
> > - device_ctx->class_id.data[13],
> > - device_ctx->class_id.data[14],
> > - device_ctx->class_id.data[15]);
> > + device_obj->class_id.data[3], device_obj->class_id.data[2],
> > + device_obj->class_id.data[1], device_obj->class_id.data[0],
> > + device_obj->class_id.data[5], device_obj->class_id.data[4],
> > + device_obj->class_id.data[7], device_obj->class_id.data[6],
> > + device_obj->class_id.data[8], device_obj->class_id.data[9],
> > + device_obj->class_id.data[10],
> > + device_obj->class_id.data[11],
> > + device_obj->class_id.data[12],
> > + device_obj->class_id.data[13],
> > + device_obj->class_id.data[14],
> > + device_obj->class_id.data[15]);
>
> After seeing this stuff so many times I'm waiting for a helper function
> to be added for it in this subsystem. I'm sure you really don't want to
> edit GUID printk-like functions ever again, right?
I will have a separate patch for this.
>
> > static void vmbus_device_release(struct device *device)
> > {
> > - struct hyperv_device *device_ctx = device_to_hyperv_device(device);
> > + struct hyperv_device *device_obj = device_to_hyperv_device(device);
> >
> > - kfree(device_ctx);
> > + kfree(device_obj);
> >
> > - /* !!DO NOT REFERENCE device_ctx anymore at this point!! */
> > + /* !!DO NOT REFERENCE device_obj anymore at this point!! */
> > }
>
> I think by virtue of the kfree() right above this comment, it should be
> deleted. Especially as it is at the end of the function, making it
> pretty much impossible to make any sense here...
For what it is worth; this is existing code. In the spirit of one thing per patch;
there will be a patch for this.
>
> Come on, global search-and-replace needs to be done in a sane manner,
> other wise you can just send me a vi macro to run on the code, it would
> be the same thing in the end (hint, don't do that, only one person has
> ever gotten away with doing that in the history of the kernel, in an act
> never to be ever repeated again.)
Greg, apart from your objection to the name I picked to refer to variable
referring to struct hyperv_device; what else is the problem here.
Regards,
K. Y
>
> thanks,
>
> greg k-h
^ permalink raw reply
* RE: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
From: KY Srinivasan @ 2011-03-02 1:43 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <20110301025230.GE1663@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Monday, February 28, 2011 9:53 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
>
> On Fri, Feb 25, 2011 at 06:07:03PM -0800, K. Y. Srinivasan wrote:
> > This patch combines the two driver abstractions into
> > a single driver abstraction.
>
> Ah, how sweet. Unfortunatly you don't say "how" you did this.
>
> Nor do you describe _what_ those two driver abstractions were. Are we
> talking i2c and usb abstractions? gpio and spi? Driver core and
> platform? We want to know exactly what is going on here.
My mistake; I will have a more descriptive comment.
>
> Think of writing something that when you look back, in 3 years, while
> staring at a Linux hyperv driver originally written for the 2.6.9
> kernel, that somehow never got forward ported and you are tasked with
> doing this, that you can just do a simple 'git log drivers/staging/hv/'
> and instantly know just from the changelog comments exactly what you
> need to do to your driver to clean it up and properly get it to work on
> the new 8.2.2 kernel release.
>
> This changelog entry, would require you to go and dig through the guts
> of the patch itself, trying to figure out what abstractions you are
> talking about, and exactly how they were combined, all the while
> wondering _why_ they were combined.
>
> Please, think of your future self, you will thank him in the years to
> come by doing this properly. Not to mention making other's lives easier
> if you happen to have escaped this dire task by then.
>
> Oh, you have an extra space up there in the subject, please fix it next
> time.
>
> > -int blk_vsc_initialize(struct hv_driver *driver)
> > +int blk_vsc_initialize(struct driver_context *driver)
>
> "struct driver_context"? Oh please no.
Greg; this is the patch that consolidates the state in struct hv_driver into
struct driver_context. In the spirit of doing one thing in a patch;
other relevant changes are made in:
Patch[5/6]: Changes the name driver_context to hyperv_driver
Patch[6/6]: Cleanup all variable names that refer to struct hyperv_driver.
>
> I realize that you are hopefully going to later rename this to something
> else, but remember, a few patches back you thought that the "ctx" name
> wasn't nice. And here you go resuscitating it from the graveyard of
> pointy bits.
As I noted in a different email, may be the granularity I chose in breaking up
these patches is causing all this confusion.
>
> And what happens if your future patch is rejected? You are stuck with a
> "driver_context" structure in a subsystem? That's a pretty big abuse of
> the global namespace, don't you think? It sounds like something that
> should go into include/linux/device.h
>
> Please be careful about names, they mean things, even when you think
> they don't.
Greg, would it be better if I just had one patch for dealing with all device related issues and a
Separate patch for dealing all driver related issues?
Regards,
K. Y
^ permalink raw reply
* RE: [PATCH 6/6] Staging: hv: Cleanup hyperv_driver variable names
From: KY Srinivasan @ 2011-03-02 1:44 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
virtualization@lists.osdl.org, devel@linuxdriverproject.org
In-Reply-To: <20110301025859.GG1663@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Monday, February 28, 2011 9:59 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: Re: [PATCH 6/6] Staging: hv: Cleanup hyperv_driver variable names
>
> On Fri, Feb 25, 2011 at 06:07:58PM -0800, K. Y. Srinivasan wrote:
> > The title says it all.
>
> That's a horrible changelog comment. So bad that I would rather see an
> empty message than this one. Seriously, it give no description, and
> makes us think that the whole patch is obvious, when it really isn't.
>
> What did you change them to?
> What did you change them from?
> What was your motivation in changing them?
> How were you feeling when the names changed?
>
> Ok, maybe not the last one, but you get the idea.
Greg, these changes (patches 1 through 6) change so much in this
sub-system that until these changes go in, our cleanup efforts are stalled. That
is the main reason I was so hasty in submitting these patches.
Clearly, I need to provide a better changelog comment; and I will. Looking at your
other comments, I am wondering if the granularity I chose for breaking up the
changes that had to be done is also a significant part of the problem. If it is ok with you,
I could generate a patch that deals with all device related issues and a patch that deals with
all driver related issues. These patches obviously will do more than one thing (however
they will all be related); but at least they
won't have intermediate state that would be objectionable. Let me know.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH 3/6] Staging: hv: Cleanup hyperv_device variable names
From: Greg KH @ 2011-03-02 5:39 UTC (permalink / raw)
To: KY Srinivasan
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E04801660E@TK5EX14MBXC128.redmond.corp.microsoft.com>
On Wed, Mar 02, 2011 at 01:42:37AM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Monday, February 28, 2011 9:44 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> > Janssen
> > Subject: Re: [PATCH 3/6] Staging: hv: Cleanup hyperv_device variable names
> >
> > On Fri, Feb 25, 2011 at 06:06:32PM -0800, K. Y. Srinivasan wrote:
> > > Cleanup the names of variables that refer to the
> > > hyperv_device abstraction.
> >
> > Clean them up to be what? Shorter? Nice? Full of rounded edges so
> > that when we bump into them in the dark they don't poke us and cause us
> > to shreak in pain?
> >
> > >
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> >
> > Sweet, you cloned yourself, I thought only Alan Cox had achieved that
> > goal...
> >
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > >
> > > ---
> > > drivers/staging/hv/blkvsc_drv.c | 12 ++--
> > > drivers/staging/hv/netvsc.c | 4 +-
> > > drivers/staging/hv/netvsc_drv.c | 36 ++++----
> > > drivers/staging/hv/storvsc_drv.c | 44 +++++-----
> > > drivers/staging/hv/vmbus_drv.c | 164 +++++++++++++++++++-----------------
> > --
> > > 5 files changed, 130 insertions(+), 130 deletions(-)
> > >
> > > diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
> > > index 58ab0e8..305a665 100644
> > > --- a/drivers/staging/hv/blkvsc_drv.c
> > > +++ b/drivers/staging/hv/blkvsc_drv.c
> > > @@ -95,7 +95,7 @@ struct blkvsc_request {
> > > /* Per device structure */
> > > struct block_device_context {
> > > /* point back to our device context */
> > > - struct hyperv_device *device_ctx;
> > > + struct hyperv_device *device_obj;
> >
> > Hey, I was right, it does have more rounded edges, nicely done.
> >
> >
> > > -static int netvsc_device_add(struct hyperv_device *device,
> > > - void *additional_info);
> > > +static int
> > > +netvsc_device_add(struct hyperv_device *device, void *additional_info);
> >
> > Again with the function return value hiding. Please don't.
> >
> > > --- a/drivers/staging/hv/storvsc_drv.c
> > > +++ b/drivers/staging/hv/storvsc_drv.c
> > > @@ -43,7 +43,7 @@ struct host_device_context {
> > > /* must be 1st field
> > > * FIXME this is a bug */
> > > /* point back to our device context */
> > > - struct hyperv_device *device_ctx;
> > > + struct hyperv_device *device_obj;
> >
> > I really don't understand this change at all. "obj" is just as vapid
> > and clueless as "ctx" is, and it seems very gratuitous to change this.
> > And I should know, I have made a lot of gratuitous renames in my time in
> > the kernel...
>
> Greg, there are not that many options here. As I recall there was universal
> objection to the use of *context/*ctx to refer to device or driver objects.
> The name I chose is fairly descriptive of what it represents. If there is consensus
> on a better name, I will use it.
Do it like other subsystems, call it a device with the prefix for the
type. So for this one, it would be:
struct hyperv_device *hyperv_device;
or
struct hyperv_device *hyperv_dev;
> > Come on, global search-and-replace needs to be done in a sane manner,
> > other wise you can just send me a vi macro to run on the code, it would
> > be the same thing in the end (hint, don't do that, only one person has
> > ever gotten away with doing that in the history of the kernel, in an act
> > never to be ever repeated again.)
>
> Greg, apart from your objection to the name I picked to refer to variable
> referring to struct hyperv_device; what else is the problem here.
Changing the comment that should be removed instead.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
From: Greg KH @ 2011-03-02 5:41 UTC (permalink / raw)
To: KY Srinivasan
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E04801661A@TK5EX14MBXC128.redmond.corp.microsoft.com>
On Wed, Mar 02, 2011 at 01:43:13AM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Monday, February 28, 2011 9:53 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> > Janssen
> > Subject: Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
> >
> > On Fri, Feb 25, 2011 at 06:07:03PM -0800, K. Y. Srinivasan wrote:
> > > This patch combines the two driver abstractions into
> > > a single driver abstraction.
> >
> > Ah, how sweet. Unfortunatly you don't say "how" you did this.
> >
> > Nor do you describe _what_ those two driver abstractions were. Are we
> > talking i2c and usb abstractions? gpio and spi? Driver core and
> > platform? We want to know exactly what is going on here.
>
> My mistake; I will have a more descriptive comment.
>
> >
> > Think of writing something that when you look back, in 3 years, while
> > staring at a Linux hyperv driver originally written for the 2.6.9
> > kernel, that somehow never got forward ported and you are tasked with
> > doing this, that you can just do a simple 'git log drivers/staging/hv/'
> > and instantly know just from the changelog comments exactly what you
> > need to do to your driver to clean it up and properly get it to work on
> > the new 8.2.2 kernel release.
> >
> > This changelog entry, would require you to go and dig through the guts
> > of the patch itself, trying to figure out what abstractions you are
> > talking about, and exactly how they were combined, all the while
> > wondering _why_ they were combined.
> >
> > Please, think of your future self, you will thank him in the years to
> > come by doing this properly. Not to mention making other's lives easier
> > if you happen to have escaped this dire task by then.
> >
> > Oh, you have an extra space up there in the subject, please fix it next
> > time.
> >
> > > -int blk_vsc_initialize(struct hv_driver *driver)
> > > +int blk_vsc_initialize(struct driver_context *driver)
> >
> > "struct driver_context"? Oh please no.
>
> Greg; this is the patch that consolidates the state in struct hv_driver into
> struct driver_context. In the spirit of doing one thing in a patch;
> other relevant changes are made in:
> Patch[5/6]: Changes the name driver_context to hyperv_driver
> Patch[6/6]: Cleanup all variable names that refer to struct hyperv_driver.
Yes, but on its own, this patch is wrong, that is not a valid name, even
if it is a "temporary" name.
> > I realize that you are hopefully going to later rename this to something
> > else, but remember, a few patches back you thought that the "ctx" name
> > wasn't nice. And here you go resuscitating it from the graveyard of
> > pointy bits.
>
> As I noted in a different email, may be the granularity I chose in breaking up
> these patches is causing all this confusion.
Yes, as I think you need to go much finer as you were doing more than
one thing in these patches, and not describing them properly at all.
Please try to redo them in a simpler manner, probably breaking it into
more steps, so we can properly review them.
> > And what happens if your future patch is rejected? You are stuck with a
> > "driver_context" structure in a subsystem? That's a pretty big abuse of
> > the global namespace, don't you think? It sounds like something that
> > should go into include/linux/device.h
> >
> > Please be careful about names, they mean things, even when you think
> > they don't.
>
> Greg, would it be better if I just had one patch for dealing with all
> device related issues and a Separate patch for dealing all driver
> related issues?
Again, only do one thing per patch. So yes, of course they should be
separate.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 6/6] Staging: hv: Cleanup hyperv_driver variable names
From: Greg KH @ 2011-03-02 5:42 UTC (permalink / raw)
To: KY Srinivasan
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E048016633@TK5EX14MBXC128.redmond.corp.microsoft.com>
On Wed, Mar 02, 2011 at 01:44:11AM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Monday, February 28, 2011 9:59 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> > Janssen
> > Subject: Re: [PATCH 6/6] Staging: hv: Cleanup hyperv_driver variable names
> >
> > On Fri, Feb 25, 2011 at 06:07:58PM -0800, K. Y. Srinivasan wrote:
> > > The title says it all.
> >
> > That's a horrible changelog comment. So bad that I would rather see an
> > empty message than this one. Seriously, it give no description, and
> > makes us think that the whole patch is obvious, when it really isn't.
> >
> > What did you change them to?
> > What did you change them from?
> > What was your motivation in changing them?
> > How were you feeling when the names changed?
> >
> > Ok, maybe not the last one, but you get the idea.
>
> Greg, these changes (patches 1 through 6) change so much in this
> sub-system that until these changes go in, our cleanup efforts are
> stalled.
Ok, but that's not my issue, it's yours :)
And what's the rush?
> That is the main reason I was so hasty in submitting these patches.
> Clearly, I need to provide a better changelog comment; and I will.
> Looking at your other comments, I am wondering if the granularity I
> chose for breaking up the changes that had to be done is also a
> significant part of the problem. If it is ok with you, I could
> generate a patch that deals with all device related issues and a
> patch that deals with all driver related issues. These patches
> obviously will do more than one thing (however they will all be
> related); but at least they won't have intermediate state that would
> be objectionable. Let me know.
Ick, no, you need to break these up into smaller pieces than just the 6
you did here, as you were still doing more than one thing per patch.
Please make them simpler, not more complex.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/6] Staging: hv: Unify hyper-v device abstractions
From: Dan Carpenter @ 2011-03-02 6:35 UTC (permalink / raw)
To: KY Srinivasan
Cc: Greg KH, gregkh@suse.de, linux-kernel@vger.kernel.org,
virtualization@lists.osdl.org, devel@linuxdriverproject.org
In-Reply-To: <6E21E5352C11B742B20C142EB499E0480165E6@TK5EX14MBXC128.redmond.corp.microsoft.com>
The problem is that everyone reading [patch 1/6] thinks you're renaming
hv_device to vm_device or introducing a new struct vm_device. That
makes people annoyed. If you had written the patch description like this:
In the original code, the structs vm_device included a struct
hv_device. This patch moves the members from hv_device directly
into struct vm_device.
The members ->dev_type and ->dev_instance from hv_device were
the same as ->class_id and ->device_id in vm_device so those
were not copied over.
Now that everything is included into vm_device directly,
hv_device is unused and we can delete the definition.
There still might be issues with the patch, but at least you would be
talking about the same thing.
regards,
dan carpenter
^ permalink raw reply
* [PATCH 0/2] Fix hot-unplug: device removal while port in use
From: Amit Shah @ 2011-03-02 8:23 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah
A crash was observed when a device gets removed while a port is in
use. When the port gets removed, we tried to free vq buffers. The vq
no longer exists at this stage, just ensure we don't access it.
The second patch fixes a warning where the pci region is already
freed. I'm not sure what or how the region gets freed, any clues
there will be helpful.
Thanks,
Amit
Amit Shah (2):
virtio: console: Don't access vqs if device was unplugged
virtio: console: Don't call device_destroy() on port device
drivers/char/virtio_console.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
--
1.7.4
^ permalink raw reply
* [PATCH 1/2] virtio: console: Don't access vqs if device was unplugged
From: Amit Shah @ 2011-03-02 8:23 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, stable
In-Reply-To: <cover.1299054025.git.amit.shah@redhat.com>
If a virtio-console device gets unplugged while a port is open, a
subsequent close() call on the port accesses vqs to free up buffers.
This can lead to a crash.
The buffers are already freed up as a result of the call to
unplug_ports() from virtcons_remove(). The fix is to simply not access
vq information if port->portdev is NULL.
Reported-by: juzhang <juzhang@redhat.com>
CC: stable@kernel.org
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 4903931..84b164d 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -388,6 +388,10 @@ static void discard_port_data(struct port *port)
unsigned int len;
int ret;
+ if (!port->portdev) {
+ /* Device has been unplugged. vqs are already gone. */
+ return;
+ }
vq = port->in_vq;
if (port->inbuf)
buf = port->inbuf;
@@ -470,6 +474,10 @@ static void reclaim_consumed_buffers(struct port *port)
void *buf;
unsigned int len;
+ if (!port->portdev) {
+ /* Device has been unplugged. vqs are already gone. */
+ return;
+ }
while ((buf = virtqueue_get_buf(port->out_vq, &len))) {
kfree(buf);
port->outvq_full = false;
--
1.7.4
^ permalink raw reply related
* [PATCH 2/2] virtio: console: Don't call device_destroy() on port device
From: Amit Shah @ 2011-03-02 8:23 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, stable
In-Reply-To: <cover.1299054025.git.amit.shah@redhat.com>
This results in a warning mentioning the region being removed is already
gone.
CC: stable@kernel.org
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 84b164d..5ac9fd9 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1244,7 +1244,7 @@ static void remove_port(struct kref *kref)
port = container_of(kref, struct port, kref);
sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
- device_destroy(pdrvdata.class, port->dev->devt);
+
cdev_del(port->cdev);
kfree(port->name);
--
1.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox