From c38dbbfab1bc47b0f3a1eceea0fa45e44c477092 Mon Sep 17 00:00:00 2001 From: James Smart Date: Thu, 20 Jun 2019 13:07:00 -0700 Subject: nvme-fcloop: fix inconsistent lock state warnings With extra debug on, inconsistent lock state warnings are being called out as the tfcp_req->reqlock is being taken out without irq, while some calling sequences have the sequence in a softirq state. Change the lock taking/release to raise/drop irq. Signed-off-by: James Smart Reviewed-by: Minwoo Im Signed-off-by: Christoph Hellwig --- drivers/nvme/target/fcloop.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c index b8c1cc54a0db..e64969d2a7c5 100644 --- a/drivers/nvme/target/fcloop.c +++ b/drivers/nvme/target/fcloop.c @@ -434,7 +434,7 @@ fcloop_fcp_recv_work(struct work_struct *work) int ret = 0; bool aborted = false; - spin_lock(&tfcp_req->reqlock); + spin_lock_irq(&tfcp_req->reqlock); switch (tfcp_req->inistate) { case INI_IO_START: tfcp_req->inistate = INI_IO_ACTIVE; @@ -443,11 +443,11 @@ fcloop_fcp_recv_work(struct work_struct *work) aborted = true; break; default: - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); WARN_ON(1); return; } - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); if (unlikely(aborted)) ret = -ECANCELED; @@ -469,7 +469,7 @@ fcloop_fcp_abort_recv_work(struct work_struct *work) struct nvmefc_fcp_req *fcpreq; bool completed = false; - spin_lock(&tfcp_req->reqlock); + spin_lock_irq(&tfcp_req->reqlock); fcpreq = tfcp_req->fcpreq; switch (tfcp_req->inistate) { case INI_IO_ABORTED: @@ -478,11 +478,11 @@ fcloop_fcp_abort_recv_work(struct work_struct *work) completed = true; break; default: - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); WARN_ON(1); return; } - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); if (unlikely(completed)) { /* remove reference taken in original abort downcall */ @@ -494,9 +494,9 @@ fcloop_fcp_abort_recv_work(struct work_struct *work) nvmet_fc_rcv_fcp_abort(tfcp_req->tport->targetport, &tfcp_req->tgt_fcp_req); - spin_lock(&tfcp_req->reqlock); + spin_lock_irq(&tfcp_req->reqlock); tfcp_req->fcpreq = NULL; - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); fcloop_call_host_done(fcpreq, tfcp_req, -ECANCELED); /* call_host_done releases reference for abort downcall */ @@ -513,10 +513,10 @@ fcloop_tgt_fcprqst_done_work(struct work_struct *work) container_of(work, struct fcloop_fcpreq, tio_done_work); struct nvmefc_fcp_req *fcpreq; - spin_lock(&tfcp_req->reqlock); + spin_lock_irq(&tfcp_req->reqlock); fcpreq = tfcp_req->fcpreq; tfcp_req->inistate = INI_IO_COMPLETED; - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); fcloop_call_host_done(fcpreq, tfcp_req, tfcp_req->status); } @@ -621,12 +621,12 @@ fcloop_fcp_op(struct nvmet_fc_target_port *tgtport, int fcp_err = 0, active, aborted; u8 op = tgt_fcpreq->op; - spin_lock(&tfcp_req->reqlock); + spin_lock_irq(&tfcp_req->reqlock); fcpreq = tfcp_req->fcpreq; active = tfcp_req->active; aborted = tfcp_req->aborted; tfcp_req->active = true; - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); if (unlikely(active)) /* illegal - call while i/o active */ @@ -634,9 +634,9 @@ fcloop_fcp_op(struct nvmet_fc_target_port *tgtport, if (unlikely(aborted)) { /* target transport has aborted i/o prior */ - spin_lock(&tfcp_req->reqlock); + spin_lock_irq(&tfcp_req->reqlock); tfcp_req->active = false; - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); tgt_fcpreq->transferred_length = 0; tgt_fcpreq->fcp_error = -ECANCELED; tgt_fcpreq->done(tgt_fcpreq); @@ -693,9 +693,9 @@ fcloop_fcp_op(struct nvmet_fc_target_port *tgtport, break; } - spin_lock(&tfcp_req->reqlock); + spin_lock_irq(&tfcp_req->reqlock); tfcp_req->active = false; - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); tgt_fcpreq->transferred_length = xfrlen; tgt_fcpreq->fcp_error = fcp_err; @@ -715,9 +715,9 @@ fcloop_tgt_fcp_abort(struct nvmet_fc_target_port *tgtport, * (one doing io, other doing abort) and only kills ops posted * after the abort request */ - spin_lock(&tfcp_req->reqlock); + spin_lock_irq(&tfcp_req->reqlock); tfcp_req->aborted = true; - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); tfcp_req->status = NVME_SC_INTERNAL; @@ -765,7 +765,7 @@ fcloop_fcp_abort(struct nvme_fc_local_port *localport, return; /* break initiator/target relationship for io */ - spin_lock(&tfcp_req->reqlock); + spin_lock_irq(&tfcp_req->reqlock); switch (tfcp_req->inistate) { case INI_IO_START: case INI_IO_ACTIVE: @@ -775,11 +775,11 @@ fcloop_fcp_abort(struct nvme_fc_local_port *localport, abortio = false; break; default: - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); WARN_ON(1); return; } - spin_unlock(&tfcp_req->reqlock); + spin_unlock_irq(&tfcp_req->reqlock); if (abortio) /* leave the reference while the work item is scheduled */ -- cgit v1.2.3 From e0620bf858d3f5e7121d9e429cf7a8f04ab29bf7 Mon Sep 17 00:00:00 2001 From: James Smart Date: Thu, 20 Jun 2019 13:17:01 -0700 Subject: nvme-fcloop: resolve warnings on RCU usage and sleep warnings With additional debugging enabled, seeing warnings for suspicious RCU usage or Sleeping function called from invalid context. These both map to allocation of a work structure which is currently GFP_KERNEL, meaning it can sleep. For the RCU warning, the sequence was sleeping while holding the RCU lock. Convert the allocation to GFP_ATOMIC. Signed-off-by: James Smart Reviewed-by: Minwoo Im Signed-off-by: Christoph Hellwig --- drivers/nvme/target/fcloop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c index e64969d2a7c5..b50b53db3746 100644 --- a/drivers/nvme/target/fcloop.c +++ b/drivers/nvme/target/fcloop.c @@ -535,7 +535,7 @@ fcloop_fcp_req(struct nvme_fc_local_port *localport, if (!rport->targetport) return -ECONNREFUSED; - tfcp_req = kzalloc(sizeof(*tfcp_req), GFP_KERNEL); + tfcp_req = kzalloc(sizeof(*tfcp_req), GFP_ATOMIC); if (!tfcp_req) return -ENOMEM; -- cgit v1.2.3 From 21774222324e018f064d4fbb661e3c09c2bcaad0 Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Wed, 26 Jun 2019 10:09:02 +0800 Subject: nvme-pci: make nvme_dev_pm_ops static Fix sparse warning: drivers/nvme/host/pci.c:2926:25: warning: symbol 'nvme_dev_pm_ops' was not declared. Should it be static? Reported-by: Hulk Robot Signed-off-by: YueHaibing Reviewed-by: Minwoo Im Reviewed-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 189352081994..f50013369cc5 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2923,7 +2923,7 @@ static int nvme_simple_resume(struct device *dev) return 0; } -const struct dev_pm_ops nvme_dev_pm_ops = { +static const struct dev_pm_ops nvme_dev_pm_ops = { .suspend = nvme_suspend, .resume = nvme_resume, .freeze = nvme_simple_suspend, -- cgit v1.2.3 From 4fe06923f5181d57178e01add4ba54e269c59e9e Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 28 Jun 2019 09:17:48 +0200 Subject: nvme-pci: don't fall back to a 32-bit DMA mask Since Linux 5.0 drivers can safely set the largest DMA mask supported by the device, and don't need fallbacks to work around the dma mapping implementations. Signed-off-by: Christoph Hellwig Reviewed-by: Sagi Grimberg --- drivers/nvme/host/pci.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index f50013369cc5..49c1fc9907a6 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2289,8 +2289,7 @@ static int nvme_pci_enable(struct nvme_dev *dev) pci_set_master(pdev); - if (dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(64)) && - dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(32))) + if (dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(64))) goto disable; if (readl(dev->bar + NVME_REG_CSTS) == -1) { -- cgit v1.2.3 From 0298d5435276e7795b0b939d74827f6e775e7009 Mon Sep 17 00:00:00 2001 From: Alan Mikhak Date: Mon, 8 Jul 2019 10:24:12 -0700 Subject: nvme-pci: don't create a read hctx mapping without read queues Only request an IRQ mapping for read queues if at least one read queue is being allocted, as nvme_pci_map_queues() will later on ignore the unnecessary mapping request should nvme_dev_add() request such an IRQ mapping even though no read queues are being allocated. However, nvme_dev_add() can avoid making the request by checking the number of read queues without assuming. This would bring it more in line with nvme_setup_irqs() and nvme_calc_irq_sets(). Signed-off-by: Alan Mikhak Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 49c1fc9907a6..0423ddd97f4b 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2250,7 +2250,9 @@ static int nvme_dev_add(struct nvme_dev *dev) if (!dev->ctrl.tagset) { dev->tagset.ops = &nvme_mq_ops; dev->tagset.nr_hw_queues = dev->online_queues - 1; - dev->tagset.nr_maps = 2; /* default + read */ + dev->tagset.nr_maps = 1; /* default */ + if (dev->io_queues[HCTX_TYPE_READ]) + dev->tagset.nr_maps++; if (dev->io_queues[HCTX_TYPE_POLL]) dev->tagset.nr_maps++; dev->tagset.timeout = NVME_IO_TIMEOUT; -- cgit v1.2.3 From bfac8e9f55cf62a000b643a0081488badbe92d96 Mon Sep 17 00:00:00 2001 From: Alan Mikhak Date: Mon, 8 Jul 2019 10:05:11 -0700 Subject: nvme-pci: check for NULL return from pci_alloc_p2pmem() Modify nvme_alloc_sq_cmds() to call pci_free_p2pmem() to free the memory it allocated using pci_alloc_p2pmem() in case pci_p2pmem_virt_to_bus() returns null. Makes sure not to call pci_free_p2pmem() if pci_alloc_p2pmem() returned NULL, which can happen if CONFIG_PCI_P2PDMA is not configured. The current implementation is not expected to leak since pci_p2pmem_virt_to_bus() is expected to fail only if pci_alloc_p2pmem() returns null. However, checking the return value of pci_alloc_p2pmem() is more explicit. Signed-off-by: Alan Mikhak Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 0423ddd97f4b..ac2011b8dac1 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -1439,11 +1439,15 @@ static int nvme_alloc_sq_cmds(struct nvme_dev *dev, struct nvme_queue *nvmeq, if (qid && dev->cmb_use_sqes && (dev->cmbsz & NVME_CMBSZ_SQS)) { nvmeq->sq_cmds = pci_alloc_p2pmem(pdev, SQ_SIZE(depth)); - nvmeq->sq_dma_addr = pci_p2pmem_virt_to_bus(pdev, - nvmeq->sq_cmds); - if (nvmeq->sq_dma_addr) { - set_bit(NVMEQ_SQ_CMB, &nvmeq->flags); - return 0; + if (nvmeq->sq_cmds) { + nvmeq->sq_dma_addr = pci_p2pmem_virt_to_bus(pdev, + nvmeq->sq_cmds); + if (nvmeq->sq_dma_addr) { + set_bit(NVMEQ_SQ_CMB, &nvmeq->flags); + return 0; + } + + pci_free_p2pmem(pdev, nvmeq->sq_cmds, SQ_SIZE(depth)); } } -- cgit v1.2.3 From 7637de311bd2124b298a072852448b940d8a34b9 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 3 Jul 2019 09:54:44 -0700 Subject: nvme-pci: limit max_hw_sectors based on the DMA max mapping size When running a NVMe device that is attached to a addressing challenged PCIe root port that requires bounce buffering, our request sizes can easily overflow the swiotlb bounce buffer size. Limit the maximum I/O size to the limit exposed by the DMA mapping subsystem. Signed-off-by: Christoph Hellwig Reported-by: Atish Patra Tested-by: Atish Patra Reviewed-by: Sagi Grimberg --- drivers/nvme/host/pci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index ac2011b8dac1..bb970ca82517 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2503,7 +2503,8 @@ static void nvme_reset_work(struct work_struct *work) * Limit the max command size to prevent iod->sg allocations going * over a single page. */ - dev->ctrl.max_hw_sectors = NVME_MAX_KB_SZ << 1; + dev->ctrl.max_hw_sectors = min_t(u32, + NVME_MAX_KB_SZ << 1, dma_max_mapping_size(dev->dev) >> 9); dev->ctrl.max_segments = NVME_MAX_SEGS; /* -- cgit v1.2.3 From 91f6d7985310a5dc420066004142c54da2c627d8 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Wed, 26 Jun 2019 13:43:23 +0100 Subject: nvme-trace: fix spelling mistake "spcecific" -> "specific" There are two spelling mistakes in trace_seq_printf messages, fix these. Signed-off-by: Colin Ian King Reviewed-by: Minwoo Im Signed-off-by: Christoph Hellwig --- drivers/nvme/host/trace.c | 2 +- drivers/nvme/target/trace.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/host/trace.c b/drivers/nvme/host/trace.c index f01ad0fd60bb..6980ab827233 100644 --- a/drivers/nvme/host/trace.c +++ b/drivers/nvme/host/trace.c @@ -178,7 +178,7 @@ static const char *nvme_trace_fabrics_common(struct trace_seq *p, u8 *spc) { const char *ret = trace_seq_buffer_ptr(p); - trace_seq_printf(p, "spcecific=%*ph", 24, spc); + trace_seq_printf(p, "specific=%*ph", 24, spc); trace_seq_putc(p, 0); return ret; } diff --git a/drivers/nvme/target/trace.c b/drivers/nvme/target/trace.c index cdcdd14c6408..6af11d493271 100644 --- a/drivers/nvme/target/trace.c +++ b/drivers/nvme/target/trace.c @@ -146,7 +146,7 @@ static const char *nvmet_trace_fabrics_common(struct trace_seq *p, u8 *spc) { const char *ret = trace_seq_buffer_ptr(p); - trace_seq_printf(p, "spcecific=%*ph", 24, spc); + trace_seq_printf(p, "specific=%*ph", 24, spc); trace_seq_putc(p, 0); return ret; } -- cgit v1.2.3 From 4c0181bf6cc81716102308dc47779ad1f5aeded2 Mon Sep 17 00:00:00 2001 From: Tom Wu Date: Thu, 4 Jul 2019 10:19:54 +0000 Subject: nvme-trace: add delete completion and submission queue to admin cmds tracer The trace log for 'delete I/O submission queue' and 'delete I/O completion queue' command will look like as below: kworker/u49:1-3438 [003] .... 6693.070865: nvme_setup_cmd: nvme0: qid=0, cmdid=11, nsid=0, flags=0x0, meta=0x0, cmd=(nvme_admin_delete_sq sqid=1) kworker/u49:1-3438 [003] .... 6693.071171: nvme_setup_cmd: nvme0: qid=0, cmdid=8, nsid=0, flags=0x0, meta=0x0, cmd=(nvme_admin_delete_cq cqid=24) Signed-off-by: Tom Wu Reviewed-by: Max Gurtovoy Reviewed-by: Minwoo Im Reviewed-by: Israel Rukshin Signed-off-by: Christoph Hellwig --- drivers/nvme/host/trace.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/drivers/nvme/host/trace.c b/drivers/nvme/host/trace.c index 6980ab827233..9778eb0406b3 100644 --- a/drivers/nvme/host/trace.c +++ b/drivers/nvme/host/trace.c @@ -7,6 +7,17 @@ #include #include "trace.h" +static const char *nvme_trace_delete_sq(struct trace_seq *p, u8 *cdw10) +{ + const char *ret = trace_seq_buffer_ptr(p); + u16 sqid = get_unaligned_le16(cdw10); + + trace_seq_printf(p, "sqid=%u", sqid); + trace_seq_putc(p, 0); + + return ret; +} + static const char *nvme_trace_create_sq(struct trace_seq *p, u8 *cdw10) { const char *ret = trace_seq_buffer_ptr(p); @@ -23,6 +34,17 @@ static const char *nvme_trace_create_sq(struct trace_seq *p, u8 *cdw10) return ret; } +static const char *nvme_trace_delete_cq(struct trace_seq *p, u8 *cdw10) +{ + const char *ret = trace_seq_buffer_ptr(p); + u16 cqid = get_unaligned_le16(cdw10); + + trace_seq_printf(p, "cqid=%u", cqid); + trace_seq_putc(p, 0); + + return ret; +} + static const char *nvme_trace_create_cq(struct trace_seq *p, u8 *cdw10) { const char *ret = trace_seq_buffer_ptr(p); @@ -107,8 +129,12 @@ const char *nvme_trace_parse_admin_cmd(struct trace_seq *p, u8 opcode, u8 *cdw10) { switch (opcode) { + case nvme_admin_delete_sq: + return nvme_trace_delete_sq(p, cdw10); case nvme_admin_create_sq: return nvme_trace_create_sq(p, cdw10); + case nvme_admin_delete_cq: + return nvme_trace_delete_cq(p, cdw10); case nvme_admin_create_cq: return nvme_trace_create_cq(p, cdw10); case nvme_admin_identify: -- cgit v1.2.3 From 9d05a96e298aadb36e3ec971fab8d416e6fb7331 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 28 Jun 2019 09:53:30 -0700 Subject: nvmet: export I/O characteristics attributes in Identify Make the NVMe NAWUN, NAWUPF, NACWU, NPWG, NPWA, NPDG and NOWS attributes available to initator systems for the block backend. Signed-off-by: Bart Van Assche Reviewed-by: Martin K. Petersen Reviewed-by: Hannes Reinecke Signed-off-by: Christoph Hellwig --- drivers/nvme/target/admin-cmd.c | 3 +++ drivers/nvme/target/io-cmd-bdev.c | 39 +++++++++++++++++++++++++++++++++++++++ drivers/nvme/target/nvmet.h | 8 ++++++++ 3 files changed, 50 insertions(+) diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index 9f72d515fc4b..4dc12ea52f23 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c @@ -442,6 +442,9 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req) break; } + if (ns->bdev) + nvmet_bdev_set_limits(ns->bdev, id); + /* * We just provide a single LBA format that matches what the * underlying device reports. diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c index 7a1cf6437a6a..de0bff70ebb6 100644 --- a/drivers/nvme/target/io-cmd-bdev.c +++ b/drivers/nvme/target/io-cmd-bdev.c @@ -8,6 +8,45 @@ #include #include "nvmet.h" +void nvmet_bdev_set_limits(struct block_device *bdev, struct nvme_id_ns *id) +{ + const struct queue_limits *ql = &bdev_get_queue(bdev)->limits; + /* Number of physical blocks per logical block. */ + const u32 ppl = ql->physical_block_size / ql->logical_block_size; + /* Physical blocks per logical block, 0's based. */ + const __le16 ppl0b = to0based(ppl); + + /* + * For NVMe 1.2 and later, bit 1 indicates that the fields NAWUN, + * NAWUPF, and NACWU are defined for this namespace and should be + * used by the host for this namespace instead of the AWUN, AWUPF, + * and ACWU fields in the Identify Controller data structure. If + * any of these fields are zero that means that the corresponding + * field from the identify controller data structure should be used. + */ + id->nsfeat |= 1 << 1; + id->nawun = ppl0b; + id->nawupf = ppl0b; + id->nacwu = ppl0b; + + /* + * Bit 4 indicates that the fields NPWG, NPWA, NPDG, NPDA, and + * NOWS are defined for this namespace and should be used by + * the host for I/O optimization. + */ + id->nsfeat |= 1 << 4; + /* NPWG = Namespace Preferred Write Granularity. 0's based */ + id->npwg = ppl0b; + /* NPWA = Namespace Preferred Write Alignment. 0's based */ + id->npwa = id->npwg; + /* NPDG = Namespace Preferred Deallocate Granularity. 0's based */ + id->npdg = to0based(ql->discard_granularity / ql->logical_block_size); + /* NPDG = Namespace Preferred Deallocate Alignment */ + id->npda = id->npdg; + /* NOWS = Namespace Optimal Write Size */ + id->nows = to0based(ql->io_opt / ql->logical_block_size); +} + int nvmet_bdev_ns_enable(struct nvmet_ns *ns) { int ret; diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h index dc270944bb25..6ee66c610739 100644 --- a/drivers/nvme/target/nvmet.h +++ b/drivers/nvme/target/nvmet.h @@ -365,6 +365,7 @@ u16 nvmet_set_feat_async_event(struct nvmet_req *req, u32 mask); void nvmet_execute_async_event(struct nvmet_req *req); u16 nvmet_parse_connect_cmd(struct nvmet_req *req); +void nvmet_bdev_set_limits(struct block_device *bdev, struct nvme_id_ns *id); u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req); u16 nvmet_file_parse_io_cmd(struct nvmet_req *req); u16 nvmet_parse_admin_cmd(struct nvmet_req *req); @@ -492,4 +493,11 @@ static inline u32 nvmet_rw_len(struct nvmet_req *req) } u16 errno_to_nvme_status(struct nvmet_req *req, int errno); + +/* Convert a 32-bit number to a 16-bit 0's based number */ +static inline __le16 to0based(u32 a) +{ + return cpu_to_le16(max(1U, min(1U << 16, a)) - 1); +} + #endif /* _NVMET_H */ -- cgit v1.2.3 From 6605bdd59c21bb34c8f14ac4d6f2d419185f3528 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 28 Jun 2019 09:53:29 -0700 Subject: nvme: add I/O characteristics fields Several new fields have been introduced in version 1.4 of the NVMe spec at offsets that were defined as reserved in version 1.3d of the NVMe spec. Update the definition of the nvme_id_ns data structure such that it is in sync with version 1.4 of the NVMe spec. This change preserves backwards compatibility. Signed-off-by: Bart Van Assche Reviewed-by: Keith Busch Reviewed-by: Chaitanya Kulkarni Reviewed-by: Martin K. Petersen Reviewed-by: Hannes Reinecke Signed-off-by: Christoph Hellwig --- include/linux/nvme.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/include/linux/nvme.h b/include/linux/nvme.h index d98b2d8baf4e..01aa6a6c241d 100644 --- a/include/linux/nvme.h +++ b/include/linux/nvme.h @@ -315,7 +315,7 @@ struct nvme_id_ns { __u8 nmic; __u8 rescap; __u8 fpi; - __u8 rsvd33; + __u8 dlfeat; __le16 nawun; __le16 nawupf; __le16 nacwu; @@ -324,11 +324,17 @@ struct nvme_id_ns { __le16 nabspf; __le16 noiob; __u8 nvmcap[16]; - __u8 rsvd64[28]; + __le16 npwg; + __le16 npwa; + __le16 npdg; + __le16 npda; + __le16 nows; + __u8 rsvd74[18]; __le32 anagrpid; __u8 rsvd96[3]; __u8 nsattr; - __u8 rsvd100[4]; + __le16 nvmsetid; + __le16 endgid; __u8 nguid[16]; __u8 eui64[8]; struct nvme_lbaf lbaf[16]; -- cgit v1.2.3 From 81adb863349157c67ccec871e5ae5574600c50be Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 28 Jun 2019 09:53:31 -0700 Subject: nvme: set physical block size and optimal I/O size >From the NVMe 1.4 spec: NSFEAT bit 4 if set to 1: indicates that the fields NPWG, NPWA, NPDG, NPDA, and NOWS are defined for this namespace and should be used by the host for I/O optimization; [ ... ] Namespace Preferred Write Granularity (NPWG): This field indicates the smallest recommended write granularity in logical blocks for this namespace. This is a 0's based value. The size indicated should be less than or equal to Maximum Data Transfer Size (MDTS) that is specified in units of minimum memory page size. The value of this field may change if the namespace is reformatted. The size should be a multiple of Namespace Preferred Write Alignment (NPWA). Refer to section 8.25 for how this field is utilized to improve performance and endurance. [ ... ] Each Write, Write Uncorrectable, or Write Zeroes commands should address a multiple of Namespace Preferred Write Granularity (NPWG) (refer to Figure 245) and Stream Write Size (SWS) (refer to Figure 515) logical blocks (as expressed in the NLB field), and the SLBA field of the command should be aligned to Namespace Preferred Write Alignment (NPWA) (refer to Figure 245) for best performance. Signed-off-by: Bart Van Assche Reviewed-by: Hannes Reinecke Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 34 ++++++++++++++++++++++++++++++++-- drivers/nvme/host/nvme.h | 1 + 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index b2dd4e391f5c..5417110cbf1b 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -1626,6 +1626,7 @@ static void nvme_update_disk_info(struct gendisk *disk, { sector_t capacity = le64_to_cpu(id->nsze) << (ns->lba_shift - 9); unsigned short bs = 1 << ns->lba_shift; + u32 atomic_bs, phys_bs, io_opt; if (ns->lba_shift > PAGE_SHIFT) { /* unsupported block size, set capacity to 0 later */ @@ -1634,9 +1635,37 @@ static void nvme_update_disk_info(struct gendisk *disk, blk_mq_freeze_queue(disk->queue); blk_integrity_unregister(disk); + if (id->nabo == 0) { + /* + * Bit 1 indicates whether NAWUPF is defined for this namespace + * and whether it should be used instead of AWUPF. If NAWUPF == + * 0 then AWUPF must be used instead. + */ + if (id->nsfeat & (1 << 1) && id->nawupf) + atomic_bs = (1 + le16_to_cpu(id->nawupf)) * bs; + else + atomic_bs = (1 + ns->ctrl->subsys->awupf) * bs; + } else { + atomic_bs = bs; + } + phys_bs = bs; + io_opt = bs; + if (id->nsfeat & (1 << 4)) { + /* NPWG = Namespace Preferred Write Granularity */ + phys_bs *= 1 + le16_to_cpu(id->npwg); + /* NOWS = Namespace Optimal Write Size */ + io_opt *= 1 + le16_to_cpu(id->nows); + } + blk_queue_logical_block_size(disk->queue, bs); - blk_queue_physical_block_size(disk->queue, bs); - blk_queue_io_min(disk->queue, bs); + /* + * Linux filesystems assume writing a single physical block is + * an atomic operation. Hence limit the physical block size to the + * value of the Atomic Write Unit Power Fail parameter. + */ + blk_queue_physical_block_size(disk->queue, min(phys_bs, atomic_bs)); + blk_queue_io_min(disk->queue, phys_bs); + blk_queue_io_opt(disk->queue, io_opt); if (ns->ms && !ns->ext && (ns->ctrl->ops->flags & NVME_F_METADATA_SUPPORTED)) @@ -2433,6 +2462,7 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id) memcpy(subsys->firmware_rev, id->fr, sizeof(subsys->firmware_rev)); subsys->vendor_id = le16_to_cpu(id->vid); subsys->cmic = id->cmic; + subsys->awupf = le16_to_cpu(id->awupf); #ifdef CONFIG_NVME_MULTIPATH subsys->iopolicy = NVME_IOPOLICY_NUMA; #endif diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index ea45d7d393ad..716a876119c8 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -283,6 +283,7 @@ struct nvme_subsystem { char firmware_rev[8]; u8 cmic; u16 vendor_id; + u16 awupf; /* 0's based awupf value. */ struct ida ns_ida; #ifdef CONFIG_NVME_MULTIPATH enum nvme_iopolicy iopolicy; -- cgit v1.2.3 From ca7ae5c966bd4c00626d6ba05d68219f3c1fba36 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 4 Jul 2019 08:10:46 +0200 Subject: nvme-multipath: factor out a nvme_path_is_disabled helper Factor our a common helper to check if a path has been disabled by something other than the per-namespace ANA state. Signed-off-by: Hannes Reinecke [hch: split from a bigger patch] Signed-off-by: Christoph Hellwig --- drivers/nvme/host/multipath.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 499acf07d61a..5a6dbb422a9c 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -123,14 +123,19 @@ void nvme_mpath_clear_current_path(struct nvme_ns *ns) } } +static bool nvme_path_is_disabled(struct nvme_ns *ns) +{ + return ns->ctrl->state != NVME_CTRL_LIVE || + test_bit(NVME_NS_ANA_PENDING, &ns->flags); +} + static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head, int node) { int found_distance = INT_MAX, fallback_distance = INT_MAX, distance; struct nvme_ns *found = NULL, *fallback = NULL, *ns; list_for_each_entry_rcu(ns, &head->list, siblings) { - if (ns->ctrl->state != NVME_CTRL_LIVE || - test_bit(NVME_NS_ANA_PENDING, &ns->flags)) + if (nvme_path_is_disabled(ns)) continue; if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_NUMA) @@ -184,8 +189,7 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head, for (ns = nvme_next_ns(head, old); ns != old; ns = nvme_next_ns(head, ns)) { - if (ns->ctrl->state != NVME_CTRL_LIVE || - test_bit(NVME_NS_ANA_PENDING, &ns->flags)) + if (nvme_path_is_disabled(ns)) continue; if (ns->ana_state == NVME_ANA_OPTIMIZED) { -- cgit v1.2.3 From 2032d074716a811440aa9cd2e971a0716646d6af Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 4 Jul 2019 08:10:46 +0200 Subject: nvme-multipath: also check for a disabled path if there is a single sibling When we have a singular list in nvme_round_robin_path() we still need to check its validity. Signed-off-by: Hannes Reinecke Signed-off-by: Christoph Hellwig --- drivers/nvme/host/multipath.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 5a6dbb422a9c..9b6dc11fa559 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -183,8 +183,11 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head, { struct nvme_ns *ns, *found, *fallback = NULL; - if (list_is_singular(&head->list)) + if (list_is_singular(&head->list)) { + if (nvme_path_is_disabled(old)) + return NULL; return old; + } for (ns = nvme_next_ns(head, old); ns != old; -- cgit v1.2.3 From 04e70bd4a0264a3d488a9eff6e116d7dc9a77967 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 4 Jul 2019 08:10:47 +0200 Subject: nvme-multipath: do not select namespaces which are about to be removed nvme_ns_remove() will first set the NVME_NS_REMOVING flag before removing it from the list at the very last step. So to avoid selecting a namespace in nvme_find_path() which is about to be removed check the NVME_NS_REMOVING flag, too, when selecting a new path. Signed-off-by: Hannes Reinecke Signed-off-by: Christoph Hellwig --- drivers/nvme/host/multipath.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 9b6dc11fa559..a9a927677970 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -126,7 +126,8 @@ void nvme_mpath_clear_current_path(struct nvme_ns *ns) static bool nvme_path_is_disabled(struct nvme_ns *ns) { return ns->ctrl->state != NVME_CTRL_LIVE || - test_bit(NVME_NS_ANA_PENDING, &ns->flags); + test_bit(NVME_NS_ANA_PENDING, &ns->flags) || + test_bit(NVME_NS_REMOVING, &ns->flags); } static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head, int node) -- cgit v1.2.3 From 5ba895033b8e8257451e6f85e6e516c3b3ce1a68 Mon Sep 17 00:00:00 2001 From: Mikhail Skorzhinskii Date: Thu, 4 Jul 2019 10:01:48 +0200 Subject: nvmet: print a hint while rejecting NSID 0 or 0xffffffff Adding this hint for the sake of convenience. It was spotted that a few times people spent some time before understanding what is exactly wrong in configuration process. This should save a few time in such situations, especially for people who is not very confident with NVMe requirements. Signed-off-by: Mikhail Skorzhinskii Reviewed-by: Sagi Grimberg Signed-off-by: Christoph Hellwig --- drivers/nvme/target/configfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c index 08dd5af357f7..cd52b9f15376 100644 --- a/drivers/nvme/target/configfs.c +++ b/drivers/nvme/target/configfs.c @@ -588,8 +588,10 @@ static struct config_group *nvmet_ns_make(struct config_group *group, goto out; ret = -EINVAL; - if (nsid == 0 || nsid == NVME_NSID_ALL) + if (nsid == 0 || nsid == NVME_NSID_ALL) { + pr_err("invalid nsid %#x", nsid); goto out; + } ret = -ENOMEM; ns = nvmet_ns_alloc(subsys, nsid); -- cgit v1.2.3 From 958f2a0f8121ae36a5cbff383ab94fadf1fba5eb Mon Sep 17 00:00:00 2001 From: Mikhail Skorzhinskii Date: Thu, 4 Jul 2019 09:59:18 +0200 Subject: nvme-tcp: set the STABLE_WRITES flag when data digests are enabled There was a few false alarms sighted on target side about wrong data digest while performing high throughput load to XFS filesystem shared through NVMoF TCP. This flag tells the rest of the kernel to ensure that the data buffer does not change while the write is in flight. It incurs a performance penalty, so only enable it when it is actually needed, i.e. when we are calculating data digests. Although even with this change in place, ext2 users can steel experience false positives, as ext2 is not respecting this flag. This may be apply to vfat as well. Signed-off-by: Mikhail Skorzhinskii Signed-off-by: Mike Playle Reviewed-by: Sagi Grimberg Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 5417110cbf1b..f4340dc1d399 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -3304,6 +3305,10 @@ static int nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) goto out_free_ns; } + if (ctrl->opts->data_digest) + ns->queue->backing_dev_info->capabilities + |= BDI_CAP_STABLE_WRITES; + blk_queue_flag_set(QUEUE_FLAG_NONROT, ns->queue); if (ctrl->ops->flags & NVME_F_PCI_P2PDMA) blk_queue_flag_set(QUEUE_FLAG_PCI_P2PDMA, ns->queue); -- cgit v1.2.3 From 37c15219599f7a4baa73f6e3432afc69ba7cc530 Mon Sep 17 00:00:00 2001 From: Mikhail Skorzhinskii Date: Mon, 8 Jul 2019 12:31:29 +0200 Subject: nvme-tcp: don't use sendpage for SLAB pages According to commit a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab objects") and previous discussion, tcp_sendpage should not be used for pages that is managed by SLAB, as SLAB is not taking page reference counters into consideration. Signed-off-by: Mikhail Skorzhinskii Reviewed-by: Sagi Grimberg Signed-off-by: Christoph Hellwig --- drivers/nvme/host/tcp.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 08a2501b9357..606b13d35d16 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -860,7 +860,14 @@ static int nvme_tcp_try_send_data(struct nvme_tcp_request *req) else flags |= MSG_MORE; - ret = kernel_sendpage(queue->sock, page, offset, len, flags); + /* can't zcopy slab pages */ + if (unlikely(PageSlab(page))) { + ret = sock_no_sendpage(queue->sock, page, offset, len, + flags); + } else { + ret = kernel_sendpage(queue->sock, page, offset, len, + flags); + } if (ret <= 0) return ret; -- cgit v1.2.3 From 4c73cbdff1119d088ed16d63def59ad32b11b18f Mon Sep 17 00:00:00 2001 From: James Smart Date: Fri, 28 Jun 2019 17:26:08 -0700 Subject: nvme-fc: fix module unloads while lports still pending Current code allows the module to be unloaded even if there are pending data structures, such as localports and controllers on the localports, that have yet to hit their reference counting to remove them. Fix by having exit entrypoint explicitly delete every controller, which in turn will remove references on the remoteports and localports causing them to be deleted as well. The exit entrypoint, after initiating the deletes, will wait for the last localport to be deleted before continuing. Signed-off-by: James Smart Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fc.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c index 9b497d785ed7..1a391aa1f7d5 100644 --- a/drivers/nvme/host/fc.c +++ b/drivers/nvme/host/fc.c @@ -204,6 +204,9 @@ static DEFINE_IDA(nvme_fc_ctrl_cnt); static struct workqueue_struct *nvme_fc_wq; +static bool nvme_fc_waiting_to_unload; +static DECLARE_COMPLETION(nvme_fc_unload_proceed); + /* * These items are short-term. They will eventually be moved into * a generic FC class. See comments in module init. @@ -229,6 +232,8 @@ nvme_fc_free_lport(struct kref *ref) /* remove from transport list */ spin_lock_irqsave(&nvme_fc_lock, flags); list_del(&lport->port_list); + if (nvme_fc_waiting_to_unload && list_empty(&nvme_fc_lport_list)) + complete(&nvme_fc_unload_proceed); spin_unlock_irqrestore(&nvme_fc_lock, flags); ida_simple_remove(&nvme_fc_local_port_cnt, lport->localport.port_num); @@ -3456,11 +3461,51 @@ out_destroy_wq: return ret; } +static void +nvme_fc_delete_controllers(struct nvme_fc_rport *rport) +{ + struct nvme_fc_ctrl *ctrl; + + spin_lock(&rport->lock); + list_for_each_entry(ctrl, &rport->ctrl_list, ctrl_list) { + dev_warn(ctrl->ctrl.device, + "NVME-FC{%d}: transport unloading: deleting ctrl\n", + ctrl->cnum); + nvme_delete_ctrl(&ctrl->ctrl); + } + spin_unlock(&rport->lock); +} + +static void +nvme_fc_cleanup_for_unload(void) +{ + struct nvme_fc_lport *lport; + struct nvme_fc_rport *rport; + + list_for_each_entry(lport, &nvme_fc_lport_list, port_list) { + list_for_each_entry(rport, &lport->endp_list, endp_list) { + nvme_fc_delete_controllers(rport); + } + } +} + static void __exit nvme_fc_exit_module(void) { - /* sanity check - all lports should be removed */ - if (!list_empty(&nvme_fc_lport_list)) - pr_warn("%s: localport list not empty\n", __func__); + unsigned long flags; + bool need_cleanup = false; + + spin_lock_irqsave(&nvme_fc_lock, flags); + nvme_fc_waiting_to_unload = true; + if (!list_empty(&nvme_fc_lport_list)) { + need_cleanup = true; + nvme_fc_cleanup_for_unload(); + } + spin_unlock_irqrestore(&nvme_fc_lock, flags); + if (need_cleanup) { + pr_info("%s: waiting for ctlr deletes\n", __func__); + wait_for_completion(&nvme_fc_unload_proceed); + pr_info("%s: ctrl deletes complete\n", __func__); + } nvmf_unregister_transport(&nvme_fc_transport); -- cgit v1.2.3 From 420dc733f980246f2179e0144f9cedab9ad4a91e Mon Sep 17 00:00:00 2001 From: Sagi Grimberg Date: Wed, 10 Jul 2019 09:31:31 -0700 Subject: nvme: fix regression upon hot device removal and insertion When we validate the new controller id, we want to skip controllers that are either deleting or dead. Fix the check to do that and not on the newly added controller. Fixes: 1b1031ca63b2 ("nvme: validate cntlid during controller initialisation") Reported-by: Jon Derrick Tested-by: Jon Derrick Signed-off-by: Sagi Grimberg Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index f4340dc1d399..3077cd4d75bf 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2416,8 +2416,8 @@ static bool nvme_validate_cntlid(struct nvme_subsystem *subsys, lockdep_assert_held(&nvme_subsystems_lock); list_for_each_entry(tmp, &subsys->ctrls, subsys_entry) { - if (ctrl->state == NVME_CTRL_DELETING || - ctrl->state == NVME_CTRL_DEAD) + if (tmp->state == NVME_CTRL_DELETING || + tmp->state == NVME_CTRL_DEAD) continue; if (tmp->cntlid == ctrl->cntlid) { -- cgit v1.2.3