Linux virtualization list
 help / color / mirror / Atom feed
* [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

* Re: [PATCH 0/2] Fix hot-unplug: device removal while port in use
From: Rusty Russell @ 2011-03-02 11:08 UTC (permalink / raw)
  To: Virtualization List; +Cc: Amit Shah
In-Reply-To: <cover.1299054025.git.amit.shah@redhat.com>

On Wed,  2 Mar 2011 13:53:06 +0530, Amit Shah <amit.shah@redhat.com> wrote:
> 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.

Put a printk and WARN_ON() in the pci region freeing code, look through
the backtraces?

Rusty.

^ permalink raw reply

* Re: [PATCH 0/2] Fix hot-unplug: device removal while port in use
From: Amit Shah @ 2011-03-02 13:38 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Virtualization List
In-Reply-To: <87aahdn5k7.fsf@rustcorp.com.au>

On (Wed) 02 Mar 2011 [21:38:08], Rusty Russell wrote:
> On Wed,  2 Mar 2011 13:53:06 +0530, Amit Shah <amit.shah@redhat.com> wrote:
> > 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.
> 
> Put a printk and WARN_ON() in the pci region freeing code, look through
> the backtraces?

Well what seems to be happening is kref_put() in port_fops_release()
calls remove_port(), which calls device_destroy().  Now this triggers
another fput() on the same file, causing port_fops_release() to be
called again, which leads to device_destroy() being called on the same
region.

Slightly more clueful, but still clueless as to why this happens.

		Amit

^ permalink raw reply

* RE: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
From: KY Srinivasan @ 2011-03-03  2:50 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: <20110302054125.GB28382@kroah.com>



> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Wednesday, March 02, 2011 12:41 AM
> 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 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.

Greg, the temporary name happens to be the name currently in use in the 
code - this is not the name I introduced. Think of this as the surviving 
data structure after  the hv_driver state is consolidated into 
(the existing) driver_context data structure.  I did this in the spirit of 
doing one thing at a time. If I am going to be
picking a more appropriate name for the consolidated data structure; I 
might as well pick the final name that we want this unified driver abstraction 
to be called. 


> 
> > > 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.

Based on your comments on intermediate names, would you recommend that
as part  of consolidating the driver abstractions, I also rename this combined 
state. 

Regards,

K. Y

^ permalink raw reply

* Re: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
From: Greg KH @ 2011-03-03  6:10 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Greg KH, linux-kernel@vger.kernel.org,
	devel@linuxdriverproject.org, virtualization@lists.osdl.org,
	Haiyang Zhang, Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E048016A13@TK5EX14MBXC128.redmond.corp.microsoft.com>

On Thu, Mar 03, 2011 at 02:50:00AM +0000, KY Srinivasan wrote:
> > > > "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.
> 
> Greg, the temporary name happens to be the name currently in use in the 
> code - this is not the name I introduced.

There is not a "struct driver_context" in the code that I see today, or
am I missing something?  That's my objection here, please don't use that
name, it's not valid for a subsystem to use, even for a tiny bit.

> Think of this as the surviving data structure after  the hv_driver
> state is consolidated into (the existing) driver_context data
> structure.  I did this in the spirit of doing one thing at a time. If
> I am going to be picking a more appropriate name for the consolidated
> data structure; I might as well pick the final name that we want this
> unified driver abstraction to be called. 

Your final name is fine, it's the intermediate one I'm objecting to.

How about 'struct hv_driver_context' instead?

> > > > 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.
> 
> Based on your comments on intermediate names, would you recommend that
> as part  of consolidating the driver abstractions, I also rename this combined 
> state. 

Probably, if I understand what you are referring to.  Please post code
so that I really know what you are doing :)

thanks,

greg k-h

^ permalink raw reply

* Re: Bug inkvm_set_irq
From: Michael S. Tsirkin @ 2011-03-03 14:47 UTC (permalink / raw)
  To: Jean-Philippe Menil; +Cc: netdev, kvm, virtualization
In-Reply-To: <4D6D0510.4060300@univ-nantes.fr>

On Tue, Mar 01, 2011 at 03:39:12PM +0100, Jean-Philippe Menil wrote:
> 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.

Not null, this seems to be invalid.
I suspect use after free where the kvm pointer is
pointing at some random memory. Use after free?
Could you please try enabling a slab debugger,
recompile and rerun the test?

-- 
MST

^ permalink raw reply

* Re: Bug inkvm_set_irq
From: Jean-Philippe Menil @ 2011-03-03 15:26 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20110303144715.GA12400@redhat.com>

Le 03/03/2011 15:47, Michael S. Tsirkin a écrit :
> On Tue, Mar 01, 2011 at 03:39:12PM +0100, Jean-Philippe Menil wrote:
>> 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.
> Not null, this seems to be invalid.
> I suspect use after free where the kvm pointer is
> pointing at some random memory. Use after free?
> Could you please try enabling a slab debugger,
> recompile and rerun the test?
>
Hi,

I'm not sure to activate the right thing.
Is that what you want?

CONFIG_SLAB=y
CONFIG_SLABINFO=y
CONFIG_DEBUG_SLAB=y
CONFIG_DEBUG_SLAB_LEAK=y

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-03-03 15:55 UTC (permalink / raw)
  To: Jean-Philippe Menil; +Cc: netdev, kvm, virtualization
In-Reply-To: <4D6FB313.1010400@univ-nantes.fr>

On Thu, Mar 03, 2011 at 04:26:11PM +0100, Jean-Philippe Menil wrote:
> Le 03/03/2011 15:47, Michael S. Tsirkin a écrit :
> >On Tue, Mar 01, 2011 at 03:39:12PM +0100, Jean-Philippe Menil wrote:
> >>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.
> >Not null, this seems to be invalid.
> >I suspect use after free where the kvm pointer is
> >pointing at some random memory. Use after free?
> >Could you please try enabling a slab debugger,
> >recompile and rerun the test?
> >
> Hi,
> 
> I'm not sure to activate the right thing.
> Is that what you want?
> 
> CONFIG_SLAB=y
> CONFIG_SLABINFO=y
> CONFIG_DEBUG_SLAB=y
> CONFIG_DEBUG_SLAB_LEAK=y
> 
> Regards.

Yes, maybe disable SLAB_LEAK.

> -- 
> 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: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
From: KY Srinivasan @ 2011-03-03 21:16 UTC (permalink / raw)
  To: Greg KH
  Cc: Greg KH, linux-kernel@vger.kernel.org,
	devel@linuxdriverproject.org, virtualization@lists.osdl.org,
	Haiyang Zhang, Hank Janssen
In-Reply-To: <20110303061002.GD32209@suse.de>



> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Thursday, March 03, 2011 1:10 AM
> To: KY Srinivasan
> Cc: Greg KH; 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 Thu, Mar 03, 2011 at 02:50:00AM +0000, KY Srinivasan wrote:
> > > > > "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.
> >
> > Greg, the temporary name happens to be the name currently in use in the
> > code - this is not the name I introduced.
> 
> There is not a "struct driver_context" in the code that I see today, or
> am I missing something?  That's my objection here, please don't use that
> name, it's not valid for a subsystem to use, even for a tiny bit.

Look at the file vmbus.h  you will see struct driver_context. This has been there for as long 
as I have seen this code. 

> 
> > Think of this as the surviving data structure after  the hv_driver
> > state is consolidated into (the existing) driver_context data
> > structure.  I did this in the spirit of doing one thing at a time. If
> > I am going to be picking a more appropriate name for the consolidated
> > data structure; I might as well pick the final name that we want this
> > unified driver abstraction to be called.
> 
> Your final name is fine, it's the intermediate one I'm objecting to.

Ok.

> 
> How about 'struct hv_driver_context' instead?
> 
> > > > > 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.
> >
> > Based on your comments on intermediate names, would you recommend that
> > as part  of consolidating the driver abstractions, I also rename this combined
> > state.
> 
> Probably, if I understand what you are referring to.  Please post code
> so that I really know what you are doing :)
> 
> thanks,
> 
> greg k-h

Regards,

K. Y

^ permalink raw reply

* Re: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
From: Greg KH @ 2011-03-03 21:22 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Greg KH, linux-kernel@vger.kernel.org,
	devel@linuxdriverproject.org, virtualization@lists.osdl.org,
	Haiyang Zhang, Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E048016CD9@TK5EX14MBXC128.redmond.corp.microsoft.com>

On Thu, Mar 03, 2011 at 09:16:29PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@suse.de]
> > Sent: Thursday, March 03, 2011 1:10 AM
> > To: KY Srinivasan
> > Cc: Greg KH; 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 Thu, Mar 03, 2011 at 02:50:00AM +0000, KY Srinivasan wrote:
> > > > > > "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.
> > >
> > > Greg, the temporary name happens to be the name currently in use in the
> > > code - this is not the name I introduced.
> > 
> > There is not a "struct driver_context" in the code that I see today, or
> > am I missing something?  That's my objection here, please don't use that
> > name, it's not valid for a subsystem to use, even for a tiny bit.
> 
> Look at the file vmbus.h  you will see struct driver_context. This has
> been there for as long as I have seen this code. 

Ok, I am rightly corrected, I totally missed that, you are right.

Feel free to resend after addressing the other issues.

I'll fix up the hv_mouse driver, you don't have to worry about that one
if you don't want to, just ignore it please.

thanks,

greg k-h

^ permalink raw reply

* [PATCH] virtio: console: Don't access vqs if device was unplugged
From: Rusty Russell @ 2011-03-04  3:34 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Amit Shah, virtualization

From: Amit Shah <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>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 drivers/char/virtio_console.c |    8 ++++++++
 1 file changed, 8 insertions(+)

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;

^ permalink raw reply related

* Re: Bug inkvm_set_irq
From: Jean-Philippe Menil @ 2011-03-04  9:22 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20110303155533.GA13310@redhat.com>

Le 03/03/2011 16:55, Michael S. Tsirkin a écrit :
> On Thu, Mar 03, 2011 at 04:26:11PM +0100, Jean-Philippe Menil wrote:
>> Le 03/03/2011 15:47, Michael S. Tsirkin a écrit :
>>> On Tue, Mar 01, 2011 at 03:39:12PM +0100, Jean-Philippe Menil wrote:
>>>> 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.
>>> Not null, this seems to be invalid.
>>> I suspect use after free where the kvm pointer is
>>> pointing at some random memory. Use after free?
>>> Could you please try enabling a slab debugger,
>>> recompile and rerun the test?
>>>
>> Hi,
>>
>> I'm not sure to activate the right thing.
>> Is that what you want?
>>
>> CONFIG_SLAB=y
>> CONFIG_SLABINFO=y
>> CONFIG_DEBUG_SLAB=y
>> CONFIG_DEBUG_SLAB_LEAK=y
>>
>> Regards.
> Yes, maybe disable SLAB_LEAK.
>
>> -- 
>> 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 kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hi,

so this time, here is what markup_oops says:

root@ayrshire:~# cat oops-0403.txt | perl markup_oops.pl -m 
/lib/modules/2.6.37.2.999-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko 
/boot/vmlinuz-2.6.37.2.999-dsiun-110105+
vmaoffset = 18446744072102948864 ffffffffa03d9811:    48 89 
e5                 mov    %rsp,%rbp
  ffffffffa03d9814:    41 57                    push   %r15
  ffffffffa03d9816:    41 89 cf                 mov    %ecx,%r15d  |  
%r15 => 1  %ecx = 1
  ffffffffa03d9819:    41 56                    push   %r14        |  
%r14 => ffffffffa03daa50
  ffffffffa03d981b:    41 55                    push   %r13
  ffffffffa03d981d:    49 89 fd                 mov    %rdi,%r13   |  
%edi = 6b6b6b6b6b6b6b6b  %r13 => 6b6b6b6b6b6b6b6b
  ffffffffa03d9820:    41 54                    push   %r12        |  
%r12 => 6b6b6b6b6b6b6b6b
  ffffffffa03d9822:    53                       push   %rbx
  ffffffffa03d9823:    89 d3                    mov    %edx,%ebx   |  
%ebx => 6b6b6b6b
  ffffffffa03d9825:    48 81 ec a8 00 00 00     sub    $0xa8,%rsp
  ffffffffa03d982c:    8b 15 00 00 00 00        mov    
0x0(%rip),%edx        # ffffffffa03d9832 <kvm_set_irq+0x22>
  ffffffffa03d9832:    89 b5 3c ff ff ff        mov    %esi,-0xc4(%rbp) 
|  %esi = 0
  ffffffffa03d9838:    85 d2                    test   %edx,%edx   |  
%edx => 0
  ffffffffa03d983a:    0f 85 d5 00 00 00        jne    ffffffffa03d9915 
<kvm_set_irq+0x105>
*ffffffffa03d9840:    49 8b 85 58 24 00 00     mov    0x2458(%r13),%rax 
|  %eax = 0  %r13 = 6b6b6b6b6b6b6b6b <--- faulting instruction
  ffffffffa03d9847:    3b 98 28 01 00 00        cmp    0x128(%rax),%ebx
  ffffffffa03d984d:    73 61                    jae    ffffffffa03d98b0 
<kvm_set_irq+0xa0>
  ffffffffa03d984f:    89 db                    mov    %ebx,%ebx
  ffffffffa03d9851:    48 8b 84 d8 30 01 00     mov    
0x130(%rax,%rbx,8),%rax
  ffffffffa03d9858:    00
  ffffffffa03d9859:    48 85 c0                 test   %rax,%rax
  ffffffffa03d985c:    74 52                    je     ffffffffa03d98b0 
<kvm_set_irq+0xa0>
  ffffffffa03d985e:    48 8d 95 40 ff ff ff     lea    -0xc0(%rbp),%rdx
  ffffffffa03d9865:    31 db                    xor    %ebx,%ebx
  ffffffffa03d9867:    48 8b 08                 mov    (%rax),%rcx
  ffffffffa03d986a:    83 c3 01                 add    $0x1,%ebx
  ffffffffa03d986d:    0f 18 09                 prefetcht0 (%rcx)
  ffffffffa03d9870:    48 8b 48 e0              mov    -0x20(%rax),%rcx
  ffffffffa03d9874:    48 89 0a                 mov    %rcx,(%rdx)
  ffffffffa03d9877:    48 8b 48 e8              mov    -0x18(%rax),%rcx
  ffffffffa03d987b:    48 89 4a 08              mov    %rcx,0x8(%rdx)
  ffffffffa03d987f:    48 8b 48 f0              mov    -0x10(%rax),%rcx
  ffffffffa03d9883:    48 89 4a 10              mov    %rcx,0x10(%rdx)
  ffffffffa03d9887:    48 8b 48 f8              mov    -0x8(%rax),%rcx
  ffffffffa03d988b:    48 89 4a 18              mov    %rcx,0x18(%rdx)


Is that you wanted, the "6b6b6b6b6b6b6b6b" ?

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-03-04  9:35 UTC (permalink / raw)
  To: Jean-Philippe Menil; +Cc: netdev, kvm, virtualization
In-Reply-To: <4D70AF3B.5080007@univ-nantes.fr>

On Fri, Mar 04, 2011 at 10:22:03AM +0100, Jean-Philippe Menil wrote:
> Le 03/03/2011 16:55, Michael S. Tsirkin a écrit :
> >On Thu, Mar 03, 2011 at 04:26:11PM +0100, Jean-Philippe Menil wrote:
> >>Le 03/03/2011 15:47, Michael S. Tsirkin a écrit :
> >>>On Tue, Mar 01, 2011 at 03:39:12PM +0100, Jean-Philippe Menil wrote:
> >>>>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.
> >>>Not null, this seems to be invalid.
> >>>I suspect use after free where the kvm pointer is
> >>>pointing at some random memory. Use after free?
> >>>Could you please try enabling a slab debugger,
> >>>recompile and rerun the test?
> >>>
> >>Hi,
> >>
> >>I'm not sure to activate the right thing.
> >>Is that what you want?
> >>
> >>CONFIG_SLAB=y
> >>CONFIG_SLABINFO=y
> >>CONFIG_DEBUG_SLAB=y
> >>CONFIG_DEBUG_SLAB_LEAK=y
> >>
> >>Regards.
> >Yes, maybe disable SLAB_LEAK.
> >
> >>-- 
> >>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 kvm" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Hi,
> 
> so this time, here is what markup_oops says:
> 
> root@ayrshire:~# cat oops-0403.txt | perl markup_oops.pl -m
> /lib/modules/2.6.37.2.999-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko
> /boot/vmlinuz-2.6.37.2.999-dsiun-110105+
> vmaoffset = 18446744072102948864 ffffffffa03d9811:    48 89 e5
> mov    %rsp,%rbp
>  ffffffffa03d9814:    41 57                    push   %r15
>  ffffffffa03d9816:    41 89 cf                 mov    %ecx,%r15d  |
> %r15 => 1  %ecx = 1
>  ffffffffa03d9819:    41 56                    push   %r14        |
> %r14 => ffffffffa03daa50
>  ffffffffa03d981b:    41 55                    push   %r13
>  ffffffffa03d981d:    49 89 fd                 mov    %rdi,%r13   |
> %edi = 6b6b6b6b6b6b6b6b  %r13 => 6b6b6b6b6b6b6b6b
>  ffffffffa03d9820:    41 54                    push   %r12        |
> %r12 => 6b6b6b6b6b6b6b6b
>  ffffffffa03d9822:    53                       push   %rbx
>  ffffffffa03d9823:    89 d3                    mov    %edx,%ebx   |
> %ebx => 6b6b6b6b
>  ffffffffa03d9825:    48 81 ec a8 00 00 00     sub    $0xa8,%rsp
>  ffffffffa03d982c:    8b 15 00 00 00 00        mov    0x0(%rip),%edx
> # ffffffffa03d9832 <kvm_set_irq+0x22>
>  ffffffffa03d9832:    89 b5 3c ff ff ff        mov
> %esi,-0xc4(%rbp) |  %esi = 0
>  ffffffffa03d9838:    85 d2                    test   %edx,%edx   |
> %edx => 0
>  ffffffffa03d983a:    0f 85 d5 00 00 00        jne
> ffffffffa03d9915 <kvm_set_irq+0x105>
> *ffffffffa03d9840:    49 8b 85 58 24 00 00     mov
> 0x2458(%r13),%rax |  %eax = 0  %r13 = 6b6b6b6b6b6b6b6b <--- faulting
> instruction
>  ffffffffa03d9847:    3b 98 28 01 00 00        cmp    0x128(%rax),%ebx
>  ffffffffa03d984d:    73 61                    jae
> ffffffffa03d98b0 <kvm_set_irq+0xa0>
>  ffffffffa03d984f:    89 db                    mov    %ebx,%ebx
>  ffffffffa03d9851:    48 8b 84 d8 30 01 00     mov
> 0x130(%rax,%rbx,8),%rax
>  ffffffffa03d9858:    00
>  ffffffffa03d9859:    48 85 c0                 test   %rax,%rax
>  ffffffffa03d985c:    74 52                    je
> ffffffffa03d98b0 <kvm_set_irq+0xa0>
>  ffffffffa03d985e:    48 8d 95 40 ff ff ff     lea    -0xc0(%rbp),%rdx
>  ffffffffa03d9865:    31 db                    xor    %ebx,%ebx
>  ffffffffa03d9867:    48 8b 08                 mov    (%rax),%rcx
>  ffffffffa03d986a:    83 c3 01                 add    $0x1,%ebx
>  ffffffffa03d986d:    0f 18 09                 prefetcht0 (%rcx)
>  ffffffffa03d9870:    48 8b 48 e0              mov    -0x20(%rax),%rcx
>  ffffffffa03d9874:    48 89 0a                 mov    %rcx,(%rdx)
>  ffffffffa03d9877:    48 8b 48 e8              mov    -0x18(%rax),%rcx
>  ffffffffa03d987b:    48 89 4a 08              mov    %rcx,0x8(%rdx)
>  ffffffffa03d987f:    48 8b 48 f0              mov    -0x10(%rax),%rcx
>  ffffffffa03d9883:    48 89 4a 10              mov    %rcx,0x10(%rdx)
>  ffffffffa03d9887:    48 8b 48 f8              mov    -0x8(%rax),%rcx
>  ffffffffa03d988b:    48 89 4a 18              mov    %rcx,0x18(%rdx)
> 
> 
> Is that you wanted, the "6b6b6b6b6b6b6b6b" ?
> 
> Regards.

Yes, excellent. So now we can detect the problem by comparing
kvm with 6b6b6b6b6b6b6b6b, and print out stuff to understand where
this comes from.
I will prepare such a debugging patch.

For that, could you please tell me which kernel version, exactly, are
you using?


-- 
MST

^ permalink raw reply

* Re: Bug inkvm_set_irq
From: Jean-Philippe Menil @ 2011-03-04  9:39 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20110304093500.GA31844@redhat.com>

Le 04/03/2011 10:35, Michael S. Tsirkin a écrit :
> On Fri, Mar 04, 2011 at 10:22:03AM +0100, Jean-Philippe Menil wrote:
>> Le 03/03/2011 16:55, Michael S. Tsirkin a écrit :
>>> On Thu, Mar 03, 2011 at 04:26:11PM +0100, Jean-Philippe Menil wrote:
>>>> Le 03/03/2011 15:47, Michael S. Tsirkin a écrit :
>>>>> On Tue, Mar 01, 2011 at 03:39:12PM +0100, Jean-Philippe Menil wrote:
>>>>>> 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.
>>>>> Not null, this seems to be invalid.
>>>>> I suspect use after free where the kvm pointer is
>>>>> pointing at some random memory. Use after free?
>>>>> Could you please try enabling a slab debugger,
>>>>> recompile and rerun the test?
>>>>>
>>>> Hi,
>>>>
>>>> I'm not sure to activate the right thing.
>>>> Is that what you want?
>>>>
>>>> CONFIG_SLAB=y
>>>> CONFIG_SLABINFO=y
>>>> CONFIG_DEBUG_SLAB=y
>>>> CONFIG_DEBUG_SLAB_LEAK=y
>>>>
>>>> Regards.
>>> Yes, maybe disable SLAB_LEAK.
>>>
>>>> -- 
>>>> 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 kvm" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Hi,
>>
>> so this time, here is what markup_oops says:
>>
>> root@ayrshire:~# cat oops-0403.txt | perl markup_oops.pl -m
>> /lib/modules/2.6.37.2.999-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko
>> /boot/vmlinuz-2.6.37.2.999-dsiun-110105+
>> vmaoffset = 18446744072102948864 ffffffffa03d9811:    48 89 e5
>> mov    %rsp,%rbp
>>   ffffffffa03d9814:    41 57                    push   %r15
>>   ffffffffa03d9816:    41 89 cf                 mov    %ecx,%r15d  |
>> %r15 =>  1  %ecx = 1
>>   ffffffffa03d9819:    41 56                    push   %r14        |
>> %r14 =>  ffffffffa03daa50
>>   ffffffffa03d981b:    41 55                    push   %r13
>>   ffffffffa03d981d:    49 89 fd                 mov    %rdi,%r13   |
>> %edi = 6b6b6b6b6b6b6b6b  %r13 =>  6b6b6b6b6b6b6b6b
>>   ffffffffa03d9820:    41 54                    push   %r12        |
>> %r12 =>  6b6b6b6b6b6b6b6b
>>   ffffffffa03d9822:    53                       push   %rbx
>>   ffffffffa03d9823:    89 d3                    mov    %edx,%ebx   |
>> %ebx =>  6b6b6b6b
>>   ffffffffa03d9825:    48 81 ec a8 00 00 00     sub    $0xa8,%rsp
>>   ffffffffa03d982c:    8b 15 00 00 00 00        mov    0x0(%rip),%edx
>> # ffffffffa03d9832<kvm_set_irq+0x22>
>>   ffffffffa03d9832:    89 b5 3c ff ff ff        mov
>> %esi,-0xc4(%rbp) |  %esi = 0
>>   ffffffffa03d9838:    85 d2                    test   %edx,%edx   |
>> %edx =>  0
>>   ffffffffa03d983a:    0f 85 d5 00 00 00        jne
>> ffffffffa03d9915<kvm_set_irq+0x105>
>> *ffffffffa03d9840:    49 8b 85 58 24 00 00     mov
>> 0x2458(%r13),%rax |  %eax = 0  %r13 = 6b6b6b6b6b6b6b6b<--- faulting
>> instruction
>>   ffffffffa03d9847:    3b 98 28 01 00 00        cmp    0x128(%rax),%ebx
>>   ffffffffa03d984d:    73 61                    jae
>> ffffffffa03d98b0<kvm_set_irq+0xa0>
>>   ffffffffa03d984f:    89 db                    mov    %ebx,%ebx
>>   ffffffffa03d9851:    48 8b 84 d8 30 01 00     mov
>> 0x130(%rax,%rbx,8),%rax
>>   ffffffffa03d9858:    00
>>   ffffffffa03d9859:    48 85 c0                 test   %rax,%rax
>>   ffffffffa03d985c:    74 52                    je
>> ffffffffa03d98b0<kvm_set_irq+0xa0>
>>   ffffffffa03d985e:    48 8d 95 40 ff ff ff     lea    -0xc0(%rbp),%rdx
>>   ffffffffa03d9865:    31 db                    xor    %ebx,%ebx
>>   ffffffffa03d9867:    48 8b 08                 mov    (%rax),%rcx
>>   ffffffffa03d986a:    83 c3 01                 add    $0x1,%ebx
>>   ffffffffa03d986d:    0f 18 09                 prefetcht0 (%rcx)
>>   ffffffffa03d9870:    48 8b 48 e0              mov    -0x20(%rax),%rcx
>>   ffffffffa03d9874:    48 89 0a                 mov    %rcx,(%rdx)
>>   ffffffffa03d9877:    48 8b 48 e8              mov    -0x18(%rax),%rcx
>>   ffffffffa03d987b:    48 89 4a 08              mov    %rcx,0x8(%rdx)
>>   ffffffffa03d987f:    48 8b 48 f0              mov    -0x10(%rax),%rcx
>>   ffffffffa03d9883:    48 89 4a 10              mov    %rcx,0x10(%rdx)
>>   ffffffffa03d9887:    48 8b 48 f8              mov    -0x8(%rax),%rcx
>>   ffffffffa03d988b:    48 89 4a 18              mov    %rcx,0x18(%rdx)
>>
>>
>> Is that you wanted, the "6b6b6b6b6b6b6b6b" ?
>>
>> Regards.
> Yes, excellent. So now we can detect the problem by comparing
> kvm with 6b6b6b6b6b6b6b6b, and print out stuff to understand where
> this comes from.
> I will prepare such a debugging patch.
>
> For that, could you please tell me which kernel version, exactly, are
> you using?
>
>
Yes, it's a 2.6.37.2 kernel.

Thanks a lot.


-- 
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: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
From: KY Srinivasan @ 2011-03-04 15:18 UTC (permalink / raw)
  To: Greg KH
  Cc: Greg KH, linux-kernel@vger.kernel.org,
	devel@linuxdriverproject.org, virtualization@lists.osdl.org,
	Haiyang Zhang, Hank Janssen
In-Reply-To: <20110303212212.GA507@suse.de>



> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Thursday, March 03, 2011 4:22 PM
> To: KY Srinivasan
> Cc: Greg KH; 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 Thu, Mar 03, 2011 at 09:16:29PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@suse.de]
> > > Sent: Thursday, March 03, 2011 1:10 AM
> > > To: KY Srinivasan
> > > Cc: Greg KH; 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 Thu, Mar 03, 2011 at 02:50:00AM +0000, KY Srinivasan wrote:
> > > > > > > "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.
> > > >
> > > > Greg, the temporary name happens to be the name currently in use in the
> > > > code - this is not the name I introduced.
> > >
> > > There is not a "struct driver_context" in the code that I see today, or
> > > am I missing something?  That's my objection here, please don't use that
> > > name, it's not valid for a subsystem to use, even for a tiny bit.
> >
> > Look at the file vmbus.h  you will see struct driver_context. This has
> > been there for as long as I have seen this code.
> 
> Ok, I am rightly corrected, I totally missed that, you are right.
> 
> Feel free to resend after addressing the other issues.
> 
> I'll fix up the hv_mouse driver, you don't have to worry about that one
> if you don't want to, just ignore it please.

Greg, I am working on a patch-set that hopefully will address all
the concerns that were raised. As part of this effort, I will also
deal with the mouse driver. I should have these patches out next week.
Thanks for your patience here.

Regards,

K. Y 

^ permalink raw reply

* [PATCH] vhost: copy_from_user -> __copy_from_user
From: Michael S. Tsirkin @ 2011-03-06 11:33 UTC (permalink / raw)
  To: Michael S. Tsirkin, kvm, virtualization, netdev, linux-kernel

copy_from_user is pretty high on perf top profile,
replacing it with __copy_from_user helps.
It's also safe because we do access_ok checks during setup.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/vhost/vhost.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ade0568..01701b8 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1153,7 +1153,7 @@ int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq,
 			       i, vq->num, head);
 			return -EINVAL;
 		}
-		ret = copy_from_user(&desc, vq->desc + i, sizeof desc);
+		ret = __copy_from_user(&desc, vq->desc + i, sizeof desc);
 		if (unlikely(ret)) {
 			vq_err(vq, "Failed to get descriptor: idx %d addr %p\n",
 			       i, vq->desc + i);
-- 
1.7.3.2.91.g446ac

^ permalink raw reply related

* IGCC'11 deadline in 2 weeks (2nd International Gree Computing Conference)
From: Ming Zhao @ 2011-03-06 12:48 UTC (permalink / raw)
  To: virtualization

Dear Colleague,

On behalf of the Conference Organizing Committee, we would like to
inform you of the:

SECOND INTERNATIONAL GREEN COMPUTING CONFERENCE (IGCC'11):
Moto: Sustainable Computing and Computing for Sustainability

Orlando, Florida, USA, (The Holiday Inn at Disneyworld) July 25-28,
2011, www.green-conf.org

The Second International Green Computing Conference, technically
cosponsored by IEEE, addresses key issues and topics related to energy
efficiency in computing and promoting environmentally friendly
computer technologies and systems. The conference provides a forum for
discussing, sharing and investigating the state-of-the-art for all
aspects of green computing, which include energy-efficient use of
computers, design of algorithms and systems for
environmentally-friendly computer technologies, and a wide range of
related topics. The conference seeks papers with unpublished material
and previously unsubmitted material pertaining to the design and use
of sustainable hardware and software systems, algorithms, and
applications. The conference will hold forums, workshops, and
tutorials on hot topics related to how the carbon footprint of
computing can be reduced and how computers can contribute to the
environment and overall well-being of the planet.

Topics of interest are:

* Power-aware software
* Configurable and renewable energy
* Code profiling and transformation for power management
* Low power electronics
* Power-aware middleware
* Embedded systems, ASICs and FPGSs
* Power-efficient architectures and chip designs
* Power leakage and dissipation
* Resource management to optimize performance and power
* Power implications for portable and mobile computing
* Runtime systems that assist in power saving
* Power aware networking
* Models for collective optimization of power and performance
* Reliability of Power-aware computers
* Monitoring tools for power and performance
* Use of sensors for climate monitoring
* Algorithms for reduced power, energy and heat
* Smart control for eco-friendly buildings
* Power-aware applications
* Thermal control of data centers
* Static and dynamic data allocation for distributed servers
* Energy recycling
* Efficient circuit design for energy harvesting
* Energy efficient power and cooling infrastructure
* Power efficient cluster and enterprise computing
* Configurable and renewable energy
* Component level power management, e.g., memory, disk.
* Low power electronics
* Power leakage and dissipation
* Embedded systems, ASICs and FPGSs


Review of Manuscripts

All submitted manuscripts will be reviewed by the conference Technical
Committee and evaluated on correctness, originality, technical
strength, significance, quality of presentation, interest and
relevance to the conference scope. Papers must be received by the
submission deadline to be considered for inclusion in the conference.


Best Paper Awards

Awards will be given for best papers in different categories.


Important Dates:

* Submission Deadline: Monday, March 18, 2011, 11:59pm, U.S. Eastern
Daylight Time
* Paper notification: May 13, 2011
* Camera ready papers due: June 6, 2011

The final appearance of the accepted papers in the official conference
proceedings (through IEEE Digital Library) is contingent on two
conditions: (1) that at least one author of an accepted paper
registers for the conference at the time of the submission of the
final manuscript and (2) that one of the authors presents the paper at
the conference in person. Full details of the conference may be found
at the conference website: www.green-conf.org. To directly access
submission please go to
https://www.easychair.org/login.cgi?conf=igcc101


Workshops

Several workshops on related topics will be held in conjunction with
the Green Computing conference.

1. Energy Consumption and Reliability of Storage Systems; Co-Chairs:
Ali Butt, Virginia Tech.; and Chris Gniady, U. of Arizona;
2. Power Measurement and Profiling; Weisong Shi (Wayne State);
3. Thermal Management Dhireesha Kudithipudi, RIT;
4. Low Power System on Chip, Partha Pande, WSU.


Committees

* General Co-Chairs: Ishfaq Ahmad, University of Texas at Arlington,
and Sanjay Ranka, University of Florida.

* Technical Committee: Co-Chairs: Israel Koren, University of
Massachusetts, and Daniel Mosse University of Pittsburgh;

* Members: Tarek Abdelzaher, UIUC; David Albonesi, Cornell; Ahmed
Amer, Santa Clara Univ; Martin Arlitt, HP Labs and University of
Calgary; Hakan Aydin, George Mason Univ; Luca Benini, Univ. of
Bologna; Ricardo Bianchini, Rutgers; John Carter, IBM; Ranveer
Chandra, Microsoft Research; Vince Freeh, NCSU; Rong Ge, Marquette
Univ; Sameh Gobriel, Intel Research; Rajesh Gupta, UCSD; Tohru
Ishihara, Kyushu Univ; Randy Katz, UC Berkeley; Uli Kremer, Rutgers;
C. Mani Krishna, U Mass.; Rakesh Kumar, UIUC;  Alvin Lebeck, Duke
University; Yann-Hang Lee, ASU; Julius Leite, UFF/Brazil;  Xue Liu,
UNL; Satoshi Matsouka, TITECH-Japan; Rami G. Melhem, University of
Pittsburgh; Avi Mendelson. Microsoft, Israel; Bruce Normad, Lawrence
Berkeley Lab; Lucy Nowell, DOE: Partha Ranganathan, HP Labs; Tajana
Rosing, UCSD; Efi Rotem, Intel:; Anthony Rowe, CMU; Martin Schulz,
LLNL; Ankur Srivastva, University of Maryland;  Osman Unsal, BSC,
Barcelona; Shengquan Wang, Michigan; Dakai Zhu, UTSA.

* Workshop Chair: Sanjay Ranka, University of Florida, USA.

* Publication Chair: Saeed Rajput, Nova Southeastern University, USA

* Publicity Co-Chairs: Javier Alonso, Technical University of
Catalonia, Spain; Ming Zhao Florida International University, USA.

* Steering Committee: Behrooz Shirazi (Co-Chair), Washington State
University, USA; Ishfaq Ahmad (Co-Chair), University of Texas at
Arlington, USA; Rajesh Gupta, University of California at San Diego,
USA; Sandeep Gupta, Arizona State University, USA; Ali Hurson,
Missouri University of Science and Technology, USA; Ashfaq Khokhar,
University of Illinois at Chicago, USA; Francis Lau, University of
Hong Kong, China; Sanjay Ranka, University of Florida, USA.


Keynote Speaker: David Culler, UC Berkeley.


We look forward to receiving your submission(s), and hope you will be
able to join us in Orlando in July.


Sincerely,

IGCC'11 Organizing Committee

^ permalink raw reply

* Re: [PATCH] vhost: copy_from_user -> __copy_from_user
From: David Miller @ 2011-03-07  2:03 UTC (permalink / raw)
  To: mst; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20110306113349.GA24333@redhat.com>

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Sun, 6 Mar 2011 13:33:49 +0200

> copy_from_user is pretty high on perf top profile,
> replacing it with __copy_from_user helps.
> It's also safe because we do access_ok checks during setup.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Is Rusty going to take this or should I?

^ permalink raw reply

* Re: [PATCH] vhost: copy_from_user -> __copy_from_user
From: Michael S. Tsirkin @ 2011-03-07  6:30 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20110306.180339.193718218.davem@davemloft.net>

On Sun, Mar 06, 2011 at 06:03:39PM -0800, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Sun, 6 Mar 2011 13:33:49 +0200
> 
> > copy_from_user is pretty high on perf top profile,
> > replacing it with __copy_from_user helps.
> > It's also safe because we do access_ok checks during setup.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Is Rusty going to take this or should I?

Sorry about not making this clear.  I'll give it a day or two for review
then put it on the vhost tree myself.
Thanks!

^ permalink raw reply

* [PATCH 00/16] Staging: hv: Consolidate driver and device abstractions
From: K. Y. Srinivasan @ 2011-03-07 21:01 UTC (permalink / raw)
  To: kys, gregkh, linux-kernel, devel, virtualization

Hyper-V has maintained both its class independent driver and 
device state in two independent data structures:
Driver state: struct driver_context (vmbus.h) and struct hv_driver
(vmbus_api.h)
Device state: struct vm_device (vmbus.h) and struct hv_device (vmbus_api.h)

While sruct driver_context and struct vm_device embed generic 
Linux abstractions of struct device_driver and struct device respectively; 
the lower level hyperv abstraction: struct hv_driver and struct hv_device
have maintained state needed to communicate with the host. This partitioning
made sense at a point in time when it was not clear how much of 
the hypervisor interaction would be open sourced. Given where we are
today, there is no reason to keep this layering. This patchset 
consolidates all the driver state into a single structure: 
struct hv_driver while all the device state is consolidated into
a single structure: struct hv_device. This consolidation simplifies
the code while simultaneously getting rid of redundant state -
for instance in the current code, both struct driver_context and
struct hv_driver both have state to represent the class as well as
instance id. We do this consolidation by moving state from higher
level structures (struct driver_context and struct vm_device) to
lower level structures (struct hv_driver and hv_device) respectively.

While it has not been a goal of this effort to cleanup structure and 
variable names, this consolidation effort has resulted in some cleanup
with regards to structure and variable names: a) Some of the badly
named structures have been eliminated (struct driver_context etc.) 
while hopefully all newly introduced names are acceptable.  

Patches 1 through 11 deal with consolidating the device driver
state while patches 12 through 16  deal with consolidating
device state.


Regards,

K. Y

^ permalink raw reply

* [PATCH 01/16] Staging: hv: Use generic device_driver probe function
From: K. Y. Srinivasan @ 2011-03-07 21:20 UTC (permalink / raw)
  To: kys, gregkh, linux-kernel, devel, virtualization
  Cc: Abhishek Kane, Haiyang Zhang, Hank Janssen

In preparation for moving all the state from struct driver_context
to struct hv_driver, eliminate the probe() function from
struct driver_context and use generic device_driver probe
function.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/blkvsc_drv.c  |    2 +-
 drivers/staging/hv/hv_mouse.c    |    2 +-
 drivers/staging/hv/netvsc_drv.c  |    2 +-
 drivers/staging/hv/storvsc_drv.c |    2 +-
 drivers/staging/hv/vmbus_drv.c   |    5 +++--
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 36a0adb..0dff557 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -186,7 +186,7 @@ static int blkvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	memcpy(&drv_ctx->class_id, &storvsc_drv_obj->base.dev_type,
 	       sizeof(struct hv_guid));
 
-	drv_ctx->probe = blkvsc_probe;
+	drv_ctx->driver.probe = blkvsc_probe;
 	drv_ctx->remove = blkvsc_remove;
 	drv_ctx->shutdown = blkvsc_shutdown;
 
diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
index 1aaaef4..4a25f3b 100644
--- a/drivers/staging/hv/hv_mouse.c
+++ b/drivers/staging/hv/hv_mouse.c
@@ -1021,7 +1021,7 @@ static int __init mousevsc_init(void)
 	memcpy(&drv_ctx->class_id, &input_drv_obj->Base.dev_type,
 	       sizeof(struct hv_guid));
 
-	drv_ctx->probe = mousevsc_probe;
+	drv_ctx->driver.probe = mousevsc_probe;
 	drv_ctx->remove = mousevsc_remove;
 
 	/* The driver belongs to vmbus */
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 03f9740..083f0d3 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -511,7 +511,7 @@ static int netvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	memcpy(&drv_ctx->class_id, &net_drv_obj->base.dev_type,
 	       sizeof(struct hv_guid));
 
-	drv_ctx->probe = netvsc_probe;
+	drv_ctx->driver.probe = netvsc_probe;
 	drv_ctx->remove = netvsc_remove;
 
 	/* The driver belongs to vmbus */
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index a8427ff..3fe4bdb 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -164,7 +164,7 @@ static int storvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	memcpy(&drv_ctx->class_id, &storvsc_drv_obj->base.dev_type,
 	       sizeof(struct hv_guid));
 
-	drv_ctx->probe = storvsc_probe;
+	drv_ctx->driver.probe = storvsc_probe;
 	drv_ctx->remove = storvsc_remove;
 
 	/* The driver belongs to vmbus */
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 459c707..fee2a8f 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -919,8 +919,9 @@ static int vmbus_probe(struct device *child_device)
 			device_to_vm_device(child_device);
 
 	/* Let the specific open-source driver handles the probe if it can */
-	if (driver_ctx->probe) {
-		ret = device_ctx->probe_error = driver_ctx->probe(child_device);
+	if (driver_ctx->driver.probe) {
+		ret = device_ctx->probe_error =
+		driver_ctx->driver.probe(child_device);
 		if (ret != 0) {
 			DPRINT_ERR(VMBUS_DRV, "probe() failed for device %s "
 				   "(%p) on driver %s (%d)...",
-- 
1.5.5.6

^ permalink raw reply related

* [PATCH 02/16] Staging: hv: Use generic device_driver remove function
From: K. Y. Srinivasan @ 2011-03-07 21:21 UTC (permalink / raw)
  To: kys, gregkh, linux-kernel, devel, virtualization
  Cc: Haiyang Zhang, Abhishek Kane

In preparation for moving all the state from struct driver_context
to struct hv_driver, eliminate the remove() function from
struct driver_context and use generic device_driver remove()
function.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/blkvsc_drv.c  |    2 +-
 drivers/staging/hv/hv_mouse.c    |    2 +-
 drivers/staging/hv/netvsc_drv.c  |    2 +-
 drivers/staging/hv/storvsc_drv.c |    2 +-
 drivers/staging/hv/vmbus_drv.c   |    4 ++--
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 0dff557..3e86a56 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -187,7 +187,7 @@ static int blkvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	       sizeof(struct hv_guid));
 
 	drv_ctx->driver.probe = blkvsc_probe;
-	drv_ctx->remove = blkvsc_remove;
+	drv_ctx->driver.remove = blkvsc_remove;
 	drv_ctx->shutdown = blkvsc_shutdown;
 
 	/* The driver belongs to vmbus */
diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
index 4a25f3b..6e2a937 100644
--- a/drivers/staging/hv/hv_mouse.c
+++ b/drivers/staging/hv/hv_mouse.c
@@ -1022,7 +1022,7 @@ static int __init mousevsc_init(void)
 	       sizeof(struct hv_guid));
 
 	drv_ctx->driver.probe = mousevsc_probe;
-	drv_ctx->remove = mousevsc_remove;
+	drv_ctx->driver.remove = mousevsc_remove;
 
 	/* The driver belongs to vmbus */
 	vmbus_child_driver_register(drv_ctx);
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 083f0d3..e2013b5 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -512,7 +512,7 @@ static int netvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	       sizeof(struct hv_guid));
 
 	drv_ctx->driver.probe = netvsc_probe;
-	drv_ctx->remove = netvsc_remove;
+	drv_ctx->driver.remove = netvsc_remove;
 
 	/* The driver belongs to vmbus */
 	ret = vmbus_child_driver_register(drv_ctx);
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 3fe4bdb..3855271 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -165,7 +165,7 @@ static int storvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	       sizeof(struct hv_guid));
 
 	drv_ctx->driver.probe = storvsc_probe;
-	drv_ctx->remove = storvsc_remove;
+	drv_ctx->driver.remove = storvsc_remove;
 
 	/* The driver belongs to vmbus */
 	ret = vmbus_child_driver_register(drv_ctx);
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index fee2a8f..4df2be0 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -964,8 +964,8 @@ static int vmbus_remove(struct device *child_device)
 		 * Let the specific open-source driver handles the removal if
 		 * it can
 		 */
-		if (driver_ctx->remove) {
-			ret = driver_ctx->remove(child_device);
+		if (driver_ctx->driver.remove) {
+			ret = driver_ctx->driver.remove(child_device);
 		} else {
 			DPRINT_ERR(VMBUS_DRV,
 				   "remove() method not set for driver - %s",
-- 
1.5.5.6

^ permalink raw reply related

* [PATCH 03/16] Staging: hv: Use generic device_driver shutdown function
From: K. Y. Srinivasan @ 2011-03-07 21:22 UTC (permalink / raw)
  To: kys, gregkh, linux-kernel, devel, virtualization
  Cc: Haiyang Zhang, Abhishek Kane

In preparation for moving all the state from struct driver_context
to struct hv_driver, eliminate the shutdown() function from
struct driver_context and use generic device_driver shutdown()
function.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/blkvsc_drv.c |    2 +-
 drivers/staging/hv/vmbus_drv.c  |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 3e86a56..9ccd5dc 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -188,7 +188,7 @@ static int blkvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 
 	drv_ctx->driver.probe = blkvsc_probe;
 	drv_ctx->driver.remove = blkvsc_remove;
-	drv_ctx->shutdown = blkvsc_shutdown;
+	drv_ctx->driver.shutdown = blkvsc_shutdown;
 
 	/* The driver belongs to vmbus */
 	ret = vmbus_child_driver_register(drv_ctx);
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 4df2be0..6ef5bee 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -1000,8 +1000,8 @@ static void vmbus_shutdown(struct device *child_device)
 	driver_ctx = driver_to_driver_context(child_device->driver);
 
 	/* Let the specific open-source driver handles the removal if it can */
-	if (driver_ctx->shutdown)
-		driver_ctx->shutdown(child_device);
+	if (driver_ctx->driver.shutdown)
+		driver_ctx->driver.shutdown(child_device);
 
 	return;
 }
-- 
1.5.5.6

^ permalink raw reply related

* [PATCH 04/16] Staging: hv: Remove unnecessary function pointers in driver_context
From: K. Y. Srinivasan @ 2011-03-07 21:23 UTC (permalink / raw)
  To: kys, gregkh, linux-kernel, devel, virtualization
  Cc: Haiyang Zhang, Abhishek Kane

Get rid of the unnecessary function pointers for probe(),
remove() and shutdown() from struct driver_context.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus.h |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/hv/vmbus.h b/drivers/staging/hv/vmbus.h
index 42f2adb..8bcfb40 100644
--- a/drivers/staging/hv/vmbus.h
+++ b/drivers/staging/hv/vmbus.h
@@ -33,14 +33,6 @@ struct driver_context {
 
 	struct device_driver driver;
 
-	/*
-	 * Use these methods instead of the struct device_driver so 2.6 kernel
-	 * stops complaining
-	 * TODO - fix this!
-	 */
-	int (*probe)(struct device *);
-	int (*remove)(struct device *);
-	void (*shutdown)(struct device *);
 };
 
 struct vm_device {
-- 
1.5.5.6

^ permalink raw reply related

* [PATCH 05/16] Staging: hv: Change the signature for vmbus_child_driver_register
From: K. Y. Srinivasan @ 2011-03-07 21:23 UTC (permalink / raw)
  To: kys, gregkh, linux-kernel, devel, virtualization
  Cc: Haiyang Zhang, Abhishek Kane

In preparation for moving the element driver from the
struct driver_context to struct hv_driver, change the
signature for the function vmbus_child_driver_register()
to take a pointer to struct device_driver.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/blkvsc_drv.c  |    2 +-
 drivers/staging/hv/hv_mouse.c    |    2 +-
 drivers/staging/hv/netvsc_drv.c  |    2 +-
 drivers/staging/hv/storvsc_drv.c |    2 +-
 drivers/staging/hv/vmbus.h       |    2 +-
 drivers/staging/hv/vmbus_drv.c   |   11 +++++------
 6 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 9ccd5dc..d5e1881 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -191,7 +191,7 @@ static int blkvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	drv_ctx->driver.shutdown = blkvsc_shutdown;
 
 	/* The driver belongs to vmbus */
-	ret = vmbus_child_driver_register(drv_ctx);
+	ret = vmbus_child_driver_register(&drv_ctx->driver);
 
 	return ret;
 }
diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
index 6e2a937..d2290f8 100644
--- a/drivers/staging/hv/hv_mouse.c
+++ b/drivers/staging/hv/hv_mouse.c
@@ -1025,7 +1025,7 @@ static int __init mousevsc_init(void)
 	drv_ctx->driver.remove = mousevsc_remove;
 
 	/* The driver belongs to vmbus */
-	vmbus_child_driver_register(drv_ctx);
+	vmbus_child_driver_register(&drv_ctx->driver);
 
 	return 0;
 }
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index e2013b5..a50a9a6 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -515,7 +515,7 @@ static int netvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	drv_ctx->driver.remove = netvsc_remove;
 
 	/* The driver belongs to vmbus */
-	ret = vmbus_child_driver_register(drv_ctx);
+	ret = vmbus_child_driver_register(&drv_ctx->driver);
 
 	return ret;
 }
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 3855271..8ab8df5 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -168,7 +168,7 @@ static int storvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	drv_ctx->driver.remove = storvsc_remove;
 
 	/* The driver belongs to vmbus */
-	ret = vmbus_child_driver_register(drv_ctx);
+	ret = vmbus_child_driver_register(&drv_ctx->driver);
 
 	return ret;
 }
diff --git a/drivers/staging/hv/vmbus.h b/drivers/staging/hv/vmbus.h
index 8bcfb40..2295fe2 100644
--- a/drivers/staging/hv/vmbus.h
+++ b/drivers/staging/hv/vmbus.h
@@ -61,7 +61,7 @@ static inline struct driver_context *driver_to_driver_context(struct device_driv
 
 
 /* Vmbus interface */
-int vmbus_child_driver_register(struct driver_context *driver_ctx);
+int vmbus_child_driver_register(struct device_driver *drv);
 void vmbus_child_driver_unregister(struct driver_context *driver_ctx);
 
 extern struct completion hv_channel_ready;
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 6ef5bee..685376b 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -617,9 +617,8 @@ static void vmbus_bus_exit(void)
 
 /**
  * vmbus_child_driver_register() - Register a vmbus's child driver
- * @driver_ctx:        Pointer to driver structure you want to register
+ * @drv:        Pointer to driver structure you want to register
  *
- * @driver_ctx is of type &struct driver_context
  *
  * Registers the given driver with Linux through the 'driver_register()' call
  * And sets up the hyper-v vmbus handling for this driver.
@@ -627,17 +626,17 @@ static void vmbus_bus_exit(void)
  *
  * Mainly used by Hyper-V drivers.
  */
-int vmbus_child_driver_register(struct driver_context *driver_ctx)
+int vmbus_child_driver_register(struct device_driver *drv)
 {
 	int ret;
 
 	DPRINT_INFO(VMBUS_DRV, "child driver (%p) registering - name %s",
-		    driver_ctx, driver_ctx->driver.name);
+		    drv, drv->name);
 
 	/* The child driver on this vmbus */
-	driver_ctx->driver.bus = &vmbus_drv.bus;
+	drv->bus = &vmbus_drv.bus;
 
-	ret = driver_register(&driver_ctx->driver);
+	ret = driver_register(drv);
 
 	vmbus_request_offers();
 
-- 
1.5.5.6

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox