From 0f8087ecdeac921fc4920f1328f55c15080bc6aa Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Wed, 21 Oct 2015 13:19:33 -0400 Subject: block: Consolidate static integrity profile properties We previously made a complete copy of a device's data integrity profile even though several of the fields inside the blk_integrity struct are pointers to fixed template entries in t10-pi.c. Split the static and per-device portions so that we can reference the template directly. Signed-off-by: Martin K. Petersen Reported-by: Christoph Hellwig Reviewed-by: Sagi Grimberg Cc: Dan Williams Signed-off-by: Dan Williams Signed-off-by: Jens Axboe --- drivers/nvme/host/pci.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/nvme') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 22d83752ae87..04e3d60a1e45 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -558,7 +558,7 @@ static int nvme_noop_generate(struct blk_integrity_iter *iter) return 0; } -struct blk_integrity nvme_meta_noop = { +struct blk_integrity_profile nvme_meta_noop = { .name = "NVME_META_NOOP", .generate_fn = nvme_noop_generate, .verify_fn = nvme_noop_verify, @@ -570,14 +570,14 @@ static void nvme_init_integrity(struct nvme_ns *ns) switch (ns->pi_type) { case NVME_NS_DPS_PI_TYPE3: - integrity = t10_pi_type3_crc; + integrity.profile = &t10_pi_type3_crc; break; case NVME_NS_DPS_PI_TYPE1: case NVME_NS_DPS_PI_TYPE2: - integrity = t10_pi_type1_crc; + integrity.profile = &t10_pi_type1_crc; break; default: - integrity = nvme_meta_noop; + integrity.profile = &nvme_meta_noop; break; } integrity.tuple_size = ns->ms; -- cgit v1.2.3 From 25520d55cdb6ee289abc68f553d364d22478ff54 Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Wed, 21 Oct 2015 13:19:49 -0400 Subject: block: Inline blk_integrity in struct gendisk Up until now the_integrity profile has been dynamically allocated and attached to struct gendisk after the disk has been made active. This causes problems because NVMe devices need to register the profile prior to the partition table being read due to a mandatory metadata buffer requirement. In addition, DM goes through hoops to deal with preallocating, but not initializing integrity profiles. Since the integrity profile is small (4 bytes + a pointer), Christoph suggested moving it to struct gendisk proper. This requires several changes: - Moving the blk_integrity definition to genhd.h. - Inlining blk_integrity in struct gendisk. - Removing the dynamic allocation code. - Adding helper functions which allow gendisk to set up and tear down the integrity sysfs dir when a disk is added/deleted. - Adding a blk_integrity_revalidate() callback for updating the stable pages bdi setting. - The calls that depend on whether a device has an integrity profile or not now key off of the bi->profile pointer. - Simplifying the integrity support routines in DM (Mike Snitzer). Signed-off-by: Martin K. Petersen Reported-by: Christoph Hellwig Reviewed-by: Sagi Grimberg Signed-off-by: Mike Snitzer Cc: Dan Williams Signed-off-by: Dan Williams Signed-off-by: Jens Axboe --- drivers/nvme/host/pci.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/nvme') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 04e3d60a1e45..8d2aeaaa3895 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -538,7 +538,7 @@ static void nvme_dif_remap(struct request *req, virt = bip_get_seed(bip); phys = nvme_block_nr(ns, blk_rq_pos(req)); nlb = (blk_rq_bytes(req) >> ns->lba_shift); - ts = ns->disk->integrity->tuple_size; + ts = ns->disk->integrity.tuple_size; for (i = 0; i < nlb; i++, virt++, phys++) { pi = (struct t10_pi_tuple *)p; @@ -2044,8 +2044,7 @@ static int nvme_revalidate_disk(struct gendisk *disk) ns->pi_type = pi_type; blk_queue_logical_block_size(ns->queue, bs); - if (ns->ms && !blk_get_integrity(disk) && (disk->flags & GENHD_FL_UP) && - !ns->ext) + if (ns->ms && !ns->ext) nvme_init_integrity(ns); if (ns->ms && !(ns->ms == 8 && ns->pi_type) && !blk_get_integrity(disk)) -- cgit v1.2.3 From 9609b9942b180a50b0162419abd2932a41117fe9 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 21 Oct 2015 13:19:55 -0400 Subject: md, dm, scsi, nvme, libnvdimm: drop blk_integrity_unregister() at shutdown Now that the integrity profile is statically allocated there is no work to do when shutting down an integrity enabled block device. Cc: Matthew Wilcox Cc: Mike Snitzer Cc: James Bottomley Acked-by: NeilBrown Acked-by: Keith Busch Acked-by: Vishal Verma Tested-by: Ross Zwisler Signed-off-by: Dan Williams Signed-off-by: Jens Axboe --- drivers/nvme/host/pci.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/nvme') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 8d2aeaaa3895..ad06f8dcf582 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2407,11 +2407,8 @@ static void nvme_ns_remove(struct nvme_ns *ns) if (kill) blk_set_queue_dying(ns->queue); - if (ns->disk->flags & GENHD_FL_UP) { - if (blk_get_integrity(ns->disk)) - blk_integrity_unregister(ns->disk); + if (ns->disk->flags & GENHD_FL_UP) del_gendisk(ns->disk); - } if (kill || !blk_queue_dying(ns->queue)) { blk_mq_abort_requeue_list(ns->queue); blk_cleanup_queue(ns->queue); -- cgit v1.2.3 From 4cfc766e07a5ed709a9d5289c8644fe78e9f24de Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 21 Oct 2015 13:20:07 -0400 Subject: nvme: suspend i/o during runtime blk_integrity_unregister Synchronize pending i/o against a change in the integrity profile to avoid the possibility of spurious integrity errors. Cc: Matthew Wilcox Acked-by: Keith Busch [keith: also protect dynamic integrity registration] Signed-off-by: Dan Williams Signed-off-by: Jens Axboe --- drivers/nvme/host/pci.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/nvme') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index ad06f8dcf582..4757a2c9366e 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2035,6 +2035,7 @@ static int nvme_revalidate_disk(struct gendisk *disk) pi_type = ns->ms == sizeof(struct t10_pi_tuple) ? id->dps & NVME_NS_DPS_PI_MASK : 0; + blk_mq_freeze_queue(disk->queue); if (blk_get_integrity(disk) && (ns->pi_type != pi_type || ns->ms != old_ms || bs != queue_logical_block_size(disk->queue) || @@ -2054,6 +2055,7 @@ static int nvme_revalidate_disk(struct gendisk *disk) if (dev->oncs & NVME_CTRL_ONCS_DSM) nvme_config_discard(ns); + blk_mq_unfreeze_queue(disk->queue); kfree(id); return 0; -- cgit v1.2.3 From ac6fc48c9fb7d3220ec4e0be0c29bb314ea75f9f Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 21 Oct 2015 13:20:18 -0400 Subject: block: move blk_integrity to request_queue A trace like the following proceeds a crash in bio_integrity_process() when it goes to use an already freed blk_integrity profile. BUG: unable to handle kernel paging request at ffff8800d31b10d8 IP: [] 0xffff8800d31b10d8 PGD 2f65067 PUD 21fffd067 PMD 80000000d30001e3 Oops: 0011 [#1] SMP Dumping ftrace buffer: --------------------------------- ndctl-2222 2.... 44526245us : disk_release: pmem1s systemd--2223 4.... 44573945us : bio_integrity_endio: pmem1s <...>-409 4.... 44574005us : bio_integrity_process: pmem1s --------------------------------- [..] Call Trace: [] ? bio_integrity_process+0x159/0x2d0 [] bio_integrity_verify_fn+0x36/0x60 [] process_one_work+0x1cc/0x4e0 Given that a request_queue is pinned while i/o is in flight and that a gendisk is allowed to have a shorter lifetime, move blk_integrity to request_queue to satisfy requests arriving after the gendisk has been torn down. Cc: Christoph Hellwig Cc: Martin K. Petersen [martin: fix the CONFIG_BLK_DEV_INTEGRITY=n case] Tested-by: Ross Zwisler Signed-off-by: Dan Williams Signed-off-by: Jens Axboe --- drivers/nvme/host/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/nvme') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 4757a2c9366e..2fa28680ad0f 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -538,7 +538,7 @@ static void nvme_dif_remap(struct request *req, virt = bip_get_seed(bip); phys = nvme_block_nr(ns, blk_rq_pos(req)); nlb = (blk_rq_bytes(req) >> ns->lba_shift); - ts = ns->disk->integrity.tuple_size; + ts = ns->disk->queue->integrity.tuple_size; for (i = 0; i < nlb; i++, virt++, phys++) { pi = (struct t10_pi_tuple *)p; -- cgit v1.2.3 From 4125a09b0a0d579ebace17f0e62b03ab9d5ab2f4 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 21 Oct 2015 13:20:29 -0400 Subject: block, libnvdimm, nvme: provide a built-in blk_integrity nop profile The libnvidmm-btt and nvme drivers use blk_integrity to reserve space for per-sector metadata, but sometimes without protection checksums. This property is generically useful, so teach the block core to internally specify a nop profile if one is not provided at registration time. Cc: Keith Busch Cc: Matthew Wilcox Suggested-by: Christoph Hellwig [hch: kill the local nvme nop profile as well] Acked-by: Martin K. Petersen Signed-off-by: Dan Williams Signed-off-by: Jens Axboe --- drivers/nvme/host/pci.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) (limited to 'drivers/nvme') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 2fa28680ad0f..9bea542afc4f 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -548,22 +548,6 @@ static void nvme_dif_remap(struct request *req, kunmap_atomic(pmap); } -static int nvme_noop_verify(struct blk_integrity_iter *iter) -{ - return 0; -} - -static int nvme_noop_generate(struct blk_integrity_iter *iter) -{ - return 0; -} - -struct blk_integrity_profile nvme_meta_noop = { - .name = "NVME_META_NOOP", - .generate_fn = nvme_noop_generate, - .verify_fn = nvme_noop_verify, -}; - static void nvme_init_integrity(struct nvme_ns *ns) { struct blk_integrity integrity; @@ -577,7 +561,7 @@ static void nvme_init_integrity(struct nvme_ns *ns) integrity.profile = &t10_pi_type1_crc; break; default: - integrity.profile = &nvme_meta_noop; + integrity.profile = NULL; break; } integrity.tuple_size = ns->ms; -- cgit v1.2.3