From 3a5088c8c11ac9d56f7e90acaaae9e0e98c1ff94 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Sat, 15 Apr 2017 20:38:22 +0800 Subject: block: respect BLK_MQ_F_NO_SCHED If one driver claims that it doesn't support io scheduler via BLK_MQ_F_NO_SCHED, we should not allow to change and show the availabe io schedulers. This patch adds check to enhance this behaviour. Signed-off-by: Ming Lei Signed-off-by: Jens Axboe --- block/elevator.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/block/elevator.c b/block/elevator.c index dbeecf7be719..4d9084a14c10 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -1098,12 +1098,20 @@ int elevator_change(struct request_queue *q, const char *name) } EXPORT_SYMBOL(elevator_change); +static inline bool elv_support_iosched(struct request_queue *q) +{ + if (q->mq_ops && q->tag_set && (q->tag_set->flags & + BLK_MQ_F_NO_SCHED)) + return false; + return true; +} + ssize_t elv_iosched_store(struct request_queue *q, const char *name, size_t count) { int ret; - if (!(q->mq_ops || q->request_fn)) + if (!(q->mq_ops || q->request_fn) || !elv_support_iosched(q)) return count; ret = __elevator_change(q, name); @@ -1135,7 +1143,7 @@ ssize_t elv_iosched_show(struct request_queue *q, char *name) len += sprintf(name+len, "[%s] ", elv->elevator_name); continue; } - if (__e->uses_mq && q->mq_ops) + if (__e->uses_mq && q->mq_ops && elv_support_iosched(q)) len += sprintf(name+len, "%s ", __e->elevator_name); else if (!__e->uses_mq && !q->mq_ops) len += sprintf(name+len, "%s ", __e->elevator_name); -- cgit v1.2.3 From 4981d04dd8f1ab19e2cce008da556d7f099b6e68 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Sat, 15 Apr 2017 20:38:23 +0800 Subject: mtip32xx: pass BLK_MQ_F_NO_SCHED The recent introduced MQ IO scheduler breaks mtip32xx in the following way. mtip32xx use the 'request_index' passed to .init_request() as hardware tag index for initializing hardware queue, and it actually require that rq->tag is always same with 'request_index' passed to .init_request(). Current blk-mq IO scheduler can't guarantee this point, so this patch passes BLK_MQ_F_NO_SCHED and at least make mtip32xx working. This patch fixes the following strange hardware failure. The issue can be triggered easily when doing I/O with mq-deadline enabled. [ 186.972578] {1}[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 32993 [ 186.972578] {1}[Hardware Error]: event severity: fatal [ 186.972579] {1}[Hardware Error]: Error 0, type: fatal [ 186.972580] {1}[Hardware Error]: section_type: PCIe error [ 186.972580] {1}[Hardware Error]: port_type: 0, PCIe end point [ 186.972581] {1}[Hardware Error]: version: 1.0 [ 186.972581] {1}[Hardware Error]: command: 0x0407, status: 0x0010 [ 186.972582] {1}[Hardware Error]: device_id: 0000:07:00.0 [ 186.972582] {1}[Hardware Error]: slot: 4 [ 186.972583] {1}[Hardware Error]: secondary_bus: 0x00 [ 186.972583] {1}[Hardware Error]: vendor_id: 0x1344, device_id: 0x5150 [ 186.972584] {1}[Hardware Error]: class_code: 008001 [ 186.972585] Kernel panic - not syncing: Fatal hardware error! Reported-by: Jozef Mikovic Signed-off-by: Ming Lei Signed-off-by: Jens Axboe --- drivers/block/mtip32xx/mtip32xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index f96ab717534c..1d1dc11aa5fa 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -3969,7 +3969,7 @@ static int mtip_block_initialize(struct driver_data *dd) dd->tags.reserved_tags = 1; dd->tags.cmd_size = sizeof(struct mtip_cmd); dd->tags.numa_node = dd->numa_node; - dd->tags.flags = BLK_MQ_F_SHOULD_MERGE; + dd->tags.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_NO_SCHED; dd->tags.driver_data = dd; dd->tags.timeout = MTIP_NCQ_CMD_TIMEOUT_MS; -- cgit v1.2.3 From ff5350a86b20de23991e474e006e2ff2732b218e Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Thu, 20 Apr 2017 13:37:55 -0700 Subject: nvme: Adjust the Samsung APST quirk I got a couple more reports: the Samsung APST issues appears to affect multiple 950-series devices in Dell XPS 15 9550 and Precision 5510 laptops. Change the quirk: rather than blacklisting the firmware on the first problematic SSD that was reported, disable APST on all 144d:a802 devices if they're installed in the two affected Dell models. While we're at it, disable only the deepest sleep state instead of all of them -- the reporters say that this is sufficient to fix the problem. (I have a device that appears to be entirely identical to one of the affected devices, but I have a different Dell laptop, so it's not the case that all Samsung devices with firmware BXW75D0Q are broken under all circumstances.) Samsung engineers have an affected system, and hopefully they'll give us a better workaround some time soon. In the mean time, this should minimize regressions. See https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1678184 Cc: Kai-Heng Feng Signed-off-by: Andy Lutomirski Signed-off-by: Jens Axboe --- drivers/nvme/host/core.c | 18 ++++++++---------- drivers/nvme/host/nvme.h | 5 +++++ drivers/nvme/host/pci.c | 26 +++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 9583a5f58a1d..00b2818dac31 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -1315,6 +1315,14 @@ static void nvme_configure_apst(struct nvme_ctrl *ctrl) if (target) table->entries[state] = target; + /* + * Don't allow transitions to the deepest state + * if it's quirked off. + */ + if (state == ctrl->npss && + (ctrl->quirks & NVME_QUIRK_NO_DEEPEST_PS)) + continue; + /* * Is this state a useful non-operational state for * higher-power states to autonomously transition to? @@ -1387,16 +1395,6 @@ struct nvme_core_quirk_entry { }; static const struct nvme_core_quirk_entry core_quirks[] = { - /* - * Seen on a Samsung "SM951 NVMe SAMSUNG 256GB": using APST causes - * the controller to go out to lunch. It dies when the watchdog - * timer reads CSTS and gets 0xffffffff. - */ - { - .vid = 0x144d, - .fr = "BXW75D0Q", - .quirks = NVME_QUIRK_NO_APST, - }, }; /* match is null-terminated but idstr is space-padded. */ diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 2aa20e3e5675..ab2d6ec7eb5c 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -83,6 +83,11 @@ enum nvme_quirks { * APST should not be used. */ NVME_QUIRK_NO_APST = (1 << 4), + + /* + * The deepest sleep state should not be used. + */ + NVME_QUIRK_NO_DEEPEST_PS = (1 << 5), }; /* diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 26a5fd05fe88..5d309535abbd 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -1943,10 +1944,31 @@ static int nvme_dev_map(struct nvme_dev *dev) return -ENODEV; } +static unsigned long check_dell_samsung_bug(struct pci_dev *pdev) +{ + if (pdev->vendor == 0x144d && pdev->device == 0xa802) { + /* + * Several Samsung devices seem to drop off the PCIe bus + * randomly when APST is on and uses the deepest sleep state. + * This has been observed on a Samsung "SM951 NVMe SAMSUNG + * 256GB", a "PM951 NVMe SAMSUNG 512GB", and a "Samsung SSD + * 950 PRO 256GB", but it seems to be restricted to two Dell + * laptops. + */ + if (dmi_match(DMI_SYS_VENDOR, "Dell Inc.") && + (dmi_match(DMI_PRODUCT_NAME, "XPS 15 9550") || + dmi_match(DMI_PRODUCT_NAME, "Precision 5510"))) + return NVME_QUIRK_NO_DEEPEST_PS; + } + + return 0; +} + static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id) { int node, result = -ENOMEM; struct nvme_dev *dev; + unsigned long quirks = id->driver_data; node = dev_to_node(&pdev->dev); if (node == NUMA_NO_NODE) @@ -1978,8 +2000,10 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (result) goto put_pci; + quirks |= check_dell_samsung_bug(pdev); + result = nvme_init_ctrl(&dev->ctrl, &pdev->dev, &nvme_pci_ctrl_ops, - id->driver_data); + quirks); if (result) goto release_pools; -- cgit v1.2.3 From be56945c4edd5a3da15f8254b68d1ddb1588d0c4 Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Thu, 20 Apr 2017 13:37:56 -0700 Subject: nvme: Quirk APST off on "THNSF5256GPUK TOSHIBA" There's a report that it malfunctions with APST on. See https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1678184 Cc: Kai-Heng Feng Signed-off-by: Andy Lutomirski Signed-off-by: Jens Axboe --- drivers/nvme/host/core.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 00b2818dac31..eeb409c287b8 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -1395,6 +1395,15 @@ struct nvme_core_quirk_entry { }; static const struct nvme_core_quirk_entry core_quirks[] = { + { + /* + * This Toshiba device seems to die using any APST states. See: + * https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1678184/comments/11 + */ + .vid = 0x1179, + .mn = "THNSF5256GPUK TOSHIBA", + .quirks = NVME_QUIRK_NO_APST, + } }; /* match is null-terminated but idstr is space-padded. */ -- cgit v1.2.3 From 3a07bb1d7676ea24bc1585ba2a80604852c005e7 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 20 Apr 2017 14:53:28 -0600 Subject: blk-mq: fix potential oops with polling and blk-mq scheduler If we have a scheduler attached, blk_mq_tag_to_rq() on the scheduled tags will return NULL if a request is no longer in flight. This is different than using the normal tags, where it will always return the fixed request. Check for this condition for polling, in case we happen to enter polling for a completed request. The request address remains valid, so this check and return should be perfectly safe. Fixes: bd166ef183c2 ("blk-mq-sched: add framework for MQ capable IO schedulers") Tested-by: Stephen Bates Signed-off-by: Jens Axboe --- block/blk-mq.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 572966f49596..c7836a1ded97 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2928,8 +2928,17 @@ bool blk_mq_poll(struct request_queue *q, blk_qc_t cookie) hctx = q->queue_hw_ctx[blk_qc_t_to_queue_num(cookie)]; if (!blk_qc_t_is_internal(cookie)) rq = blk_mq_tag_to_rq(hctx->tags, blk_qc_t_to_tag(cookie)); - else + else { rq = blk_mq_tag_to_rq(hctx->sched_tags, blk_qc_t_to_tag(cookie)); + /* + * With scheduling, if the request has completed, we'll + * get a NULL return here, as we clear the sched tag when + * that happens. The request still remains valid, like always, + * so we should be safe with just the NULL check. + */ + if (!rq) + return false; + } return __blk_mq_poll(hctx, rq); } -- cgit v1.2.3