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/md/dm-table.c | 88 +++++++++++++++++++++++++++------------------------ drivers/md/md.c | 9 ++---- 2 files changed, 50 insertions(+), 47 deletions(-) (limited to 'drivers/md') diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index e76ed003769e..061152a43730 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c @@ -1014,15 +1014,16 @@ static int dm_table_build_index(struct dm_table *t) return r; } +static bool integrity_profile_exists(struct gendisk *disk) +{ + return !!blk_get_integrity(disk); +} + /* * Get a disk whose integrity profile reflects the table's profile. - * If %match_all is true, all devices' profiles must match. - * If %match_all is false, all devices must at least have an - * allocated integrity profile; but uninitialized is ok. * Returns NULL if integrity support was inconsistent or unavailable. */ -static struct gendisk * dm_table_get_integrity_disk(struct dm_table *t, - bool match_all) +static struct gendisk * dm_table_get_integrity_disk(struct dm_table *t) { struct list_head *devices = dm_table_get_devices(t); struct dm_dev_internal *dd = NULL; @@ -1030,10 +1031,8 @@ static struct gendisk * dm_table_get_integrity_disk(struct dm_table *t, list_for_each_entry(dd, devices, list) { template_disk = dd->dm_dev->bdev->bd_disk; - if (!blk_get_integrity(template_disk)) + if (!integrity_profile_exists(template_disk)) goto no_integrity; - if (!match_all && !blk_integrity_is_initialized(template_disk)) - continue; /* skip uninitialized profiles */ else if (prev_disk && blk_integrity_compare(prev_disk, template_disk) < 0) goto no_integrity; @@ -1052,34 +1051,40 @@ no_integrity: } /* - * Register the mapped device for blk_integrity support if - * the underlying devices have an integrity profile. But all devices - * may not have matching profiles (checking all devices isn't reliable + * Register the mapped device for blk_integrity support if the + * underlying devices have an integrity profile. But all devices may + * not have matching profiles (checking all devices isn't reliable * during table load because this table may use other DM device(s) which - * must be resumed before they will have an initialized integity profile). - * Stacked DM devices force a 2 stage integrity profile validation: - * 1 - during load, validate all initialized integrity profiles match - * 2 - during resume, validate all integrity profiles match + * must be resumed before they will have an initialized integity + * profile). Consequently, stacked DM devices force a 2 stage integrity + * profile validation: First pass during table load, final pass during + * resume. */ -static int dm_table_prealloc_integrity(struct dm_table *t, struct mapped_device *md) +static int dm_table_register_integrity(struct dm_table *t) { + struct mapped_device *md = t->md; struct gendisk *template_disk = NULL; - template_disk = dm_table_get_integrity_disk(t, false); + template_disk = dm_table_get_integrity_disk(t); if (!template_disk) return 0; - if (!blk_integrity_is_initialized(dm_disk(md))) { + if (!integrity_profile_exists(dm_disk(md))) { t->integrity_supported = 1; - return blk_integrity_register(dm_disk(md), NULL); + /* + * Register integrity profile during table load; we can do + * this because the final profile must match during resume. + */ + blk_integrity_register(dm_disk(md), + blk_get_integrity(template_disk)); + return 0; } /* - * If DM device already has an initalized integrity + * If DM device already has an initialized integrity * profile the new profile should not conflict. */ - if (blk_integrity_is_initialized(template_disk) && - blk_integrity_compare(dm_disk(md), template_disk) < 0) { + if (blk_integrity_compare(dm_disk(md), template_disk) < 0) { DMWARN("%s: conflict with existing integrity profile: " "%s profile mismatch", dm_device_name(t->md), @@ -1087,7 +1092,7 @@ static int dm_table_prealloc_integrity(struct dm_table *t, struct mapped_device return 1; } - /* Preserve existing initialized integrity profile */ + /* Preserve existing integrity profile */ t->integrity_supported = 1; return 0; } @@ -1112,7 +1117,7 @@ int dm_table_complete(struct dm_table *t) return r; } - r = dm_table_prealloc_integrity(t, t->md); + r = dm_table_register_integrity(t); if (r) { DMERR("could not register integrity profile."); return r; @@ -1278,29 +1283,30 @@ combine_limits: } /* - * Set the integrity profile for this device if all devices used have - * matching profiles. We're quite deep in the resume path but still - * don't know if all devices (particularly DM devices this device - * may be stacked on) have matching profiles. Even if the profiles - * don't match we have no way to fail (to resume) at this point. + * Verify that all devices have an integrity profile that matches the + * DM device's registered integrity profile. If the profiles don't + * match then unregister the DM device's integrity profile. */ -static void dm_table_set_integrity(struct dm_table *t) +static void dm_table_verify_integrity(struct dm_table *t) { struct gendisk *template_disk = NULL; - if (!blk_get_integrity(dm_disk(t->md))) - return; + if (t->integrity_supported) { + /* + * Verify that the original integrity profile + * matches all the devices in this table. + */ + template_disk = dm_table_get_integrity_disk(t); + if (template_disk && + blk_integrity_compare(dm_disk(t->md), template_disk) >= 0) + return; + } - template_disk = dm_table_get_integrity_disk(t, true); - if (template_disk) - blk_integrity_register(dm_disk(t->md), - blk_get_integrity(template_disk)); - else if (blk_integrity_is_initialized(dm_disk(t->md))) - DMWARN("%s: device no longer has a valid integrity profile", - dm_device_name(t->md)); - else + if (integrity_profile_exists(dm_disk(t->md))) { DMWARN("%s: unable to establish an integrity profile", dm_device_name(t->md)); + blk_integrity_unregister(dm_disk(t->md)); + } } static int device_flush_capable(struct dm_target *ti, struct dm_dev *dev, @@ -1500,7 +1506,7 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q, else queue_flag_set_unlocked(QUEUE_FLAG_NO_SG_MERGE, q); - dm_table_set_integrity(t); + dm_table_verify_integrity(t); /* * Determine whether or not this queue's I/O timings contribute diff --git a/drivers/md/md.c b/drivers/md/md.c index c702de18207a..2af9d590e1a0 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -1962,12 +1962,9 @@ int md_integrity_register(struct mddev *mddev) * All component devices are integrity capable and have matching * profiles, register the common profile for the md device. */ - if (blk_integrity_register(mddev->gendisk, - bdev_get_integrity(reference->bdev)) != 0) { - printk(KERN_ERR "md: failed to register integrity for %s\n", - mdname(mddev)); - return -EINVAL; - } + blk_integrity_register(mddev->gendisk, + bdev_get_integrity(reference->bdev)); + printk(KERN_NOTICE "md: data integrity enabled on %s\n", mdname(mddev)); if (bioset_integrity_create(mddev->bio_set, BIO_POOL_SIZE)) { printk(KERN_ERR "md: failed to create integrity pool for %s\n", -- cgit v1.2.3-59-g8ed1b 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/md/dm.c | 2 -- drivers/md/md.c | 1 - drivers/nvdimm/btt.c | 1 - drivers/nvme/host/pci.c | 5 +---- drivers/scsi/sd.c | 1 - 5 files changed, 1 insertion(+), 9 deletions(-) (limited to 'drivers/md') diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 6264781dc69a..f4d953e10e2f 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -2233,8 +2233,6 @@ static void cleanup_mapped_device(struct mapped_device *md) spin_lock(&_minor_lock); md->disk->private_data = NULL; spin_unlock(&_minor_lock); - if (blk_get_integrity(md->disk)) - blk_integrity_unregister(md->disk); del_gendisk(md->disk); put_disk(md->disk); } diff --git a/drivers/md/md.c b/drivers/md/md.c index 2af9d590e1a0..7d07caceb349 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -5539,7 +5539,6 @@ static int do_md_stop(struct mddev *mddev, int mode, if (mddev->hold_active == UNTIL_STOP) mddev->hold_active = 0; } - blk_integrity_unregister(disk); md_new_event(mddev); sysfs_notify_dirent_safe(mddev->sysfs_state); return 0; diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c index 254239746020..eae93ab8ffcd 100644 --- a/drivers/nvdimm/btt.c +++ b/drivers/nvdimm/btt.c @@ -1279,7 +1279,6 @@ static int btt_blk_init(struct btt *btt) static void btt_blk_cleanup(struct btt *btt) { - blk_integrity_unregister(btt->btt_disk); del_gendisk(btt->btt_disk); put_disk(btt->btt_disk); blk_cleanup_queue(btt->btt_queue); 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); diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 3f370228bf31..9e85211ea1d1 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -3068,7 +3068,6 @@ static void scsi_disk_release(struct device *dev) ida_remove(&sd_index_ida, sdkp->index); spin_unlock(&sd_index_lock); - blk_integrity_unregister(disk); disk->private_data = NULL; put_disk(disk); put_device(&sdkp->device->sdev_gendev); -- cgit v1.2.3-59-g8ed1b From c7bfced9a6716ff66c9d61f934bb60af08d4688c Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 21 Oct 2015 13:20:02 -0400 Subject: md: 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. Given linear_add() is suspending the mddev before manipulating the mddev, do the same for the other personalities. Acked-by: NeilBrown Signed-off-by: Dan Williams Signed-off-by: Jens Axboe --- drivers/md/md.c | 1 + drivers/md/multipath.c | 2 ++ drivers/md/raid1.c | 2 ++ drivers/md/raid10.c | 2 ++ 4 files changed, 7 insertions(+) (limited to 'drivers/md') diff --git a/drivers/md/md.c b/drivers/md/md.c index 7d07caceb349..714aa92db174 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -1994,6 +1994,7 @@ void md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev) if (bi_rdev && blk_integrity_compare(mddev->gendisk, rdev->bdev->bd_disk) >= 0) return; + WARN_ON_ONCE(!mddev->suspended); printk(KERN_NOTICE "disabling data integrity on %s\n", mdname(mddev)); blk_integrity_unregister(mddev->gendisk); } diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c index d132f06afdd1..7331a80d89f1 100644 --- a/drivers/md/multipath.c +++ b/drivers/md/multipath.c @@ -264,7 +264,9 @@ static int multipath_add_disk(struct mddev *mddev, struct md_rdev *rdev) spin_unlock_irq(&conf->device_lock); rcu_assign_pointer(p->rdev, rdev); err = 0; + mddev_suspend(mddev); md_integrity_add_rdev(rdev, mddev); + mddev_resume(mddev); break; } diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c index 049df6c4a8cc..a881b111fa35 100644 --- a/drivers/md/raid1.c +++ b/drivers/md/raid1.c @@ -1621,7 +1621,9 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev) break; } } + mddev_suspend(mddev); md_integrity_add_rdev(rdev, mddev); + mddev_resume(mddev); if (mddev->queue && blk_queue_discard(bdev_get_queue(rdev->bdev))) queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue); print_conf(conf); diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 7c99a4037715..6f0ec107996a 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -1736,7 +1736,9 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev) rcu_assign_pointer(p->rdev, rdev); break; } + mddev_suspend(mddev); md_integrity_add_rdev(rdev, mddev); + mddev_resume(mddev); if (mddev->queue && blk_queue_discard(bdev_get_queue(rdev->bdev))) queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, mddev->queue); -- cgit v1.2.3-59-g8ed1b