Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Fwd: [PATCH]: Fix crash in virtio_blk during modprobe ; rmmod ; modprobe]
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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
