Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-19 Thread Rusty Russell
Joe Perches j...@perches.com writes: On Sun, 2014-03-16 at 22:00 -0700, Joe Perches wrote: On Mon, 2014-03-17 at 14:25 +1030, Rusty Russell wrote: Erk, our tests are insufficient. Testbuilding an allmodconfig with this now: Good idea. diff --git a/include/linux/moduleparam.h

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-19 Thread Greg Kroah-Hartman
On Wed, Mar 19, 2014 at 05:07:50PM +1030, Rusty Russell wrote: Joe Perches j...@perches.com writes: On Sun, 2014-03-16 at 22:00 -0700, Joe Perches wrote: On Mon, 2014-03-17 at 14:25 +1030, Rusty Russell wrote: Erk, our tests are insufficient. Testbuilding an allmodconfig with this

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-19 Thread Bjorn Helgaas
On Wed, Mar 19, 2014 at 12:37 AM, Rusty Russell ru...@rustcorp.com.au wrote: Joe Perches j...@perches.com writes: On Sun, 2014-03-16 at 22:00 -0700, Joe Perches wrote: On Mon, 2014-03-17 at 14:25 +1030, Rusty Russell wrote: Erk, our tests are insufficient. Testbuilding an allmodconfig with

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-19 Thread Joe Perches
On Wed, 2014-03-19 at 17:07 +1030, Rusty Russell wrote: Summary of http://lkml.org/lkml/2014/3/14/363 : Ted: module_param(queue_depth, int, 444) Joe: 0444! Rusty: User perms = group perms = other perms? Joe: CLASS_ATTR, DEVICE_ATTR, SENSOR_ATTR and SENSOR_ATTR_2? smile Adding:

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-19 Thread Joe Perches
Couple more bikesheddy things: Is there ever a reason to use a non __builtin_const_p(perms)? Maybe that should be a BUILD_BUG_ON too BUILD_BUG_ON(!builtin_const_p_perms) My brain of little size gets confused by the BUILD_BUG_ON_ZERO(foo) + vs BUILD_BUG_ON(foo); as it

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-19 Thread Rusty Russell
Bjorn Helgaas bhelg...@google.com writes: On Wed, Mar 19, 2014 at 12:37 AM, Rusty Russell ru...@rustcorp.com.au wrote: Side effect of stricter permissions means removing the unnecessary S_IFREG from drivers/pci/slot.c. Suggested-by: Joe Perches j...@perches.com Cc: Bjorn Helgaas

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-19 Thread Rusty Russell
Joe Perches j...@perches.com writes: Couple more bikesheddy things: Is there ever a reason to use a non __builtin_const_p(perms)? It's a bit conservative, and anyway, the test is useless since AFAICT BUILD_BUG_ON() is a noop if !__builtin_const_p(). I removed it and re-tested. Maybe that

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-17 Thread Joe Perches
On Mon, 2014-03-17 at 14:25 +1030, Rusty Russell wrote: Erk, our tests are insufficient. Testbuilding an allmodconfig with this now: Good idea. diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h [] @@ -188,6 +188,9 @@ struct kparam_array /* Default value instead

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-17 Thread Joe Perches
On Sun, 2014-03-16 at 22:00 -0700, Joe Perches wrote: On Mon, 2014-03-17 at 14:25 +1030, Rusty Russell wrote: Erk, our tests are insufficient. Testbuilding an allmodconfig with this now: Good idea. diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h [] @@

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-16 Thread Rusty Russell
Theodore Ts'o ty...@mit.edu writes: On Fri, Mar 14, 2014 at 10:38:40AM -0700, Joe Perches wrote: +static int queue_depth = 64; +module_param(queue_depth, int, 444); 444? Really Ted? Oops, *blush*. Thanks for catching that. Erk, our tests are insufficient. Testbuilding an

[PATCH] virtio-blk: make the queue depth configurable

2014-03-14 Thread Theodore Ts'o
The current virtio block sets a queue depth of 64. With a sufficiently fast device, using a queue depth of 256 can double the IOPS which can be sustained. So make the queue depth something which can be set at module load time or via a kernel boot-time parameter. Signed-off-by: Theodore Ts'o

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-14 Thread Joe Perches
On Fri, 2014-03-14 at 13:31 -0400, Theodore Ts'o wrote: The current virtio block sets a queue depth of 64. With a sufficiently fast device, using a queue depth of 256 can double the IOPS which can be sustained. So make the queue depth something which can be set at module load time or via a

Re: [PATCH] virtio-blk: make the queue depth configurable

2014-03-14 Thread Theodore Ts'o
On Fri, Mar 14, 2014 at 10:38:40AM -0700, Joe Perches wrote: +static int queue_depth = 64; +module_param(queue_depth, int, 444); 444? Really Ted? Oops, *blush*. Thanks for catching that. - Ted ___