patch 1-3,5 are some cleanups.
patch 4: create a separate work queue for virtio-scsi
to improve the performance, I tested with tmpfs backed
disk, the config file is like below,
[global]
bsrange=4k-64k
ioengine=libaio
direct=1
iodepth=4
loops=10
size=64M
Before:
Disk stats (read/write):
sda: ios=6547/5275, merge=37/39, ticks=3144/2645, in_queue=5780, util=81.62%
sdb: ios=6542/5277, merge=54/59, ticks=3234/3143, in_queue=6364, util=84.50%
sdc: ios=6532/5244, merge=65/62, ticks=3440/3076, in_queue=6505, util=86.83%
After:
Disk stats (read/write):
sda: ios=6349/5318, merge=77/89, ticks=4820/3169, in_queue=7991, util=90.62%
sdb: ios=6364/5358, merge=39/53, ticks=3810/2816, in_queue=6615, util=84.63%
sdc: ios=6458/5352, merge=74/47, ticks=4425/3048, in_queue=7459, util=88.41%
Wanlong Gao (5):
virtio-scsi: remove the useless assignment
virtio-scsi: remove the needless variable gfp_mask
virtio-scsi: use pr_err instead of printk
virtio-scsi: create a separate work queue for virtio-scsi
virtio-scsi: tidy up the goto label in init()
Cc: James E.J. Bottomley <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: [email protected]
Cc: [email protected]
drivers/scsi/virtio_scsi.c | 43 +++++++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 20 deletions(-)
--
1.8.0
Reassign err is not needed, just a cleanup.
Cc: James E.J. Bottomley <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Wanlong Gao <[email protected]>
---
drivers/scsi/virtio_scsi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index d5f9f45..8746c37 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -669,7 +669,6 @@ static int virtscsi_init(struct virtio_device *vdev,
goto out;
}
}
- err = 0;
out:
if (err)
--
1.8.0
Tidy up the goto label in init(), and remove the useless
NULL pointer assignment.
Cc: James E.J. Bottomley <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Wanlong Gao <[email protected]>
---
drivers/scsi/virtio_scsi.c | 28 +++++++++++-----------------
1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index fc05240..afee1d4 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -793,13 +793,13 @@ static int __init init(void)
virtscsi_wq = alloc_workqueue("virtio-scsi", 0, 0);
if (!virtscsi_wq) {
pr_err("alloc_workqueue() for virtscsi_wq failed\n");
- goto error;
+ goto workqueue_failed;
}
virtscsi_cmd_cache = KMEM_CACHE(virtio_scsi_cmd, 0);
if (!virtscsi_cmd_cache) {
pr_err("kmem_cache_create() for virtscsi_cmd_cache failed\n");
- goto error;
+ goto kmem_cache_failed;
}
@@ -808,27 +808,21 @@ static int __init init(void)
virtscsi_cmd_cache);
if (!virtscsi_cmd_pool) {
pr_err("mempool_create() for virtscsi_cmd_pool failed\n");
- goto error;
+ goto mempool_failed;
}
ret = register_virtio_driver(&virtio_scsi_driver);
if (ret < 0)
- goto error;
+ goto register_failed;
return 0;
-error:
- if (virtscsi_cmd_pool) {
- mempool_destroy(virtscsi_cmd_pool);
- virtscsi_cmd_pool = NULL;
- }
- if (virtscsi_cmd_cache) {
- kmem_cache_destroy(virtscsi_cmd_cache);
- virtscsi_cmd_cache = NULL;
- }
- if (virtscsi_wq) {
- destroy_workqueue(virtscsi_wq);
- virtscsi_wq = NULL;
- }
+register_failed:
+ mempool_destroy(virtscsi_cmd_pool);
+mempool_failed:
+ kmem_cache_destroy(virtscsi_cmd_cache);
+kmem_cache_failed:
+ destroy_workqueue(virtscsi_wq);
+workqueue_failed:
return ret;
}
--
1.8.0
Create a separate work queue for virtio-scsi to improve the performance.
Cc: James E.J. Bottomley <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Wanlong Gao <[email protected]>
---
drivers/scsi/virtio_scsi.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 765138a..fc05240 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -27,6 +27,8 @@
#define VIRTIO_SCSI_MEMPOOL_SZ 64
#define VIRTIO_SCSI_EVENT_LEN 8
+struct workqueue_struct *virtscsi_wq;
+
/* Command queue element */
struct virtio_scsi_cmd {
struct scsi_cmnd *sc;
@@ -337,7 +339,7 @@ static void virtscsi_complete_event(void *buf)
struct virtio_scsi_event_node *event_node = buf;
INIT_WORK(&event_node->work, virtscsi_handle_event);
- schedule_work(&event_node->work);
+ queue_work(virtscsi_wq, &event_node->work);
}
static void virtscsi_event_done(struct virtqueue *vq)
@@ -788,6 +790,12 @@ static int __init init(void)
{
int ret = -ENOMEM;
+ virtscsi_wq = alloc_workqueue("virtio-scsi", 0, 0);
+ if (!virtscsi_wq) {
+ pr_err("alloc_workqueue() for virtscsi_wq failed\n");
+ goto error;
+ }
+
virtscsi_cmd_cache = KMEM_CACHE(virtio_scsi_cmd, 0);
if (!virtscsi_cmd_cache) {
pr_err("kmem_cache_create() for virtscsi_cmd_cache failed\n");
@@ -817,6 +825,10 @@ error:
kmem_cache_destroy(virtscsi_cmd_cache);
virtscsi_cmd_cache = NULL;
}
+ if (virtscsi_wq) {
+ destroy_workqueue(virtscsi_wq);
+ virtscsi_wq = NULL;
+ }
return ret;
}
@@ -825,6 +837,7 @@ static void __exit fini(void)
unregister_virtio_driver(&virtio_scsi_driver);
mempool_destroy(virtscsi_cmd_pool);
kmem_cache_destroy(virtscsi_cmd_cache);
+ destroy_workqueue(virtscsi_wq);
}
module_init(init);
module_exit(fini);
--
1.8.0
Just use the macro instead of define a variable.
Cc: James E.J. Bottomley <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Wanlong Gao <[email protected]>
---
drivers/scsi/virtio_scsi.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 8746c37..5390229 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -587,11 +587,10 @@ static struct virtio_scsi_target_state *virtscsi_alloc_tgt(
struct virtio_device *vdev, int sg_elems)
{
struct virtio_scsi_target_state *tgt;
- gfp_t gfp_mask = GFP_KERNEL;
/* We need extra sg elements at head and tail. */
tgt = kmalloc(sizeof(*tgt) + sizeof(tgt->sg[0]) * (sg_elems + 2),
- gfp_mask);
+ GFP_KERNEL);
if (!tgt)
return NULL;
--
1.8.0
Use pr_err() instead of printk() for code cleanups.
Cc: James E.J. Bottomley <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Wanlong Gao <[email protected]>
---
drivers/scsi/virtio_scsi.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 5390229..765138a 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -790,8 +790,7 @@ static int __init init(void)
virtscsi_cmd_cache = KMEM_CACHE(virtio_scsi_cmd, 0);
if (!virtscsi_cmd_cache) {
- printk(KERN_ERR "kmem_cache_create() for "
- "virtscsi_cmd_cache failed\n");
+ pr_err("kmem_cache_create() for virtscsi_cmd_cache failed\n");
goto error;
}
@@ -800,8 +799,7 @@ static int __init init(void)
mempool_create_slab_pool(VIRTIO_SCSI_MEMPOOL_SZ,
virtscsi_cmd_cache);
if (!virtscsi_cmd_pool) {
- printk(KERN_ERR "mempool_create() for"
- "virtscsi_cmd_pool failed\n");
+ pr_err("mempool_create() for virtscsi_cmd_pool failed\n");
goto error;
}
ret = register_virtio_driver(&virtio_scsi_driver);
--
1.8.0
On Wed, Nov 7, 2012 at 6:18 PM, Wanlong Gao <[email protected]> wrote:
> patch 1-3,5 are some cleanups.
> patch 4: create a separate work queue for virtio-scsi
> to improve the performance, I tested with tmpfs backed
> disk, the config file is like below,
> [global]
> bsrange=4k-64k
> ioengine=libaio
> direct=1
> iodepth=4
> loops=10
> size=64M
>
> Before:
> Disk stats (read/write):
> sda: ios=6547/5275, merge=37/39, ticks=3144/2645, in_queue=5780, util=81.62%
> sdb: ios=6542/5277, merge=54/59, ticks=3234/3143, in_queue=6364, util=84.50%
> sdc: ios=6532/5244, merge=65/62, ticks=3440/3076, in_queue=6505, util=86.83%
The result is about the disk stat. Can you please post the iops and
the bw instead?
> After:
> Disk stats (read/write):
> sda: ios=6349/5318, merge=77/89, ticks=4820/3169, in_queue=7991, util=90.62%
> sdb: ios=6364/5358, merge=39/53, ticks=3810/2816, in_queue=6615, util=84.63%
> sdc: ios=6458/5352, merge=74/47, ticks=4425/3048, in_queue=7459, util=88.41%
>
>
> Wanlong Gao (5):
> virtio-scsi: remove the useless assignment
> virtio-scsi: remove the needless variable gfp_mask
> virtio-scsi: use pr_err instead of printk
> virtio-scsi: create a separate work queue for virtio-scsi
> virtio-scsi: tidy up the goto label in init()
>
>
>
> Cc: James E.J. Bottomley <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Cc: Rusty Russell <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> drivers/scsi/virtio_scsi.c | 43 +++++++++++++++++++++++--------------------
> 1 file changed, 23 insertions(+), 20 deletions(-)
>
> --
> 1.8.0
>
> --
> 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
--
Asias He
On 11/07/2012 08:23 PM, Asias He wrote:
> On Wed, Nov 7, 2012 at 6:18 PM, Wanlong Gao <[email protected]> wrote:
>> patch 1-3,5 are some cleanups.
>> patch 4: create a separate work queue for virtio-scsi
>> to improve the performance, I tested with tmpfs backed
>> disk, the config file is like below,
>> [global]
>> bsrange=4k-64k
>> ioengine=libaio
>> direct=1
>> iodepth=4
>> loops=10
>> size=64M
>>
>> Before:
>> Disk stats (read/write):
>> sda: ios=6547/5275, merge=37/39, ticks=3144/2645, in_queue=5780, util=81.62%
>> sdb: ios=6542/5277, merge=54/59, ticks=3234/3143, in_queue=6364, util=84.50%
>> sdc: ios=6532/5244, merge=65/62, ticks=3440/3076, in_queue=6505, util=86.83%
>
> The result is about the disk stat. Can you please post the iops and
> the bw instead?
Sure,
before:
write: io=589788KB, bw=122211KB/s, iops=3584 , runt= 4826msec
bw (KB/s) : min=24208, max=116272, per=57.47%, avg=70236.40, stdev=35496.33
WRITE: io=589788KB, aggrb=122210KB/s, minb=122210KB/s, maxb=122210KB/s, mint=4826msec, maxt=4826msec
after:
write: io=589788KB, bw=124244KB/s, iops=3530 , runt= 4747msec
bw (KB/s) : min=63496, max=117943, per=73.39%, avg=91184.20, stdev=20062.42
WRITE: io=589788KB, aggrb=124244KB/s, minb=124244KB/s, maxb=124244KB/s, mint=4747msec, maxt=4747msec
Thanks,
Wanlong Gao
>
>> After:
>> Disk stats (read/write):
>> sda: ios=6349/5318, merge=77/89, ticks=4820/3169, in_queue=7991, util=90.62%
>> sdb: ios=6364/5358, merge=39/53, ticks=3810/2816, in_queue=6615, util=84.63%
>> sdc: ios=6458/5352, merge=74/47, ticks=4425/3048, in_queue=7459, util=88.41%
>>
>>
>> Wanlong Gao (5):
>> virtio-scsi: remove the useless assignment
>> virtio-scsi: remove the needless variable gfp_mask
>> virtio-scsi: use pr_err instead of printk
>> virtio-scsi: create a separate work queue for virtio-scsi
>> virtio-scsi: tidy up the goto label in init()
>>
>>
>>
>> Cc: James E.J. Bottomley <[email protected]>
>> Cc: Paolo Bonzini <[email protected]>
>> Cc: Rusty Russell <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> drivers/scsi/virtio_scsi.c | 43 +++++++++++++++++++++++--------------------
>> 1 file changed, 23 insertions(+), 20 deletions(-)
>>
>> --
>> 1.8.0
>>
>> --
>> 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
>
>
>
> patch 1-3,5 are some cleanups.
> patch 4: create a separate work queue for virtio-scsi
> to improve the performance, I tested with tmpfs backed
> disk, the config file is like below,
I think something else caused the improvement, because the code you
touched (complete_event) shouldn't ever run during a normal benchmark.
It's only used for hotplug/hot-unplug.
Nevertheless, I'll queue the cleanup patches. Thanks for those.
Paolo
> [global]
> bsrange=4k-64k
> ioengine=libaio
> direct=1
> iodepth=4
> loops=10
> size=64M
>
> Before:
> Disk stats (read/write):
> sda: ios=6547/5275, merge=37/39, ticks=3144/2645, in_queue=5780,
> util=81.62%
> sdb: ios=6542/5277, merge=54/59, ticks=3234/3143, in_queue=6364,
> util=84.50%
> sdc: ios=6532/5244, merge=65/62, ticks=3440/3076, in_queue=6505,
> util=86.83%
>
> After:
> Disk stats (read/write):
> sda: ios=6349/5318, merge=77/89, ticks=4820/3169, in_queue=7991,
> util=90.62%
> sdb: ios=6364/5358, merge=39/53, ticks=3810/2816, in_queue=6615,
> util=84.63%
> sdc: ios=6458/5352, merge=74/47, ticks=4425/3048, in_queue=7459,
> util=88.41%
>
>
> Wanlong Gao (5):
> virtio-scsi: remove the useless assignment
> virtio-scsi: remove the needless variable gfp_mask
> virtio-scsi: use pr_err instead of printk
> virtio-scsi: create a separate work queue for virtio-scsi
> virtio-scsi: tidy up the goto label in init()
>
>
>
> Cc: James E.J. Bottomley <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Cc: Rusty Russell <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> drivers/scsi/virtio_scsi.c | 43
> +++++++++++++++++++++++--------------------
> 1 file changed, 23 insertions(+), 20 deletions(-)
>
> --
> 1.8.0
>
>
On 11/08/2012 06:12 AM, Paolo Bonzini wrote:
>> patch 1-3,5 are some cleanups.
>> patch 4: create a separate work queue for virtio-scsi
>> to improve the performance, I tested with tmpfs backed
>> disk, the config file is like below,
>
> I think something else caused the improvement, because the code you
> touched (complete_event) shouldn't ever run during a normal benchmark.
> It's only used for hotplug/hot-unplug.
>
> Nevertheless, I'll queue the cleanup patches. Thanks for those.
Oops, got it, I will investigate more. Thank you for teaching me.
Regards,
Wanlong Gao