public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] NVMe: Refactor doorbell
  2013-10-10 18:29 [PATCH 0/2] NVMe: Refactoring Matias Bjorling
@ 2013-10-10 18:29 ` Matias Bjorling
  0 siblings, 0 replies; 5+ messages in thread
From: Matias Bjorling @ 2013-10-10 18:29 UTC (permalink / raw)
  To: willy; +Cc: linux-kernel, linux-nvme, Matias Bjorling

The doorbell code is repeated various places. Refactor it into its own function
for clarity.

Signed-off-by: Matias Bjorling <m@bjorling.me>
---
 drivers/block/nvme-core.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index da52092..1e940e8 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -242,6 +242,13 @@ void put_nvmeq(struct nvme_queue *nvmeq)
 	put_cpu();
 }
 
+static inline void nvme_ring_doorbell(struct nvme_queue *nvmeq)
+{
+	if (++nvmeq->sq_tail == nvmeq->q_depth)
+		nvmeq->sq_tail = 0;
+	writel(nvmeq->sq_tail, nvmeq->q_db);
+}
+
 /**
  * nvme_submit_cmd() - Copy a command into a queue and ring the doorbell
  * @nvmeq: The queue to use
@@ -252,14 +259,10 @@ void put_nvmeq(struct nvme_queue *nvmeq)
 static int nvme_submit_cmd(struct nvme_queue *nvmeq, struct nvme_command *cmd)
 {
 	unsigned long flags;
-	u16 tail;
+
 	spin_lock_irqsave(&nvmeq->q_lock, flags);
-	tail = nvmeq->sq_tail;
-	memcpy(&nvmeq->sq_cmds[tail], cmd, sizeof(*cmd));
-	if (++tail == nvmeq->q_depth)
-		tail = 0;
-	writel(tail, nvmeq->q_db);
-	nvmeq->sq_tail = tail;
+	memcpy(&nvmeq->sq_cmds[nvmeq->sqtail], cmd, sizeof(*cmd));
+	nvme_ring_doorbell(nvmeq);
 	spin_unlock_irqrestore(&nvmeq->q_lock, flags);
 
 	return 0;
@@ -619,9 +622,7 @@ static int nvme_submit_discard(struct nvme_queue *nvmeq, struct nvme_ns *ns,
 	cmnd->dsm.nr = 0;
 	cmnd->dsm.attributes = cpu_to_le32(NVME_DSMGMT_AD);
 
-	if (++nvmeq->sq_tail == nvmeq->q_depth)
-		nvmeq->sq_tail = 0;
-	writel(nvmeq->sq_tail, nvmeq->q_db);
+	nvme_ring_doorbell(nvmeq);
 
 	return 0;
 }
@@ -636,9 +637,7 @@ static int nvme_submit_flush(struct nvme_queue *nvmeq, struct nvme_ns *ns,
 	cmnd->common.command_id = cmdid;
 	cmnd->common.nsid = cpu_to_le32(ns->ns_id);
 
-	if (++nvmeq->sq_tail == nvmeq->q_depth)
-		nvmeq->sq_tail = 0;
-	writel(nvmeq->sq_tail, nvmeq->q_db);
+	nvme_ring_doorbell(nvmeq);
 
 	return 0;
 }
@@ -729,9 +728,7 @@ static int nvme_submit_bio_queue(struct nvme_queue *nvmeq, struct nvme_ns *ns,
 	cmnd->rw.dsmgmt = cpu_to_le32(dsmgmt);
 
 	nvme_start_io_acct(bio);
-	if (++nvmeq->sq_tail == nvmeq->q_depth)
-		nvmeq->sq_tail = 0;
-	writel(nvmeq->sq_tail, nvmeq->q_db);
+	nvme_ring_doorbell(nvmeq);
 
 	return 0;
 
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 0/2] NVMe: Refactoring v2
@ 2013-10-11  8:48 Matias Bjorling
  2013-10-11  8:48 ` [PATCH 1/2] NVMe: Refactor doorbell Matias Bjorling
  2013-10-11  8:48 ` [PATCH 2/2] NVMe: Extract admin queue size Matias Bjorling
  0 siblings, 2 replies; 5+ messages in thread
From: Matias Bjorling @ 2013-10-11  8:48 UTC (permalink / raw)
  To: willy; +Cc: linux-kernel, linux-nvme, Matias Bjorling

Two refactor patches that increases code clarity.

The to-be blk-mq proposal patchset utilizes these as part of its transformation.
They are also useful for the current codebase and therefore submitted upfront.

v2:

For some reason I submitted the wrong patch file, with a missing underline in
sq_tail.

Matias Bjorling (2):
  NVMe: Refactor doorbell
  NVMe: Extract admin queue size

 drivers/block/nvme-core.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 1/2] NVMe: Refactor doorbell
  2013-10-11  8:48 [PATCH 0/2] NVMe: Refactoring v2 Matias Bjorling
@ 2013-10-11  8:48 ` Matias Bjorling
  2013-10-11 15:39   ` Keith Busch
  2013-10-11  8:48 ` [PATCH 2/2] NVMe: Extract admin queue size Matias Bjorling
  1 sibling, 1 reply; 5+ messages in thread
From: Matias Bjorling @ 2013-10-11  8:48 UTC (permalink / raw)
  To: willy; +Cc: linux-kernel, linux-nvme, Matias Bjorling

The doorbell code is repeated various places. Refactor it into its own function
for clarity.

Signed-off-by: Matias Bjorling <m@bjorling.me>
---
 drivers/block/nvme-core.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index da52092..40998d5 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -242,6 +242,13 @@ void put_nvmeq(struct nvme_queue *nvmeq)
 	put_cpu();
 }
 
+static inline void nvme_ring_doorbell(struct nvme_queue *nvmeq)
+{
+	if (++nvmeq->sq_tail == nvmeq->q_depth)
+		nvmeq->sq_tail = 0;
+	writel(nvmeq->sq_tail, nvmeq->q_db);
+}
+
 /**
  * nvme_submit_cmd() - Copy a command into a queue and ring the doorbell
  * @nvmeq: The queue to use
@@ -252,14 +259,10 @@ void put_nvmeq(struct nvme_queue *nvmeq)
 static int nvme_submit_cmd(struct nvme_queue *nvmeq, struct nvme_command *cmd)
 {
 	unsigned long flags;
-	u16 tail;
+
 	spin_lock_irqsave(&nvmeq->q_lock, flags);
-	tail = nvmeq->sq_tail;
-	memcpy(&nvmeq->sq_cmds[tail], cmd, sizeof(*cmd));
-	if (++tail == nvmeq->q_depth)
-		tail = 0;
-	writel(tail, nvmeq->q_db);
-	nvmeq->sq_tail = tail;
+	memcpy(&nvmeq->sq_cmds[nvmeq->sq_tail], cmd, sizeof(*cmd));
+	nvme_ring_doorbell(nvmeq);
 	spin_unlock_irqrestore(&nvmeq->q_lock, flags);
 
 	return 0;
@@ -619,9 +622,7 @@ static int nvme_submit_discard(struct nvme_queue *nvmeq, struct nvme_ns *ns,
 	cmnd->dsm.nr = 0;
 	cmnd->dsm.attributes = cpu_to_le32(NVME_DSMGMT_AD);
 
-	if (++nvmeq->sq_tail == nvmeq->q_depth)
-		nvmeq->sq_tail = 0;
-	writel(nvmeq->sq_tail, nvmeq->q_db);
+	nvme_ring_doorbell(nvmeq);
 
 	return 0;
 }
@@ -636,9 +637,7 @@ static int nvme_submit_flush(struct nvme_queue *nvmeq, struct nvme_ns *ns,
 	cmnd->common.command_id = cmdid;
 	cmnd->common.nsid = cpu_to_le32(ns->ns_id);
 
-	if (++nvmeq->sq_tail == nvmeq->q_depth)
-		nvmeq->sq_tail = 0;
-	writel(nvmeq->sq_tail, nvmeq->q_db);
+	nvme_ring_doorbell(nvmeq);
 
 	return 0;
 }
@@ -729,9 +728,7 @@ static int nvme_submit_bio_queue(struct nvme_queue *nvmeq, struct nvme_ns *ns,
 	cmnd->rw.dsmgmt = cpu_to_le32(dsmgmt);
 
 	nvme_start_io_acct(bio);
-	if (++nvmeq->sq_tail == nvmeq->q_depth)
-		nvmeq->sq_tail = 0;
-	writel(nvmeq->sq_tail, nvmeq->q_db);
+	nvme_ring_doorbell(nvmeq);
 
 	return 0;
 
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] NVMe: Extract admin queue size
  2013-10-11  8:48 [PATCH 0/2] NVMe: Refactoring v2 Matias Bjorling
  2013-10-11  8:48 ` [PATCH 1/2] NVMe: Refactor doorbell Matias Bjorling
@ 2013-10-11  8:48 ` Matias Bjorling
  1 sibling, 0 replies; 5+ messages in thread
From: Matias Bjorling @ 2013-10-11  8:48 UTC (permalink / raw)
  To: willy; +Cc: linux-kernel, linux-nvme, Matias Bjorling

The queue size of the admin queue should be defined as a constant for use in
multiple places.

Signed-off-by: Matias Bjorling <m@bjorling.me>
---
 drivers/block/nvme-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 1e940e8..d4f01d4 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -44,6 +44,7 @@
 #include <asm-generic/io-64-nonatomic-lo-hi.h>
 
 #define NVME_Q_DEPTH 1024
+#define NVME_ADMIN_Q_DEPTH 64
 #define SQ_SIZE(depth)		(depth * sizeof(struct nvme_command))
 #define CQ_SIZE(depth)		(depth * sizeof(struct nvme_completion))
 #define NVME_MINORS 64
@@ -1261,7 +1262,7 @@ static int nvme_configure_admin_queue(struct nvme_dev *dev)
 
 	nvmeq = dev->queues[0];
 	if (!nvmeq) {
-		nvmeq = nvme_alloc_queue(dev, 0, 64, 0);
+		nvmeq = nvme_alloc_queue(dev, 0, NVME_ADMIN_Q_DEPTH, 0);
 		if (!nvmeq)
 			return -ENOMEM;
 		dev->queues[0] = nvmeq;
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] NVMe: Refactor doorbell
  2013-10-11  8:48 ` [PATCH 1/2] NVMe: Refactor doorbell Matias Bjorling
@ 2013-10-11 15:39   ` Keith Busch
  0 siblings, 0 replies; 5+ messages in thread
From: Keith Busch @ 2013-10-11 15:39 UTC (permalink / raw)
  To: Matias Bjorling; +Cc: willy, linux-kernel, linux-nvme

On Fri, 11 Oct 2013, Matias Bjorling wrote:
> The doorbell code is repeated various places. Refactor it into its own function
> for clarity.
>
> Signed-off-by: Matias Bjorling <m@bjorling.me>

Looks good to me.
Reviewed-by: Keith Busch <keith.busch@intel.com>

> ---
> drivers/block/nvme-core.c | 29 +++++++++++++----------------
> 1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> index da52092..40998d5 100644
> --- a/drivers/block/nvme-core.c
> +++ b/drivers/block/nvme-core.c
> @@ -242,6 +242,13 @@ void put_nvmeq(struct nvme_queue *nvmeq)
> 	put_cpu();
> }
>
> +static inline void nvme_ring_doorbell(struct nvme_queue *nvmeq)
> +{
> +	if (++nvmeq->sq_tail == nvmeq->q_depth)
> +		nvmeq->sq_tail = 0;
> +	writel(nvmeq->sq_tail, nvmeq->q_db);
> +}
> +
> /**
>  * nvme_submit_cmd() - Copy a command into a queue and ring the doorbell
>  * @nvmeq: The queue to use
> @@ -252,14 +259,10 @@ void put_nvmeq(struct nvme_queue *nvmeq)
> static int nvme_submit_cmd(struct nvme_queue *nvmeq, struct nvme_command *cmd)
> {
> 	unsigned long flags;
> -	u16 tail;
> +
> 	spin_lock_irqsave(&nvmeq->q_lock, flags);
> -	tail = nvmeq->sq_tail;
> -	memcpy(&nvmeq->sq_cmds[tail], cmd, sizeof(*cmd));
> -	if (++tail == nvmeq->q_depth)
> -		tail = 0;
> -	writel(tail, nvmeq->q_db);
> -	nvmeq->sq_tail = tail;
> +	memcpy(&nvmeq->sq_cmds[nvmeq->sq_tail], cmd, sizeof(*cmd));
> +	nvme_ring_doorbell(nvmeq);
> 	spin_unlock_irqrestore(&nvmeq->q_lock, flags);
>
> 	return 0;
> @@ -619,9 +622,7 @@ static int nvme_submit_discard(struct nvme_queue *nvmeq, struct nvme_ns *ns,
> 	cmnd->dsm.nr = 0;
> 	cmnd->dsm.attributes = cpu_to_le32(NVME_DSMGMT_AD);
>
> -	if (++nvmeq->sq_tail == nvmeq->q_depth)
> -		nvmeq->sq_tail = 0;
> -	writel(nvmeq->sq_tail, nvmeq->q_db);
> +	nvme_ring_doorbell(nvmeq);
>
> 	return 0;
> }
> @@ -636,9 +637,7 @@ static int nvme_submit_flush(struct nvme_queue *nvmeq, struct nvme_ns *ns,
> 	cmnd->common.command_id = cmdid;
> 	cmnd->common.nsid = cpu_to_le32(ns->ns_id);
>
> -	if (++nvmeq->sq_tail == nvmeq->q_depth)
> -		nvmeq->sq_tail = 0;
> -	writel(nvmeq->sq_tail, nvmeq->q_db);
> +	nvme_ring_doorbell(nvmeq);
>
> 	return 0;
> }
> @@ -729,9 +728,7 @@ static int nvme_submit_bio_queue(struct nvme_queue *nvmeq, struct nvme_ns *ns,
> 	cmnd->rw.dsmgmt = cpu_to_le32(dsmgmt);
>
> 	nvme_start_io_acct(bio);
> -	if (++nvmeq->sq_tail == nvmeq->q_depth)
> -		nvmeq->sq_tail = 0;
> -	writel(nvmeq->sq_tail, nvmeq->q_db);
> +	nvme_ring_doorbell(nvmeq);
>
> 	return 0;
>
> -- 
> 1.8.1.2
>
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://merlin.infradead.org/mailman/listinfo/linux-nvme
>

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-11 15:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-11  8:48 [PATCH 0/2] NVMe: Refactoring v2 Matias Bjorling
2013-10-11  8:48 ` [PATCH 1/2] NVMe: Refactor doorbell Matias Bjorling
2013-10-11 15:39   ` Keith Busch
2013-10-11  8:48 ` [PATCH 2/2] NVMe: Extract admin queue size Matias Bjorling
  -- strict thread matches above, loose matches on Subject: below --
2013-10-10 18:29 [PATCH 0/2] NVMe: Refactoring Matias Bjorling
2013-10-10 18:29 ` [PATCH 1/2] NVMe: Refactor doorbell Matias Bjorling

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox