* [PATCH] nvme-pci: add module parameter for io queue depth
@ 2017-07-07 9:05 weiping zhang
2017-07-07 13:50 ` Christoph Hellwig
2017-07-07 15:29 ` Keith Busch
0 siblings, 2 replies; 8+ messages in thread
From: weiping zhang @ 2017-07-07 9:05 UTC (permalink / raw)
Adjust io queue depth more easily.
Signed-off-by: weiping zhang <zhangweiping at didichuxing.com>
---
drivers/nvme/host/pci.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 951042a..f1ac0f8 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -48,7 +48,6 @@
#include "nvme.h"
-#define NVME_Q_DEPTH 1024
#define NVME_AQ_DEPTH 256
#define SQ_SIZE(depth) (depth * sizeof(struct nvme_command))
#define CQ_SIZE(depth) (depth * sizeof(struct nvme_completion))
@@ -66,6 +65,10 @@ static bool use_cmb_sqes = true;
module_param(use_cmb_sqes, bool, 0644);
MODULE_PARM_DESC(use_cmb_sqes, "use controller's memory buffer for I/O SQes");
+static int io_queue_depth = 1024;
+module_param(io_queue_depth, int, 0644);
+MODULE_PARM_DESC(io_queue_depth, "set io queue depth");
+
static struct workqueue_struct *nvme_workq;
struct nvme_dev;
@@ -1730,7 +1733,7 @@ static int nvme_pci_enable(struct nvme_dev *dev)
cap = lo_hi_readq(dev->bar + NVME_REG_CAP);
- dev->q_depth = min_t(int, NVME_CAP_MQES(cap) + 1, NVME_Q_DEPTH);
+ dev->q_depth = min_t(int, NVME_CAP_MQES(cap) + 1, io_queue_depth);
dev->db_stride = 1 << NVME_CAP_STRIDE(cap);
dev->dbs = dev->bar + 4096;
--
2.9.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] nvme-pci: add module parameter for io queue depth
2017-07-07 9:05 [PATCH] nvme-pci: add module parameter for io queue depth weiping zhang
@ 2017-07-07 13:50 ` Christoph Hellwig
2017-07-07 14:56 ` Jens Axboe
2017-07-07 14:56 ` Jens Axboe
2017-07-07 15:29 ` Keith Busch
1 sibling, 2 replies; 8+ messages in thread
From: Christoph Hellwig @ 2017-07-07 13:50 UTC (permalink / raw)
On Fri, Jul 07, 2017@05:05:47PM +0800, weiping zhang wrote:
> Adjust io queue depth more easily.
>
> Signed-off-by: weiping zhang <zhangweiping at didichuxing.com>
Sounds useful in general, the only thin I'm worried about is that
this will affect all drives in the system, which might have
very different characteristics. But I can't really think of
a better approach.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme-pci: add module parameter for io queue depth
2017-07-07 13:50 ` Christoph Hellwig
@ 2017-07-07 14:56 ` Jens Axboe
2017-07-07 14:56 ` Jens Axboe
1 sibling, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2017-07-07 14:56 UTC (permalink / raw)
On 07/07/2017 07:50 AM, Christoph Hellwig wrote:
> On Fri, Jul 07, 2017@05:05:47PM +0800, weiping zhang wrote:
>> Adjust io queue depth more easily.
>>
>> Signed-off-by: weiping zhang <zhangweiping at didichuxing.com>
>
> Sounds useful in general, the only thin I'm worried about is that
> this will affect all drives in the system, which might have
> very different characteristics. But I can't really think of
> a better approach.
That's a generic problem that all module parameters have, and it's
sometimes quite annoying. But that should be fixed separately.
I'm fine with this patch, it's an improvement over what we have.
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme-pci: add module parameter for io queue depth
2017-07-07 13:50 ` Christoph Hellwig
2017-07-07 14:56 ` Jens Axboe
@ 2017-07-07 14:56 ` Jens Axboe
1 sibling, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2017-07-07 14:56 UTC (permalink / raw)
On 07/07/2017 07:50 AM, Christoph Hellwig wrote:
> On Fri, Jul 07, 2017@05:05:47PM +0800, weiping zhang wrote:
>> Adjust io queue depth more easily.
>>
>> Signed-off-by: weiping zhang <zhangweiping at didichuxing.com>
>
> Sounds useful in general, the only thin I'm worried about is that
> this will affect all drives in the system, which might have
> very different characteristics. But I can't really think of
> a better approach.
That's a generic problem that all module parameters have, and it's
sometimes quite annoying. But that should be fixed separately.
I'm fine with this patch, it's an improvement over what we have.
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme-pci: add module parameter for io queue depth
2017-07-07 9:05 [PATCH] nvme-pci: add module parameter for io queue depth weiping zhang
2017-07-07 13:50 ` Christoph Hellwig
@ 2017-07-07 15:29 ` Keith Busch
2017-07-07 16:53 ` weiping zhang
2017-07-10 5:54 ` Sagi Grimberg
1 sibling, 2 replies; 8+ messages in thread
From: Keith Busch @ 2017-07-07 15:29 UTC (permalink / raw)
On Fri, Jul 07, 2017@05:05:47PM +0800, weiping zhang wrote:
> Adjust io queue depth more easily.
>
> Signed-off-by: weiping zhang <zhangweiping at didichuxing.com>
> ---
> drivers/nvme/host/pci.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 951042a..f1ac0f8 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -48,7 +48,6 @@
>
> #include "nvme.h"
>
> -#define NVME_Q_DEPTH 1024
> #define NVME_AQ_DEPTH 256
> #define SQ_SIZE(depth) (depth * sizeof(struct nvme_command))
> #define CQ_SIZE(depth) (depth * sizeof(struct nvme_completion))
> @@ -66,6 +65,10 @@ static bool use_cmb_sqes = true;
> module_param(use_cmb_sqes, bool, 0644);
> MODULE_PARM_DESC(use_cmb_sqes, "use controller's memory buffer for I/O SQes");
>
> +static int io_queue_depth = 1024;
> +module_param(io_queue_depth, int, 0644);
> +MODULE_PARM_DESC(io_queue_depth, "set io queue depth");
> +
> static struct workqueue_struct *nvme_workq;
>
> struct nvme_dev;
> @@ -1730,7 +1733,7 @@ static int nvme_pci_enable(struct nvme_dev *dev)
>
> cap = lo_hi_readq(dev->bar + NVME_REG_CAP);
>
> - dev->q_depth = min_t(int, NVME_CAP_MQES(cap) + 1, NVME_Q_DEPTH);
> + dev->q_depth = min_t(int, NVME_CAP_MQES(cap) + 1, io_queue_depth);
We need to be a little more careful about this field if users can write
whatever they want. It's gotta be >= 2 or else trouble will occur.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme-pci: add module parameter for io queue depth
2017-07-07 15:29 ` Keith Busch
@ 2017-07-07 16:53 ` weiping zhang
2017-07-10 5:54 ` Sagi Grimberg
1 sibling, 0 replies; 8+ messages in thread
From: weiping zhang @ 2017-07-07 16:53 UTC (permalink / raw)
On Fri, Jul 07, 2017@11:29:48AM -0400, Keith Busch wrote:
> On Fri, Jul 07, 2017@05:05:47PM +0800, weiping zhang wrote:
> > Adjust io queue depth more easily.
> > cap = lo_hi_readq(dev->bar + NVME_REG_CAP);
> >
> > - dev->q_depth = min_t(int, NVME_CAP_MQES(cap) + 1, NVME_Q_DEPTH);
> > + dev->q_depth = min_t(int, NVME_CAP_MQES(cap) + 1, io_queue_depth);
>
> We need to be a little more careful about this field if users can write
> whatever they want. It's gotta be >= 2 or else trouble will occur.
ok, I will send patch v2, thanks all.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme-pci: add module parameter for io queue depth
2017-07-07 15:29 ` Keith Busch
2017-07-07 16:53 ` weiping zhang
@ 2017-07-10 5:54 ` Sagi Grimberg
2017-07-10 8:02 ` weiping zhang
1 sibling, 1 reply; 8+ messages in thread
From: Sagi Grimberg @ 2017-07-10 5:54 UTC (permalink / raw)
>> +static int io_queue_depth = 1024;
>> +module_param(io_queue_depth, int, 0644);
>> +MODULE_PARM_DESC(io_queue_depth, "set io queue depth");
>> +
>> static struct workqueue_struct *nvme_workq;
>>
>> struct nvme_dev;
>> @@ -1730,7 +1733,7 @@ static int nvme_pci_enable(struct nvme_dev *dev)
>>
>> cap = lo_hi_readq(dev->bar + NVME_REG_CAP);
>>
>> - dev->q_depth = min_t(int, NVME_CAP_MQES(cap) + 1, NVME_Q_DEPTH);
>> + dev->q_depth = min_t(int, NVME_CAP_MQES(cap) + 1, io_queue_depth);
>
> We need to be a little more careful about this field if users can write
> whatever they want. It's gotta be >= 2 or else trouble will occur.
Agreed, please resend the patch. Perhaps a good way is to use
module_param_cb() to verify the input value is >= 2?
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme-pci: add module parameter for io queue depth
2017-07-10 5:54 ` Sagi Grimberg
@ 2017-07-10 8:02 ` weiping zhang
0 siblings, 0 replies; 8+ messages in thread
From: weiping zhang @ 2017-07-10 8:02 UTC (permalink / raw)
On Mon, Jul 10, 2017@08:54:57AM +0300, Sagi Grimberg wrote:
>
> >>+static int io_queue_depth = 1024;
> >>+module_param(io_queue_depth, int, 0644);
> >>+MODULE_PARM_DESC(io_queue_depth, "set io queue depth");
> >We need to be a little more careful about this field if users can write
> >whatever they want. It's gotta be >= 2 or else trouble will occur.
>
> Agreed, please resend the patch. Perhaps a good way is to use
> module_param_cb() to verify the input value is >= 2?
thanks so much, I will send patch v3.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-07-10 8:02 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-07 9:05 [PATCH] nvme-pci: add module parameter for io queue depth weiping zhang
2017-07-07 13:50 ` Christoph Hellwig
2017-07-07 14:56 ` Jens Axboe
2017-07-07 14:56 ` Jens Axboe
2017-07-07 15:29 ` Keith Busch
2017-07-07 16:53 ` weiping zhang
2017-07-10 5:54 ` Sagi Grimberg
2017-07-10 8:02 ` weiping zhang
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.