[Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]

All of lore.kernel.org
 help / color / mirror / Atom feed

* [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
@ 2008-05-21 13:12 Chris Lalancette
  2008-05-22 11:33 ` Rusty Russell
  2008-05-22 11:33 ` Rusty Russell
  0 siblings, 2 replies; 11+ messages in thread
From: Chris Lalancette @ 2008-05-21 13:12 UTC (permalink / raw)
  To: virtualization; +Cc: kvm

[-- Attachment #1: Type: text/plain, Size: 49 bytes --]

(sent to the kvm mailing list erroneously first)

[-- Attachment #2: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe.eml --]
[-- Type: message/rfc822, Size: 4125 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 209 bytes --]

Fix a modprobe virtio_blk ; rmmod virtio_blk ; modprobe virtio_blk crash; this
was basically because we weren't doing "del_gendisk()" in the remove path.

Signed-off-by: Chris Lalancette <clalance@redhat.com>

[-- Attachment #2.1.2: virtio-blk-rmmod-crash.patch --]
[-- Type: text/x-patch, Size: 734 bytes --]

commit 9ae82ccb26be0155ad81b2630090e85639a0dc56
Author: Chris Lalancette <clalance@redhat.com>
Date:   Fri May 16 15:31:06 2008 -0400

    Fix a modprobe virtio_blk ; rmmod virtio_blk ; modprobe virtio_blk crash;
    this was basically because we weren't doing "del_gendisk()" in the remove
    path.

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 4962e62..c678ac5 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev)
 	vdev->config->reset(vdev);
 
 	blk_cleanup_queue(vblk->disk->queue);
+	del_gendisk(vblk->disk);
 	put_disk(vblk->disk);
 	unregister_blkdev(major, "virtblk");
 	mempool_destroy(vblk->pool);

[-- 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 related	[flat|nested] 11+ messages in thread

* Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
  2008-05-21 13:12 [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe] Chris Lalancette
@ 2008-05-22 11:33 ` Rusty Russell
  2008-05-22 11:33 ` Rusty Russell
  1 sibling, 0 replies; 11+ messages in thread
From: Rusty Russell @ 2008-05-22 11:33 UTC (permalink / raw)
  To: Chris Lalancette; +Cc: Jens Axboe, kvm, virtualization

On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote:
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 4962e62..c678ac5 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev)
>         vdev->config->reset(vdev);
>  
>         blk_cleanup_queue(vblk->disk->queue);
> +       del_gendisk(vblk->disk);
>         put_disk(vblk->disk);
>         unregister_blkdev(major, "virtblk");
>         mempool_destroy(vblk->pool);

Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to test 
here, it's my root block dev).  Other drivers seem to do blk_cleanup_queue() 
*after* del_gendisk: does it matter?

Jens CC'd: he's gentle with my dumb questions...
Rusty.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
  2008-05-21 13:12 [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe] Chris Lalancette
  2008-05-22 11:33 ` Rusty Russell
@ 2008-05-22 11:33 ` Rusty Russell
  2008-05-22 11:35   ` Jens Axboe
  2008-05-22 11:35   ` Jens Axboe
  1 sibling, 2 replies; 11+ messages in thread
From: Rusty Russell @ 2008-05-22 11:33 UTC (permalink / raw)
  To: Chris Lalancette; +Cc: virtualization, kvm, Jens Axboe

On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote:
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 4962e62..c678ac5 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev)
>         vdev->config->reset(vdev);
>  
>         blk_cleanup_queue(vblk->disk->queue);
> +       del_gendisk(vblk->disk);
>         put_disk(vblk->disk);
>         unregister_blkdev(major, "virtblk");
>         mempool_destroy(vblk->pool);

Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to test 
here, it's my root block dev).  Other drivers seem to do blk_cleanup_queue() 
*after* del_gendisk: does it matter?

Jens CC'd: he's gentle with my dumb questions...
Rusty.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
  2008-05-22 11:33 ` Rusty Russell
@ 2008-05-22 11:35   ` Jens Axboe
  2008-05-22 12:03     ` Chris Lalancette
                       ` (3 more replies)
  2008-05-22 11:35   ` Jens Axboe
  1 sibling, 4 replies; 11+ messages in thread
From: Jens Axboe @ 2008-05-22 11:35 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Chris Lalancette, virtualization, kvm

On Thu, May 22 2008, Rusty Russell wrote:
> On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote:
> > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> > index 4962e62..c678ac5 100644
> > --- a/drivers/block/virtio_blk.c
> > +++ b/drivers/block/virtio_blk.c
> > @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev)
> >         vdev->config->reset(vdev);
> >  
> >         blk_cleanup_queue(vblk->disk->queue);
> > +       del_gendisk(vblk->disk);
> >         put_disk(vblk->disk);
> >         unregister_blkdev(major, "virtblk");
> >         mempool_destroy(vblk->pool);
> 
> Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to
> test here, it's my root block dev).  Other drivers seem to do
> blk_cleanup_queue() *after* del_gendisk: does it matter?
> 
> Jens CC'd: he's gentle with my dumb questions...  Rusty.

del_gendisk() can generate IO, so it would seem safer to do that
_before_ putting the queue reference :-)

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
  2008-05-22 11:35   ` Jens Axboe
@ 2008-05-22 12:03     ` Chris Lalancette
  2008-05-22 12:07       ` Jens Axboe
  2008-05-22 12:07       ` Jens Axboe
  2008-05-22 12:03     ` Chris Lalancette
                       ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Chris Lalancette @ 2008-05-22 12:03 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Rusty Russell, virtualization, kvm

Jens Axboe wrote:
> On Thu, May 22 2008, Rusty Russell wrote:
>> On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote:
>>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>>> index 4962e62..c678ac5 100644
>>> --- a/drivers/block/virtio_blk.c
>>> +++ b/drivers/block/virtio_blk.c
>>> @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev)
>>>         vdev->config->reset(vdev);
>>>  
>>>         blk_cleanup_queue(vblk->disk->queue);
>>> +       del_gendisk(vblk->disk);
>>>         put_disk(vblk->disk);
>>>         unregister_blkdev(major, "virtblk");
>>>         mempool_destroy(vblk->pool);
>> Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to
>> test here, it's my root block dev).  Other drivers seem to do
>> blk_cleanup_queue() *after* del_gendisk: does it matter?
>>
>> Jens CC'd: he's gentle with my dumb questions...  Rusty.
> 
> del_gendisk() can generate IO, so it would seem safer to do that
> _before_ putting the queue reference :-)
> 

Ah, good to know.  Out of curiousity, why would del_gendisk() generate
additional I/O?

Chris Lalancette

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod  ; modprobe]
  2008-05-22 12:03     ` Chris Lalancette
@ 2008-05-22 12:07       ` Jens Axboe
  2008-05-22 12:07       ` Jens Axboe
  1 sibling, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2008-05-22 12:07 UTC (permalink / raw)
  To: Chris Lalancette; +Cc: kvm, virtualization

On Thu, May 22 2008, Chris Lalancette wrote:
> Jens Axboe wrote:
> > On Thu, May 22 2008, Rusty Russell wrote:
> >> On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote:
> >>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> >>> index 4962e62..c678ac5 100644
> >>> --- a/drivers/block/virtio_blk.c
> >>> +++ b/drivers/block/virtio_blk.c
> >>> @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev)
> >>>         vdev->config->reset(vdev);
> >>>  
> >>>         blk_cleanup_queue(vblk->disk->queue);
> >>> +       del_gendisk(vblk->disk);
> >>>         put_disk(vblk->disk);
> >>>         unregister_blkdev(major, "virtblk");
> >>>         mempool_destroy(vblk->pool);
> >> Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to
> >> test here, it's my root block dev).  Other drivers seem to do
> >> blk_cleanup_queue() *after* del_gendisk: does it matter?
> >>
> >> Jens CC'd: he's gentle with my dumb questions...  Rusty.
> > 
> > del_gendisk() can generate IO, so it would seem safer to do that
> > _before_ putting the queue reference :-)
> > 
> 
> Ah, good to know.  Out of curiousity, why would del_gendisk() generate
> additional I/O?

It does invalidate+sync.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod  ; modprobe]
  2008-05-22 12:03     ` Chris Lalancette
  2008-05-22 12:07       ` Jens Axboe
@ 2008-05-22 12:07       ` Jens Axboe
  1 sibling, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2008-05-22 12:07 UTC (permalink / raw)
  To: Chris Lalancette; +Cc: Rusty Russell, virtualization, kvm

On Thu, May 22 2008, Chris Lalancette wrote:
> Jens Axboe wrote:
> > On Thu, May 22 2008, Rusty Russell wrote:
> >> On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote:
> >>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> >>> index 4962e62..c678ac5 100644
> >>> --- a/drivers/block/virtio_blk.c
> >>> +++ b/drivers/block/virtio_blk.c
> >>> @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev)
> >>>         vdev->config->reset(vdev);
> >>>  
> >>>         blk_cleanup_queue(vblk->disk->queue);
> >>> +       del_gendisk(vblk->disk);
> >>>         put_disk(vblk->disk);
> >>>         unregister_blkdev(major, "virtblk");
> >>>         mempool_destroy(vblk->pool);
> >> Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to
> >> test here, it's my root block dev).  Other drivers seem to do
> >> blk_cleanup_queue() *after* del_gendisk: does it matter?
> >>
> >> Jens CC'd: he's gentle with my dumb questions...  Rusty.
> > 
> > del_gendisk() can generate IO, so it would seem safer to do that
> > _before_ putting the queue reference :-)
> > 
> 
> Ah, good to know.  Out of curiousity, why would del_gendisk() generate
> additional I/O?

It does invalidate+sync.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
  2008-05-22 11:35   ` Jens Axboe
  2008-05-22 12:03     ` Chris Lalancette
@ 2008-05-22 12:03     ` Chris Lalancette
  2008-05-23  1:16     ` Rusty Russell
  2008-05-23  1:16     ` Rusty Russell
  3 siblings, 0 replies; 11+ messages in thread
From: Chris Lalancette @ 2008-05-22 12:03 UTC (permalink / raw)
  To: Jens Axboe; +Cc: kvm, virtualization

Jens Axboe wrote:
> On Thu, May 22 2008, Rusty Russell wrote:
>> On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote:
>>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>>> index 4962e62..c678ac5 100644
>>> --- a/drivers/block/virtio_blk.c
>>> +++ b/drivers/block/virtio_blk.c
>>> @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev)
>>>         vdev->config->reset(vdev);
>>>  
>>>         blk_cleanup_queue(vblk->disk->queue);
>>> +       del_gendisk(vblk->disk);
>>>         put_disk(vblk->disk);
>>>         unregister_blkdev(major, "virtblk");
>>>         mempool_destroy(vblk->pool);
>> Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to
>> test here, it's my root block dev).  Other drivers seem to do
>> blk_cleanup_queue() *after* del_gendisk: does it matter?
>>
>> Jens CC'd: he's gentle with my dumb questions...  Rusty.
> 
> del_gendisk() can generate IO, so it would seem safer to do that
> _before_ putting the queue reference :-)
> 

Ah, good to know.  Out of curiousity, why would del_gendisk() generate
additional I/O?

Chris Lalancette

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
  2008-05-22 11:35   ` Jens Axboe
  2008-05-22 12:03     ` Chris Lalancette
  2008-05-22 12:03     ` Chris Lalancette
@ 2008-05-23  1:16     ` Rusty Russell
  2008-05-23  1:16     ` Rusty Russell
  3 siblings, 0 replies; 11+ messages in thread
From: Rusty Russell @ 2008-05-23  1:16 UTC (permalink / raw)
  To: Jens Axboe; +Cc: kvm, virtualization

On Thursday 22 May 2008 21:35:19 Jens Axboe wrote:
> On Thu, May 22 2008, Rusty Russell wrote:
> > Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to
> > test here, it's my root block dev).  Other drivers seem to do
> > blk_cleanup_queue() *after* del_gendisk: does it matter?
> >
> > Jens CC'd: he's gentle with my dumb questions...  Rusty.
>
> del_gendisk() can generate IO, so it would seem safer to do that
> _before_ putting the queue reference :-)

Thanks Jens.  Chris, I've fixed that up here.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
  2008-05-22 11:35   ` Jens Axboe
                       ` (2 preceding siblings ...)
  2008-05-23  1:16     ` Rusty Russell
@ 2008-05-23  1:16     ` Rusty Russell
  3 siblings, 0 replies; 11+ messages in thread
From: Rusty Russell @ 2008-05-23  1:16 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Chris Lalancette, virtualization, kvm

On Thursday 22 May 2008 21:35:19 Jens Axboe wrote:
> On Thu, May 22 2008, Rusty Russell wrote:
> > Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to
> > test here, it's my root block dev).  Other drivers seem to do
> > blk_cleanup_queue() *after* del_gendisk: does it matter?
> >
> > Jens CC'd: he's gentle with my dumb questions...  Rusty.
>
> del_gendisk() can generate IO, so it would seem safer to do that
> _before_ putting the queue reference :-)

Thanks Jens.  Chris, I've fixed that up here.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
  2008-05-22 11:33 ` Rusty Russell
  2008-05-22 11:35   ` Jens Axboe
@ 2008-05-22 11:35   ` Jens Axboe
  1 sibling, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2008-05-22 11:35 UTC (permalink / raw)
  To: Rusty Russell; +Cc: kvm, virtualization

On Thu, May 22 2008, Rusty Russell wrote:
> On Wednesday 21 May 2008 23:12:39 Chris Lalancette wrote:
> > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> > index 4962e62..c678ac5 100644
> > --- a/drivers/block/virtio_blk.c
> > +++ b/drivers/block/virtio_blk.c
> > @@ -294,6 +294,7 @@ static void virtblk_remove(struct virtio_device *vdev)
> >         vdev->config->reset(vdev);
> >  
> >         blk_cleanup_queue(vblk->disk->queue);
> > +       del_gendisk(vblk->disk);
> >         put_disk(vblk->disk);
> >         unregister_blkdev(major, "virtblk");
> >         mempool_destroy(vblk->pool);
> 
> Thanks Chris, it seems reasonable and I'm sure it works (kinda hard to
> test here, it's my root block dev).  Other drivers seem to do
> blk_cleanup_queue() *after* del_gendisk: does it matter?
> 
> Jens CC'd: he's gentle with my dumb questions...  Rusty.

del_gendisk() can generate IO, so it would seem safer to do that
_before_ putting the queue reference :-)

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-05-23  1:16 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-21 13:12 [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe] Chris Lalancette
2008-05-22 11:33 ` Rusty Russell
2008-05-22 11:33 ` Rusty Russell
2008-05-22 11:35   ` Jens Axboe
2008-05-22 12:03     ` Chris Lalancette
2008-05-22 12:07       ` Jens Axboe
2008-05-22 12:07       ` Jens Axboe
2008-05-22 12:03     ` Chris Lalancette
2008-05-23  1:16     ` Rusty Russell
2008-05-23  1:16     ` Rusty Russell
2008-05-22 11:35   ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.