From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: kvm@vger.kernel.org, Greg Kroah-Hartman <gregkh@suse.de>,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org,
Tejun Heo <tj@kernel.org>
Subject: [PATCHv4] virtio-blk: use ida to allocate disk index
Date: Sun, 30 Oct 2011 21:29:59 +0200 [thread overview]
Message-ID: <20111030192959.GA31143@redhat.com> (raw)
Based on a patch by Mark Wu <dwu@redhat.com>
Current index allocation in virtio-blk is based on a monotonically
increasing variable "index". This means we'll run out of numbers
after a while. It also could cause confusion about the disk
name in the case of hot-plugging disks.
Change virtio-blk to use ida to allocate index, instead.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Changes from Mark's versions:
use the new ida_simple_get
error handling cleanup
fix user after free
Works fine for me.
Jens, could you merge this for 3.2?
That is, unless Rusty complains shortly ...
drivers/block/virtio_blk.c | 30 ++++++++++++++++++++++++------
1 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 079c088..e7a5750 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -8,10 +8,13 @@
#include <linux/scatterlist.h>
#include <linux/string_helpers.h>
#include <scsi/scsi_cmnd.h>
+#include <linux/idr.h>
#define PART_BITS 4
-static int major, index;
+static int major;
+static DEFINE_IDA(vd_index_ida);
+
struct workqueue_struct *virtblk_wq;
struct virtio_blk
@@ -35,6 +38,9 @@ struct virtio_blk
/* What host tells us, plus 2 for header & tailer. */
unsigned int sg_elems;
+ /* Ida index - used to track minor number allocations. */
+ int index;
+
/* Scatterlist: can be too big for stack. */
struct scatterlist sg[/*sg_elems*/];
};
@@ -276,6 +282,11 @@ static int index_to_minor(int index)
return index << PART_BITS;
}
+static int minor_to_index(int minor)
+{
+ return minor >> PART_BITS;
+}
+
static ssize_t virtblk_serial_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -341,14 +352,17 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
{
struct virtio_blk *vblk;
struct request_queue *q;
- int err;
+ int err, index;
u64 cap;
u32 v, blk_size, sg_elems, opt_io_size;
u16 min_io_size;
u8 physical_block_exp, alignment_offset;
- if (index_to_minor(index) >= 1 << MINORBITS)
- return -ENOSPC;
+ err = ida_simple_get(&vd_index_ida, 0, minor_to_index(1 << MINORBITS),
+ GFP_KERNEL);
+ if (err < 0)
+ goto out;
+ index = err;
/* We need to know how many segments before we allocate. */
err = virtio_config_val(vdev, VIRTIO_BLK_F_SEG_MAX,
@@ -365,7 +379,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
sizeof(vblk->sg[0]) * sg_elems, GFP_KERNEL);
if (!vblk) {
err = -ENOMEM;
- goto out;
+ goto out_free_index;
}
INIT_LIST_HEAD(&vblk->reqs);
@@ -421,7 +435,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
vblk->disk->private_data = vblk;
vblk->disk->fops = &virtblk_fops;
vblk->disk->driverfs_dev = &vdev->dev;
- index++;
+ vblk->index = index;
/* configure queue flush support */
if (virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH))
@@ -516,6 +530,8 @@ out_free_vq:
vdev->config->del_vqs(vdev);
out_free_vblk:
kfree(vblk);
+out_free_index:
+ ida_simple_remove(&vd_index_ida, index);
out:
return err;
}
@@ -523,6 +539,7 @@ out:
static void __devexit virtblk_remove(struct virtio_device *vdev)
{
struct virtio_blk *vblk = vdev->priv;
+ int index = vblk->index;
flush_work(&vblk->config_work);
@@ -538,6 +555,7 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
mempool_destroy(vblk->pool);
vdev->config->del_vqs(vdev);
kfree(vblk);
+ ida_simple_remove(&vd_index_ida, index);
}
static const struct virtio_device_id id_table[] = {
--
1.7.5.53.gc233e
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Rusty Russell <rusty@rustcorp.com.au>, Tejun Heo <tj@kernel.org>,
virtualization@lists.linux-foundation.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@suse.de>
Subject: [PATCHv4] virtio-blk: use ida to allocate disk index
Date: Sun, 30 Oct 2011 21:29:59 +0200 [thread overview]
Message-ID: <20111030192959.GA31143@redhat.com> (raw)
Based on a patch by Mark Wu <dwu@redhat.com>
Current index allocation in virtio-blk is based on a monotonically
increasing variable "index". This means we'll run out of numbers
after a while. It also could cause confusion about the disk
name in the case of hot-plugging disks.
Change virtio-blk to use ida to allocate index, instead.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Changes from Mark's versions:
use the new ida_simple_get
error handling cleanup
fix user after free
Works fine for me.
Jens, could you merge this for 3.2?
That is, unless Rusty complains shortly ...
drivers/block/virtio_blk.c | 30 ++++++++++++++++++++++++------
1 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 079c088..e7a5750 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -8,10 +8,13 @@
#include <linux/scatterlist.h>
#include <linux/string_helpers.h>
#include <scsi/scsi_cmnd.h>
+#include <linux/idr.h>
#define PART_BITS 4
-static int major, index;
+static int major;
+static DEFINE_IDA(vd_index_ida);
+
struct workqueue_struct *virtblk_wq;
struct virtio_blk
@@ -35,6 +38,9 @@ struct virtio_blk
/* What host tells us, plus 2 for header & tailer. */
unsigned int sg_elems;
+ /* Ida index - used to track minor number allocations. */
+ int index;
+
/* Scatterlist: can be too big for stack. */
struct scatterlist sg[/*sg_elems*/];
};
@@ -276,6 +282,11 @@ static int index_to_minor(int index)
return index << PART_BITS;
}
+static int minor_to_index(int minor)
+{
+ return minor >> PART_BITS;
+}
+
static ssize_t virtblk_serial_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -341,14 +352,17 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
{
struct virtio_blk *vblk;
struct request_queue *q;
- int err;
+ int err, index;
u64 cap;
u32 v, blk_size, sg_elems, opt_io_size;
u16 min_io_size;
u8 physical_block_exp, alignment_offset;
- if (index_to_minor(index) >= 1 << MINORBITS)
- return -ENOSPC;
+ err = ida_simple_get(&vd_index_ida, 0, minor_to_index(1 << MINORBITS),
+ GFP_KERNEL);
+ if (err < 0)
+ goto out;
+ index = err;
/* We need to know how many segments before we allocate. */
err = virtio_config_val(vdev, VIRTIO_BLK_F_SEG_MAX,
@@ -365,7 +379,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
sizeof(vblk->sg[0]) * sg_elems, GFP_KERNEL);
if (!vblk) {
err = -ENOMEM;
- goto out;
+ goto out_free_index;
}
INIT_LIST_HEAD(&vblk->reqs);
@@ -421,7 +435,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
vblk->disk->private_data = vblk;
vblk->disk->fops = &virtblk_fops;
vblk->disk->driverfs_dev = &vdev->dev;
- index++;
+ vblk->index = index;
/* configure queue flush support */
if (virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH))
@@ -516,6 +530,8 @@ out_free_vq:
vdev->config->del_vqs(vdev);
out_free_vblk:
kfree(vblk);
+out_free_index:
+ ida_simple_remove(&vd_index_ida, index);
out:
return err;
}
@@ -523,6 +539,7 @@ out:
static void __devexit virtblk_remove(struct virtio_device *vdev)
{
struct virtio_blk *vblk = vdev->priv;
+ int index = vblk->index;
flush_work(&vblk->config_work);
@@ -538,6 +555,7 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
mempool_destroy(vblk->pool);
vdev->config->del_vqs(vdev);
kfree(vblk);
+ ida_simple_remove(&vd_index_ida, index);
}
static const struct virtio_device_id id_table[] = {
--
1.7.5.53.gc233e
next reply other threads:[~2011-10-30 19:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-30 19:29 Michael S. Tsirkin [this message]
2011-10-30 19:29 ` [PATCHv4] virtio-blk: use ida to allocate disk index Michael S. Tsirkin
2011-10-31 7:05 ` Jens Axboe
2011-10-31 23:55 ` Rusty Russell
2011-10-31 23:55 ` Rusty Russell
2011-10-31 7:05 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111030192959.GA31143@redhat.com \
--to=mst@redhat.com \
--cc=axboe@kernel.dk \
--cc=gregkh@suse.de \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tj@kernel.org \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.