* [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
* 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
* [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
* [PATCH 0/2] NVMe: Refactoring @ 2013-10-10 18:29 Matias Bjorling 2013-10-10 18:29 ` [PATCH 1/2] NVMe: Refactor doorbell Matias Bjorling 0 siblings, 1 reply; 5+ messages in thread From: Matias Bjorling @ 2013-10-10 18:29 UTC (permalink / raw) To: willy; +Cc: linux-kernel, linux-nvme, Matias Bjorling Two refactor patches that increases the 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. 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-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
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