* [PATCH v2 01/12] block: Move integrity kobject to struct gendisk
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
@ 2015-10-15 19:59 ` Dan Williams
2015-10-23 20:21 ` Jeff Moyer
2015-10-15 19:59 ` [PATCH v2 02/12] block: Consolidate static integrity profile properties Dan Williams
` (11 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: Dan Williams @ 2015-10-15 19:59 UTC (permalink / raw)
From: Martin K. Petersen <martin.petersen@oracle.com>
The integrity kobject purely exists to support the integrity
subdirectory in sysfs and doesn't really have anything to do with the
blk_integrity data structure. Move the kobject to struct gendisk where
it belongs.
Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
Reported-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
block/blk-integrity.c | 22 +++++++++++-----------
include/linux/blkdev.h | 2 --
include/linux/genhd.h | 1 +
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 75f29cf70188..182bfd2383ea 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -249,8 +249,8 @@ struct integrity_sysfs_entry {
static ssize_t integrity_attr_show(struct kobject *kobj, struct attribute *attr,
char *page)
{
- struct blk_integrity *bi =
- container_of(kobj, struct blk_integrity, kobj);
+ struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
+ struct blk_integrity *bi = blk_get_integrity(disk);
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
@@ -261,8 +261,8 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
struct attribute *attr, const char *page,
size_t count)
{
- struct blk_integrity *bi =
- container_of(kobj, struct blk_integrity, kobj);
+ struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
+ struct blk_integrity *bi = blk_get_integrity(disk);
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
ssize_t ret = 0;
@@ -385,8 +385,8 @@ subsys_initcall(blk_dev_integrity_init);
static void blk_integrity_release(struct kobject *kobj)
{
- struct blk_integrity *bi =
- container_of(kobj, struct blk_integrity, kobj);
+ struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
+ struct blk_integrity *bi = blk_get_integrity(disk);
kmem_cache_free(integrity_cachep, bi);
}
@@ -429,14 +429,14 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
if (!bi)
return -1;
- if (kobject_init_and_add(&bi->kobj, &integrity_ktype,
+ if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
&disk_to_dev(disk)->kobj,
"%s", "integrity")) {
kmem_cache_free(integrity_cachep, bi);
return -1;
}
- kobject_uevent(&bi->kobj, KOBJ_ADD);
+ kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
bi->flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
bi->interval = queue_logical_block_size(disk->queue);
@@ -479,9 +479,9 @@ void blk_integrity_unregister(struct gendisk *disk)
bi = disk->integrity;
- kobject_uevent(&bi->kobj, KOBJ_REMOVE);
- kobject_del(&bi->kobj);
- kobject_put(&bi->kobj);
+ kobject_uevent(&disk->integrity_kobj, KOBJ_REMOVE);
+ kobject_del(&disk->integrity_kobj);
+ kobject_put(&disk->integrity_kobj);
disk->integrity = NULL;
}
EXPORT_SYMBOL(blk_integrity_unregister);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 19c2e947d4d1..830f9c07d4bb 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1472,8 +1472,6 @@ struct blk_integrity {
unsigned short tag_size;
const char *name;
-
- struct kobject kobj;
};
extern bool blk_integrity_is_initialized(struct gendisk *);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 2adbfa6d02bc..9e6e0dfa97ad 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -199,6 +199,7 @@ struct gendisk {
struct disk_events *ev;
#ifdef CONFIG_BLK_DEV_INTEGRITY
struct blk_integrity *integrity;
+ struct kobject integrity_kobj;
#endif
int node_id;
};
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 01/12] block: Move integrity kobject to struct gendisk
2015-10-15 19:59 ` [PATCH v2 01/12] block: Move integrity kobject to struct gendisk Dan Williams
@ 2015-10-23 20:21 ` Jeff Moyer
0 siblings, 0 replies; 33+ messages in thread
From: Jeff Moyer @ 2015-10-23 20:21 UTC (permalink / raw)
Dan Williams <dan.j.williams at intel.com> writes:
> From: Martin K. Petersen <martin.petersen at oracle.com>
>
> The integrity kobject purely exists to support the integrity
> subdirectory in sysfs and doesn't really have anything to do with the
> blk_integrity data structure. Move the kobject to struct gendisk where
> it belongs.
>
> Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
> Reported-by: Christoph Hellwig <hch at lst.de>
> Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
> Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams at intel.com>
Reviewed-by: Jeff Moyer <jmoyer at redhat.com>
> ---
> block/blk-integrity.c | 22 +++++++++++-----------
> include/linux/blkdev.h | 2 --
> include/linux/genhd.h | 1 +
> 3 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/block/blk-integrity.c b/block/blk-integrity.c
> index 75f29cf70188..182bfd2383ea 100644
> --- a/block/blk-integrity.c
> +++ b/block/blk-integrity.c
> @@ -249,8 +249,8 @@ struct integrity_sysfs_entry {
> static ssize_t integrity_attr_show(struct kobject *kobj, struct attribute *attr,
> char *page)
> {
> - struct blk_integrity *bi =
> - container_of(kobj, struct blk_integrity, kobj);
> + struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
> + struct blk_integrity *bi = blk_get_integrity(disk);
> struct integrity_sysfs_entry *entry =
> container_of(attr, struct integrity_sysfs_entry, attr);
>
> @@ -261,8 +261,8 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
> struct attribute *attr, const char *page,
> size_t count)
> {
> - struct blk_integrity *bi =
> - container_of(kobj, struct blk_integrity, kobj);
> + struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
> + struct blk_integrity *bi = blk_get_integrity(disk);
> struct integrity_sysfs_entry *entry =
> container_of(attr, struct integrity_sysfs_entry, attr);
> ssize_t ret = 0;
> @@ -385,8 +385,8 @@ subsys_initcall(blk_dev_integrity_init);
>
> static void blk_integrity_release(struct kobject *kobj)
> {
> - struct blk_integrity *bi =
> - container_of(kobj, struct blk_integrity, kobj);
> + struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
> + struct blk_integrity *bi = blk_get_integrity(disk);
>
> kmem_cache_free(integrity_cachep, bi);
> }
> @@ -429,14 +429,14 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
> if (!bi)
> return -1;
>
> - if (kobject_init_and_add(&bi->kobj, &integrity_ktype,
> + if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
> &disk_to_dev(disk)->kobj,
> "%s", "integrity")) {
> kmem_cache_free(integrity_cachep, bi);
> return -1;
> }
>
> - kobject_uevent(&bi->kobj, KOBJ_ADD);
> + kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
>
> bi->flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
> bi->interval = queue_logical_block_size(disk->queue);
> @@ -479,9 +479,9 @@ void blk_integrity_unregister(struct gendisk *disk)
>
> bi = disk->integrity;
>
> - kobject_uevent(&bi->kobj, KOBJ_REMOVE);
> - kobject_del(&bi->kobj);
> - kobject_put(&bi->kobj);
> + kobject_uevent(&disk->integrity_kobj, KOBJ_REMOVE);
> + kobject_del(&disk->integrity_kobj);
> + kobject_put(&disk->integrity_kobj);
> disk->integrity = NULL;
> }
> EXPORT_SYMBOL(blk_integrity_unregister);
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 19c2e947d4d1..830f9c07d4bb 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1472,8 +1472,6 @@ struct blk_integrity {
> unsigned short tag_size;
>
> const char *name;
> -
> - struct kobject kobj;
> };
>
> extern bool blk_integrity_is_initialized(struct gendisk *);
> diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> index 2adbfa6d02bc..9e6e0dfa97ad 100644
> --- a/include/linux/genhd.h
> +++ b/include/linux/genhd.h
> @@ -199,6 +199,7 @@ struct gendisk {
> struct disk_events *ev;
> #ifdef CONFIG_BLK_DEV_INTEGRITY
> struct blk_integrity *integrity;
> + struct kobject integrity_kobj;
> #endif
> int node_id;
> };
>
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm at lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v2 02/12] block: Consolidate static integrity profile properties
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
2015-10-15 19:59 ` [PATCH v2 01/12] block: Move integrity kobject to struct gendisk Dan Williams
@ 2015-10-15 19:59 ` Dan Williams
2015-10-15 19:59 ` [PATCH v2 03/12] block: Reduce the size of struct blk_integrity Dan Williams
` (10 subsequent siblings)
12 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-15 19:59 UTC (permalink / raw)
From: Martin K. Petersen <martin.petersen@oracle.com>
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 <martin.petersen at oracle.com>
Reported-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
[djbw: make nvdimm profile static]
Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
block/bio-integrity.c | 8 ++++----
block/blk-integrity.c | 17 ++++++++---------
block/t10-pi.c | 16 ++++------------
drivers/nvdimm/core.c | 11 +++++++----
drivers/nvme/host/pci.c | 8 ++++----
drivers/scsi/sd_dif.c | 29 ++++++++++++++++-------------
drivers/target/target_core_iblock.c | 10 +++++-----
include/linux/blkdev.h | 20 +++++++++++---------
include/linux/t10-pi.h | 8 ++++----
9 files changed, 63 insertions(+), 64 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 14b8faf8b09d..a10ffe19a8dd 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -177,11 +177,11 @@ bool bio_integrity_enabled(struct bio *bio)
if (bi == NULL)
return false;
- if (bio_data_dir(bio) == READ && bi->verify_fn != NULL &&
+ if (bio_data_dir(bio) == READ && bi->profile->verify_fn != NULL &&
(bi->flags & BLK_INTEGRITY_VERIFY))
return true;
- if (bio_data_dir(bio) == WRITE && bi->generate_fn != NULL &&
+ if (bio_data_dir(bio) == WRITE && bi->profile->generate_fn != NULL &&
(bi->flags & BLK_INTEGRITY_GENERATE))
return true;
@@ -340,7 +340,7 @@ int bio_integrity_prep(struct bio *bio)
/* Auto-generate integrity metadata if this is a write */
if (bio_data_dir(bio) == WRITE)
- bio_integrity_process(bio, bi->generate_fn);
+ bio_integrity_process(bio, bi->profile->generate_fn);
return 0;
}
@@ -361,7 +361,7 @@ static void bio_integrity_verify_fn(struct work_struct *work)
struct bio *bio = bip->bip_bio;
struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
- bio->bi_error = bio_integrity_process(bio, bi->verify_fn);
+ bio->bi_error = bio_integrity_process(bio, bi->profile->verify_fn);
/* Restore original bio completion handler */
bio->bi_end_io = bip->bip_end_io;
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 182bfd2383ea..daf590ab3b46 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -176,10 +176,10 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
return -1;
}
- if (strcmp(b1->name, b2->name)) {
+ if (b1->profile != b2->profile) {
printk(KERN_ERR "%s: %s/%s type %s != %s\n", __func__,
gd1->disk_name, gd2->disk_name,
- b1->name, b2->name);
+ b1->profile->name, b2->profile->name);
return -1;
}
@@ -275,8 +275,8 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
static ssize_t integrity_format_show(struct blk_integrity *bi, char *page)
{
- if (bi != NULL && bi->name != NULL)
- return sprintf(page, "%s\n", bi->name);
+ if (bi != NULL && bi->profile->name != NULL)
+ return sprintf(page, "%s\n", bi->profile->name);
else
return sprintf(page, "none\n");
}
@@ -401,7 +401,8 @@ bool blk_integrity_is_initialized(struct gendisk *disk)
{
struct blk_integrity *bi = blk_get_integrity(disk);
- return (bi && bi->name && strcmp(bi->name, bi_unsupported_name) != 0);
+ return (bi && bi->profile->name && strcmp(bi->profile->name,
+ bi_unsupported_name) != 0);
}
EXPORT_SYMBOL(blk_integrity_is_initialized);
@@ -446,14 +447,12 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
/* Use the provided profile as template */
if (template != NULL) {
- bi->name = template->name;
- bi->generate_fn = template->generate_fn;
- bi->verify_fn = template->verify_fn;
+ bi->profile = template->profile;
bi->tuple_size = template->tuple_size;
bi->tag_size = template->tag_size;
bi->flags |= template->flags;
} else
- bi->name = bi_unsupported_name;
+ bi->profile->name = bi_unsupported_name;
disk->queue->backing_dev_info.capabilities |= BDI_CAP_STABLE_WRITES;
diff --git a/block/t10-pi.c b/block/t10-pi.c
index 24d6e9715318..2c97912335a9 100644
--- a/block/t10-pi.c
+++ b/block/t10-pi.c
@@ -160,38 +160,30 @@ static int t10_pi_type3_verify_ip(struct blk_integrity_iter *iter)
return t10_pi_verify(iter, t10_pi_ip_fn, 3);
}
-struct blk_integrity t10_pi_type1_crc = {
+struct blk_integrity_profile t10_pi_type1_crc = {
.name = "T10-DIF-TYPE1-CRC",
.generate_fn = t10_pi_type1_generate_crc,
.verify_fn = t10_pi_type1_verify_crc,
- .tuple_size = sizeof(struct t10_pi_tuple),
- .tag_size = 0,
};
EXPORT_SYMBOL(t10_pi_type1_crc);
-struct blk_integrity t10_pi_type1_ip = {
+struct blk_integrity_profile t10_pi_type1_ip = {
.name = "T10-DIF-TYPE1-IP",
.generate_fn = t10_pi_type1_generate_ip,
.verify_fn = t10_pi_type1_verify_ip,
- .tuple_size = sizeof(struct t10_pi_tuple),
- .tag_size = 0,
};
EXPORT_SYMBOL(t10_pi_type1_ip);
-struct blk_integrity t10_pi_type3_crc = {
+struct blk_integrity_profile t10_pi_type3_crc = {
.name = "T10-DIF-TYPE3-CRC",
.generate_fn = t10_pi_type3_generate_crc,
.verify_fn = t10_pi_type3_verify_crc,
- .tuple_size = sizeof(struct t10_pi_tuple),
- .tag_size = 0,
};
EXPORT_SYMBOL(t10_pi_type3_crc);
-struct blk_integrity t10_pi_type3_ip = {
+struct blk_integrity_profile t10_pi_type3_ip = {
.name = "T10-DIF-TYPE3-IP",
.generate_fn = t10_pi_type3_generate_ip,
.verify_fn = t10_pi_type3_verify_ip,
- .tuple_size = sizeof(struct t10_pi_tuple),
- .tag_size = 0,
};
EXPORT_SYMBOL(t10_pi_type3_ip);
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index cb62ec6a12d0..7df89b547ae1 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -399,19 +399,22 @@ static int nd_pi_nop_generate_verify(struct blk_integrity_iter *iter)
int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
{
- struct blk_integrity integrity = {
+ struct blk_integrity bi;
+ static struct blk_integrity_profile profile = {
.name = "ND-PI-NOP",
.generate_fn = nd_pi_nop_generate_verify,
.verify_fn = nd_pi_nop_generate_verify,
- .tuple_size = meta_size,
- .tag_size = meta_size,
};
int ret;
if (meta_size == 0)
return 0;
- ret = blk_integrity_register(disk, &integrity);
+ bi.profile = &profile;
+ bi.tuple_size = meta_size;
+ bi.tag_size = meta_size;
+
+ ret = blk_integrity_register(disk, &bi);
if (ret)
return ret;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index ad58ee3c3b57..5dba51d4bae6 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;
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 5c06d292b94c..5a5ec9aa26b3 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -43,6 +43,7 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
struct scsi_device *sdp = sdkp->device;
struct gendisk *disk = sdkp->disk;
u8 type = sdkp->protection_type;
+ struct blk_integrity bi;
int dif, dix;
dif = scsi_host_dif_capable(sdp->host, type);
@@ -58,36 +59,38 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
/* Enable DMA of protection information */
if (scsi_host_get_guard(sdkp->device->host) & SHOST_DIX_GUARD_IP) {
if (type == SD_DIF_TYPE3_PROTECTION)
- blk_integrity_register(disk, &t10_pi_type3_ip);
+ bi.profile = &t10_pi_type3_ip;
else
- blk_integrity_register(disk, &t10_pi_type1_ip);
+ bi.profile = &t10_pi_type1_ip;
- disk->integrity->flags |= BLK_INTEGRITY_IP_CHECKSUM;
+ bi.flags |= BLK_INTEGRITY_IP_CHECKSUM;
} else
if (type == SD_DIF_TYPE3_PROTECTION)
- blk_integrity_register(disk, &t10_pi_type3_crc);
+ bi.profile = &t10_pi_type3_crc;
else
- blk_integrity_register(disk, &t10_pi_type1_crc);
+ bi.profile = &t10_pi_type1_crc;
+ bi.tuple_size = sizeof(struct t10_pi_tuple);
sd_printk(KERN_NOTICE, sdkp,
- "Enabling DIX %s protection\n", disk->integrity->name);
+ "Enabling DIX %s protection\n", bi.profile->name);
- /* Signal to block layer that we support sector tagging */
if (dif && type) {
-
- disk->integrity->flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+ bi.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
if (!sdkp->ATO)
- return;
+ goto out;
if (type == SD_DIF_TYPE3_PROTECTION)
- disk->integrity->tag_size = sizeof(u16) + sizeof(u32);
+ bi.tag_size = sizeof(u16) + sizeof(u32);
else
- disk->integrity->tag_size = sizeof(u16);
+ bi.tag_size = sizeof(u16);
sd_printk(KERN_NOTICE, sdkp, "DIF application tag size %u\n",
- disk->integrity->tag_size);
+ bi.tag_size);
}
+
+out:
+ blk_integrity_register(disk, &bi);
}
/*
diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index 0f19e11acac2..f29c69120054 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -155,17 +155,17 @@ static int iblock_configure_device(struct se_device *dev)
if (bi) {
struct bio_set *bs = ib_dev->ibd_bio_set;
- if (!strcmp(bi->name, "T10-DIF-TYPE3-IP") ||
- !strcmp(bi->name, "T10-DIF-TYPE1-IP")) {
+ if (!strcmp(bi->profile->name, "T10-DIF-TYPE3-IP") ||
+ !strcmp(bi->profile->name, "T10-DIF-TYPE1-IP")) {
pr_err("IBLOCK export of blk_integrity: %s not"
- " supported\n", bi->name);
+ " supported\n", bi->profile->name);
ret = -ENOSYS;
goto out_blkdev_put;
}
- if (!strcmp(bi->name, "T10-DIF-TYPE3-CRC")) {
+ if (!strcmp(bi->profile->name, "T10-DIF-TYPE3-CRC")) {
dev->dev_attrib.pi_prot_type = TARGET_DIF_TYPE3_PROT;
- } else if (!strcmp(bi->name, "T10-DIF-TYPE1-CRC")) {
+ } else if (!strcmp(bi->profile->name, "T10-DIF-TYPE1-CRC")) {
dev->dev_attrib.pi_prot_type = TARGET_DIF_TYPE1_PROT;
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 830f9c07d4bb..f36c6476f1c7 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1462,16 +1462,18 @@ struct blk_integrity_iter {
typedef int (integrity_processing_fn) (struct blk_integrity_iter *);
-struct blk_integrity {
- integrity_processing_fn *generate_fn;
- integrity_processing_fn *verify_fn;
-
- unsigned short flags;
- unsigned short tuple_size;
- unsigned short interval;
- unsigned short tag_size;
+struct blk_integrity_profile {
+ integrity_processing_fn *generate_fn;
+ integrity_processing_fn *verify_fn;
+ const char *name;
+};
- const char *name;
+struct blk_integrity {
+ struct blk_integrity_profile *profile;
+ unsigned short flags;
+ unsigned short tuple_size;
+ unsigned short interval;
+ unsigned short tag_size;
};
extern bool blk_integrity_is_initialized(struct gendisk *);
diff --git a/include/linux/t10-pi.h b/include/linux/t10-pi.h
index 6a8b9942632d..dd8de82cf5b5 100644
--- a/include/linux/t10-pi.h
+++ b/include/linux/t10-pi.h
@@ -14,9 +14,9 @@ struct t10_pi_tuple {
};
-extern struct blk_integrity t10_pi_type1_crc;
-extern struct blk_integrity t10_pi_type1_ip;
-extern struct blk_integrity t10_pi_type3_crc;
-extern struct blk_integrity t10_pi_type3_ip;
+extern struct blk_integrity_profile t10_pi_type1_crc;
+extern struct blk_integrity_profile t10_pi_type1_ip;
+extern struct blk_integrity_profile t10_pi_type3_crc;
+extern struct blk_integrity_profile t10_pi_type3_ip;
#endif
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 03/12] block: Reduce the size of struct blk_integrity
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
2015-10-15 19:59 ` [PATCH v2 01/12] block: Move integrity kobject to struct gendisk Dan Williams
2015-10-15 19:59 ` [PATCH v2 02/12] block: Consolidate static integrity profile properties Dan Williams
@ 2015-10-15 19:59 ` Dan Williams
2015-10-15 20:00 ` [PATCH v2 04/12] block: Export integrity data interval size in sysfs Dan Williams
` (9 subsequent siblings)
12 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-15 19:59 UTC (permalink / raw)
From: Martin K. Petersen <martin.petersen@oracle.com>
The per-device properties in the blk_integrity structure were previously
unsigned short. However, most of the values fit inside a char. The only
exception is the data interval size and we can work around that by
storing it as a power of two.
This cuts the size of the dynamic portion of blk_integrity in half.
Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
Reported-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
block/bio-integrity.c | 4 ++--
block/blk-integrity.c | 6 +++---
include/linux/blkdev.h | 8 ++++----
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index a10ffe19a8dd..6a90eca9cea1 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -202,7 +202,7 @@ EXPORT_SYMBOL(bio_integrity_enabled);
static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
unsigned int sectors)
{
- return sectors >> (ilog2(bi->interval) - 9);
+ return sectors >> (bi->interval_exp - 9);
}
static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
@@ -229,7 +229,7 @@ static int bio_integrity_process(struct bio *bio,
bip->bip_vec->bv_offset;
iter.disk_name = bio->bi_bdev->bd_disk->disk_name;
- iter.interval = bi->interval;
+ iter.interval = 1 << bi->interval_exp;
iter.seed = bip_get_seed(bip);
iter.prot_buf = prot_buf;
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index daf590ab3b46..c7508654faff 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -155,10 +155,10 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
if (!b1 || !b2)
return -1;
- if (b1->interval != b2->interval) {
+ if (b1->interval_exp != b2->interval_exp) {
pr_err("%s: %s/%s protection interval %u != %u\n",
__func__, gd1->disk_name, gd2->disk_name,
- b1->interval, b2->interval);
+ 1 << b1->interval_exp, 1 << b2->interval_exp);
return -1;
}
@@ -440,7 +440,7 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
bi->flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
- bi->interval = queue_logical_block_size(disk->queue);
+ bi->interval_exp = ilog2(queue_logical_block_size(disk->queue));
disk->integrity = bi;
} else
bi = disk->integrity;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index f36c6476f1c7..4f1968f15e30 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1470,10 +1470,10 @@ struct blk_integrity_profile {
struct blk_integrity {
struct blk_integrity_profile *profile;
- unsigned short flags;
- unsigned short tuple_size;
- unsigned short interval;
- unsigned short tag_size;
+ unsigned char flags;
+ unsigned char tuple_size;
+ unsigned char interval_exp;
+ unsigned char tag_size;
};
extern bool blk_integrity_is_initialized(struct gendisk *);
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 04/12] block: Export integrity data interval size in sysfs
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
` (2 preceding siblings ...)
2015-10-15 19:59 ` [PATCH v2 03/12] block: Reduce the size of struct blk_integrity Dan Williams
@ 2015-10-15 20:00 ` Dan Williams
2015-10-15 20:00 ` [PATCH v2 05/12] block: Inline blk_integrity in struct gendisk Dan Williams
` (8 subsequent siblings)
12 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-15 20:00 UTC (permalink / raw)
From: Martin K. Petersen <martin.petersen@oracle.com>
The size of the data interval was not exported in the sysfs integrity
directory. Export it so that userland apps can tell whether the interval
is different from the device's logical block size.
Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
Documentation/ABI/testing/sysfs-block | 7 +++++++
block/blk-integrity.c | 14 ++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index 8df003963d99..71d184dbb70d 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -60,6 +60,13 @@ Description:
Indicates whether a storage device is capable of storing
integrity metadata. Set if the device is T10 PI-capable.
+What: /sys/block/<disk>/integrity/protection_interval_bytes
+Date: July 2015
+Contact: Martin K. Petersen <martin.petersen at oracle.com>
+Description:
+ Describes the number of data bytes which are protected
+ by one integrity tuple. Typically the device's logical
+ block size.
What: /sys/block/<disk>/integrity/write_generate
Date: June 2008
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index c7508654faff..7a96f57ed195 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -289,6 +289,14 @@ static ssize_t integrity_tag_size_show(struct blk_integrity *bi, char *page)
return sprintf(page, "0\n");
}
+static ssize_t integrity_interval_show(struct blk_integrity *bi, char *page)
+{
+ if (bi != NULL)
+ return sprintf(page, "%u\n", 1 << bi->interval_exp);
+ else
+ return sprintf(page, "0\n");
+}
+
static ssize_t integrity_verify_store(struct blk_integrity *bi,
const char *page, size_t count)
{
@@ -343,6 +351,11 @@ static struct integrity_sysfs_entry integrity_tag_size_entry = {
.show = integrity_tag_size_show,
};
+static struct integrity_sysfs_entry integrity_interval_entry = {
+ .attr = { .name = "protection_interval_bytes", .mode = S_IRUGO },
+ .show = integrity_interval_show,
+};
+
static struct integrity_sysfs_entry integrity_verify_entry = {
.attr = { .name = "read_verify", .mode = S_IRUGO | S_IWUSR },
.show = integrity_verify_show,
@@ -363,6 +376,7 @@ static struct integrity_sysfs_entry integrity_device_entry = {
static struct attribute *integrity_attrs[] = {
&integrity_format_entry.attr,
&integrity_tag_size_entry.attr,
+ &integrity_interval_entry.attr,
&integrity_verify_entry.attr,
&integrity_generate_entry.attr,
&integrity_device_entry.attr,
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 05/12] block: Inline blk_integrity in struct gendisk
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
` (3 preceding siblings ...)
2015-10-15 20:00 ` [PATCH v2 04/12] block: Export integrity data interval size in sysfs Dan Williams
@ 2015-10-15 20:00 ` Dan Williams
2015-10-15 20:00 ` [PATCH v2 06/12] md, dm, scsi, nvme, libnvdimm: drop blk_integrity_unregister() at shutdown Dan Williams
` (7 subsequent siblings)
12 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-15 20:00 UTC (permalink / raw)
From: Martin K. Petersen <martin.petersen@oracle.com>
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 <martin.petersen at oracle.com>
Reported-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
Cc: Mike Snitzer <snitzer at redhat.com>
Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
block/blk-integrity.c | 160 ++++++++++++++++-----------------------------
block/genhd.c | 2 +
block/partition-generic.c | 1
drivers/md/dm-table.c | 88 +++++++++++++------------
drivers/md/md.c | 9 +--
drivers/nvdimm/core.c | 6 --
drivers/nvme/host/pci.c | 5 +
fs/block_dev.c | 2 -
include/linux/blkdev.h | 34 ++++------
include/linux/genhd.h | 26 +++++++
10 files changed, 152 insertions(+), 181 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 7a96f57ed195..4615a3386798 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -30,10 +30,6 @@
#include "blk.h"
-static struct kmem_cache *integrity_cachep;
-
-static const char *bi_unsupported_name = "unsupported";
-
/**
* blk_rq_count_integrity_sg - Count number of integrity scatterlist elements
* @q: request queue
@@ -146,13 +142,13 @@ EXPORT_SYMBOL(blk_rq_map_integrity_sg);
*/
int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
{
- struct blk_integrity *b1 = gd1->integrity;
- struct blk_integrity *b2 = gd2->integrity;
+ struct blk_integrity *b1 = &gd1->integrity;
+ struct blk_integrity *b2 = &gd2->integrity;
- if (!b1 && !b2)
+ if (!b1->profile && !b2->profile)
return 0;
- if (!b1 || !b2)
+ if (!b1->profile || !b2->profile)
return -1;
if (b1->interval_exp != b2->interval_exp) {
@@ -163,21 +159,21 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
}
if (b1->tuple_size != b2->tuple_size) {
- printk(KERN_ERR "%s: %s/%s tuple sz %u != %u\n", __func__,
+ pr_err("%s: %s/%s tuple sz %u != %u\n", __func__,
gd1->disk_name, gd2->disk_name,
b1->tuple_size, b2->tuple_size);
return -1;
}
if (b1->tag_size && b2->tag_size && (b1->tag_size != b2->tag_size)) {
- printk(KERN_ERR "%s: %s/%s tag sz %u != %u\n", __func__,
+ pr_err("%s: %s/%s tag sz %u != %u\n", __func__,
gd1->disk_name, gd2->disk_name,
b1->tag_size, b2->tag_size);
return -1;
}
if (b1->profile != b2->profile) {
- printk(KERN_ERR "%s: %s/%s type %s != %s\n", __func__,
+ pr_err("%s: %s/%s type %s != %s\n", __func__,
gd1->disk_name, gd2->disk_name,
b1->profile->name, b2->profile->name);
return -1;
@@ -250,7 +246,7 @@ static ssize_t integrity_attr_show(struct kobject *kobj, struct attribute *attr,
char *page)
{
struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
- struct blk_integrity *bi = blk_get_integrity(disk);
+ struct blk_integrity *bi = &disk->integrity;
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
@@ -262,7 +258,7 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
size_t count)
{
struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
- struct blk_integrity *bi = blk_get_integrity(disk);
+ struct blk_integrity *bi = &disk->integrity;
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
ssize_t ret = 0;
@@ -275,7 +271,7 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
static ssize_t integrity_format_show(struct blk_integrity *bi, char *page)
{
- if (bi != NULL && bi->profile->name != NULL)
+ if (bi->profile && bi->profile->name)
return sprintf(page, "%s\n", bi->profile->name);
else
return sprintf(page, "none\n");
@@ -283,18 +279,13 @@ static ssize_t integrity_format_show(struct blk_integrity *bi, char *page)
static ssize_t integrity_tag_size_show(struct blk_integrity *bi, char *page)
{
- if (bi != NULL)
- return sprintf(page, "%u\n", bi->tag_size);
- else
- return sprintf(page, "0\n");
+ return sprintf(page, "%u\n", bi->tag_size);
}
static ssize_t integrity_interval_show(struct blk_integrity *bi, char *page)
{
- if (bi != NULL)
- return sprintf(page, "%u\n", 1 << bi->interval_exp);
- else
- return sprintf(page, "0\n");
+ return sprintf(page, "%u\n",
+ bi->interval_exp ? 1 << bi->interval_exp : 0);
}
static ssize_t integrity_verify_store(struct blk_integrity *bi,
@@ -388,113 +379,78 @@ static const struct sysfs_ops integrity_ops = {
.store = &integrity_attr_store,
};
-static int __init blk_dev_integrity_init(void)
-{
- integrity_cachep = kmem_cache_create("blkdev_integrity",
- sizeof(struct blk_integrity),
- 0, SLAB_PANIC, NULL);
- return 0;
-}
-subsys_initcall(blk_dev_integrity_init);
-
-static void blk_integrity_release(struct kobject *kobj)
-{
- struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
- struct blk_integrity *bi = blk_get_integrity(disk);
-
- kmem_cache_free(integrity_cachep, bi);
-}
-
static struct kobj_type integrity_ktype = {
.default_attrs = integrity_attrs,
.sysfs_ops = &integrity_ops,
- .release = blk_integrity_release,
};
-bool blk_integrity_is_initialized(struct gendisk *disk)
-{
- struct blk_integrity *bi = blk_get_integrity(disk);
-
- return (bi && bi->profile->name && strcmp(bi->profile->name,
- bi_unsupported_name) != 0);
-}
-EXPORT_SYMBOL(blk_integrity_is_initialized);
-
/**
* blk_integrity_register - Register a gendisk as being integrity-capable
* @disk: struct gendisk pointer to make integrity-aware
- * @template: optional integrity profile to register
+ * @template: block integrity profile to register
*
- * Description: When a device needs to advertise itself as being able
- * to send/receive integrity metadata it must use this function to
- * register the capability with the block layer. The template is a
- * blk_integrity struct with values appropriate for the underlying
- * hardware. If template is NULL the new profile is allocated but
- * not filled out. See Documentation/block/data-integrity.txt.
+ * Description: When a device needs to advertise itself as being able to
+ * send/receive integrity metadata it must use this function to register
+ * the capability with the block layer. The template is a blk_integrity
+ * struct with values appropriate for the underlying hardware. See
+ * Documentation/block/data-integrity.txt.
*/
-int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
+void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
{
- struct blk_integrity *bi;
-
- BUG_ON(disk == NULL);
+ struct blk_integrity *bi = &disk->integrity;
- if (disk->integrity == NULL) {
- bi = kmem_cache_alloc(integrity_cachep,
- GFP_KERNEL | __GFP_ZERO);
- if (!bi)
- return -1;
+ bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
+ template->flags;
+ bi->interval_exp = ilog2(queue_logical_block_size(disk->queue));
+ bi->profile = template->profile;
+ bi->tuple_size = template->tuple_size;
+ bi->tag_size = template->tag_size;
- if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
- &disk_to_dev(disk)->kobj,
- "%s", "integrity")) {
- kmem_cache_free(integrity_cachep, bi);
- return -1;
- }
-
- kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
-
- bi->flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
- bi->interval_exp = ilog2(queue_logical_block_size(disk->queue));
- disk->integrity = bi;
- } else
- bi = disk->integrity;
-
- /* Use the provided profile as template */
- if (template != NULL) {
- bi->profile = template->profile;
- bi->tuple_size = template->tuple_size;
- bi->tag_size = template->tag_size;
- bi->flags |= template->flags;
- } else
- bi->profile->name = bi_unsupported_name;
-
- disk->queue->backing_dev_info.capabilities |= BDI_CAP_STABLE_WRITES;
-
- return 0;
+ blk_integrity_revalidate(disk);
}
EXPORT_SYMBOL(blk_integrity_register);
/**
- * blk_integrity_unregister - Remove block integrity profile
- * @disk: disk whose integrity profile to deallocate
+ * blk_integrity_unregister - Unregister block integrity profile
+ * @disk: disk whose integrity profile to unregister
*
- * Description: This function frees all memory used by the block
- * integrity profile. To be called at device teardown.
+ * Description: This function unregisters the integrity capability from
+ * a block device.
*/
void blk_integrity_unregister(struct gendisk *disk)
{
- struct blk_integrity *bi;
+ blk_integrity_revalidate(disk);
+ memset(&disk->integrity, 0, sizeof(struct blk_integrity));
+}
+EXPORT_SYMBOL(blk_integrity_unregister);
- if (!disk || !disk->integrity)
+void blk_integrity_revalidate(struct gendisk *disk)
+{
+ struct blk_integrity *bi = &disk->integrity;
+
+ if (!(disk->flags & GENHD_FL_UP))
return;
- disk->queue->backing_dev_info.capabilities &= ~BDI_CAP_STABLE_WRITES;
+ if (bi->profile)
+ disk->queue->backing_dev_info.capabilities |=
+ BDI_CAP_STABLE_WRITES;
+ else
+ disk->queue->backing_dev_info.capabilities &=
+ ~BDI_CAP_STABLE_WRITES;
+}
- bi = disk->integrity;
+void blk_integrity_add(struct gendisk *disk)
+{
+ if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
+ &disk_to_dev(disk)->kobj, "%s", "integrity"))
+ return;
+ kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
+}
+
+void blk_integrity_del(struct gendisk *disk)
+{
kobject_uevent(&disk->integrity_kobj, KOBJ_REMOVE);
kobject_del(&disk->integrity_kobj);
kobject_put(&disk->integrity_kobj);
- disk->integrity = NULL;
}
-EXPORT_SYMBOL(blk_integrity_unregister);
diff --git a/block/genhd.c b/block/genhd.c
index 0c706f33a599..e5cafa51567c 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -630,6 +630,7 @@ void add_disk(struct gendisk *disk)
WARN_ON(retval);
disk_add_events(disk);
+ blk_integrity_add(disk);
}
EXPORT_SYMBOL(add_disk);
@@ -638,6 +639,7 @@ void del_gendisk(struct gendisk *disk)
struct disk_part_iter piter;
struct hd_struct *part;
+ blk_integrity_del(disk);
disk_del_events(disk);
/* invalidate stuff */
diff --git a/block/partition-generic.c b/block/partition-generic.c
index e7711133284e..3b030157ec85 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -428,6 +428,7 @@ rescan:
if (disk->fops->revalidate_disk)
disk->fops->revalidate_disk(disk);
+ blk_integrity_revalidate(disk);
check_disk_size_change(disk, bdev);
bdev->bd_invalidated = 0;
if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
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",
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index 7df89b547ae1..e85848caf8d2 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -405,7 +405,6 @@ int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
.generate_fn = nd_pi_nop_generate_verify,
.verify_fn = nd_pi_nop_generate_verify,
};
- int ret;
if (meta_size == 0)
return 0;
@@ -414,10 +413,7 @@ int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
bi.tuple_size = meta_size;
bi.tag_size = meta_size;
- ret = blk_integrity_register(disk, &bi);
- if (ret)
- return ret;
-
+ blk_integrity_register(disk, &bi);
blk_queue_max_integrity_segments(disk->queue, 1);
return 0;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 5dba51d4bae6..d5d877716fd6 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;
@@ -2042,8 +2042,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))
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 073bb57adab1..0a793c7930eb 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1075,7 +1075,7 @@ int revalidate_disk(struct gendisk *disk)
if (disk->fops->revalidate_disk)
ret = disk->fops->revalidate_disk(disk);
-
+ blk_integrity_revalidate(disk);
bdev = bdget_disk(disk, 0);
if (!bdev)
return ret;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4f1968f15e30..60669c20190f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1468,16 +1468,7 @@ struct blk_integrity_profile {
const char *name;
};
-struct blk_integrity {
- struct blk_integrity_profile *profile;
- unsigned char flags;
- unsigned char tuple_size;
- unsigned char interval_exp;
- unsigned char tag_size;
-};
-
-extern bool blk_integrity_is_initialized(struct gendisk *);
-extern int blk_integrity_register(struct gendisk *, struct blk_integrity *);
+extern void blk_integrity_register(struct gendisk *, struct blk_integrity *);
extern void blk_integrity_unregister(struct gendisk *);
extern int blk_integrity_compare(struct gendisk *, struct gendisk *);
extern int blk_rq_map_integrity_sg(struct request_queue *, struct bio *,
@@ -1488,15 +1479,20 @@ extern bool blk_integrity_merge_rq(struct request_queue *, struct request *,
extern bool blk_integrity_merge_bio(struct request_queue *, struct request *,
struct bio *);
-static inline
-struct blk_integrity *bdev_get_integrity(struct block_device *bdev)
+static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
{
- return bdev->bd_disk->integrity;
+ struct blk_integrity *bi = &disk->integrity;
+
+ if (!bi->profile)
+ return NULL;
+
+ return bi;
}
-static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
+static inline
+struct blk_integrity *bdev_get_integrity(struct block_device *bdev)
{
- return disk->integrity;
+ return blk_get_integrity(bdev->bd_disk);
}
static inline bool blk_integrity_rq(struct request *rq)
@@ -1570,10 +1566,9 @@ static inline int blk_integrity_compare(struct gendisk *a, struct gendisk *b)
{
return 0;
}
-static inline int blk_integrity_register(struct gendisk *d,
+static inline void blk_integrity_register(struct gendisk *d,
struct blk_integrity *b)
{
- return 0;
}
static inline void blk_integrity_unregister(struct gendisk *d)
{
@@ -1598,10 +1593,7 @@ static inline bool blk_integrity_merge_bio(struct request_queue *rq,
{
return true;
}
-static inline bool blk_integrity_is_initialized(struct gendisk *g)
-{
- return 0;
-}
+
static inline bool integrity_req_gap_back_merge(struct request *req,
struct bio *next)
{
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 9e6e0dfa97ad..82f4911e0ad8 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -163,6 +163,18 @@ struct disk_part_tbl {
struct disk_events;
+#if defined(CONFIG_BLK_DEV_INTEGRITY)
+
+struct blk_integrity {
+ struct blk_integrity_profile *profile;
+ unsigned char flags;
+ unsigned char tuple_size;
+ unsigned char interval_exp;
+ unsigned char tag_size;
+};
+
+#endif /* CONFIG_BLK_DEV_INTEGRITY */
+
struct gendisk {
/* major, first_minor and minors are input parameters only,
* don't use directly. Use disk_devt() and disk_max_parts().
@@ -198,9 +210,9 @@ struct gendisk {
atomic_t sync_io; /* RAID */
struct disk_events *ev;
#ifdef CONFIG_BLK_DEV_INTEGRITY
- struct blk_integrity *integrity;
+ struct blk_integrity integrity;
struct kobject integrity_kobj;
-#endif
+#endif /* CONFIG_BLK_DEV_INTEGRITY */
int node_id;
};
@@ -728,6 +740,16 @@ static inline void part_nr_sects_write(struct hd_struct *part, sector_t size)
#endif
}
+#if defined(CONFIG_BLK_DEV_INTEGRITY)
+extern void blk_integrity_add(struct gendisk *);
+extern void blk_integrity_del(struct gendisk *);
+extern void blk_integrity_revalidate(struct gendisk *);
+#else /* CONFIG_BLK_DEV_INTEGRITY */
+static inline void blk_integrity_add(struct gendisk *disk) { }
+static inline void blk_integrity_del(struct gendisk *disk) { }
+static inline void blk_integrity_revalidate(struct gendisk *disk) { }
+#endif /* CONFIG_BLK_DEV_INTEGRITY */
+
#else /* CONFIG_BLOCK */
static inline void printk_all_partitions(void) { }
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 06/12] md, dm, scsi, nvme, libnvdimm: drop blk_integrity_unregister() at shutdown
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
` (4 preceding siblings ...)
2015-10-15 20:00 ` [PATCH v2 05/12] block: Inline blk_integrity in struct gendisk Dan Williams
@ 2015-10-15 20:00 ` Dan Williams
2015-10-15 20:00 ` [PATCH v2 07/12] md: suspend i/o during runtime blk_integrity_unregister Dan Williams
` (6 subsequent siblings)
12 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-15 20:00 UTC (permalink / raw)
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 <willy at linux.intel.com>
Cc: Mike Snitzer <snitzer at redhat.com>
Cc: James Bottomley <JBottomley at Odin.com>
Acked-by: NeilBrown <neilb at suse.com>
Acked-by: Keith Busch <keith.busch at intel.com>
Acked-by: Vishal Verma <vishal.l.verma at intel.com>
Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
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(-)
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 d5d877716fd6..65190e51aa47 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2405,11 +2405,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);
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 07/12] md: suspend i/o during runtime blk_integrity_unregister
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
` (5 preceding siblings ...)
2015-10-15 20:00 ` [PATCH v2 06/12] md, dm, scsi, nvme, libnvdimm: drop blk_integrity_unregister() at shutdown Dan Williams
@ 2015-10-15 20:00 ` Dan Williams
2015-10-15 20:00 ` [PATCH v2 08/12] nvme: " Dan Williams
` (5 subsequent siblings)
12 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-15 20:00 UTC (permalink / raw)
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 <neilb at suse.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
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(+)
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);
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 08/12] nvme: suspend i/o during runtime blk_integrity_unregister
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
` (6 preceding siblings ...)
2015-10-15 20:00 ` [PATCH v2 07/12] md: suspend i/o during runtime blk_integrity_unregister Dan Williams
@ 2015-10-15 20:00 ` Dan Williams
2015-10-15 20:00 ` [PATCH v2 09/12] block: generic request_queue reference counting Dan Williams
` (4 subsequent siblings)
12 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-15 20:00 UTC (permalink / raw)
Synchronize pending i/o against a change in the integrity profile to
avoid the possibility of spurious integrity errors.
Cc: Matthew Wilcox <willy at linux.intel.com>
Acked-by: Keith Busch <keith.busch at intel.com>
[keith: also protect dynamic integrity registration]
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
drivers/nvme/host/pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 65190e51aa47..5578de67f406 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2033,6 +2033,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) ||
@@ -2052,6 +2053,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;
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 09/12] block: generic request_queue reference counting
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
` (7 preceding siblings ...)
2015-10-15 20:00 ` [PATCH v2 08/12] nvme: " Dan Williams
@ 2015-10-15 20:00 ` Dan Williams
2015-10-15 20:00 ` [PATCH v2 10/12] block: move blk_integrity to request_queue Dan Williams
` (3 subsequent siblings)
12 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-15 20:00 UTC (permalink / raw)
Allow pmem, and other synchronous/bio-based block drivers, to fallback
on a per-cpu reference count managed by the core for tracking queue
live/dead state.
The existing per-cpu reference count for the blk_mq case is promoted to
be used in all block i/o scenarios. This involves initializing it by
default, waiting for it to drop to zero at exit, and holding a live
reference over the invocation of q->make_request_fn() in
generic_make_request(). The blk_mq code continues to take its own
reference per blk_mq request and retains the ability to freeze the
queue, but the check that the queue is frozen is moved to
generic_make_request().
This fixes crash signatures like the following:
BUG: unable to handle kernel paging request at ffff880140000000
[..]
Call Trace:
[<ffffffff8145e8bf>] ? copy_user_handle_tail+0x5f/0x70
[<ffffffffa004e1e0>] pmem_do_bvec.isra.11+0x70/0xf0 [nd_pmem]
[<ffffffffa004e331>] pmem_make_request+0xd1/0x200 [nd_pmem]
[<ffffffff811c3162>] ? mempool_alloc+0x72/0x1a0
[<ffffffff8141f8b6>] generic_make_request+0xd6/0x110
[<ffffffff8141f966>] submit_bio+0x76/0x170
[<ffffffff81286dff>] submit_bh_wbc+0x12f/0x160
[<ffffffff81286e62>] submit_bh+0x12/0x20
[<ffffffff813395bd>] jbd2_write_superblock+0x8d/0x170
[<ffffffff8133974d>] jbd2_mark_journal_empty+0x5d/0x90
[<ffffffff813399cb>] jbd2_journal_destroy+0x24b/0x270
[<ffffffff810bc4ca>] ? put_pwq_unlocked+0x2a/0x30
[<ffffffff810bc6f5>] ? destroy_workqueue+0x225/0x250
[<ffffffff81303494>] ext4_put_super+0x64/0x360
[<ffffffff8124ab1a>] generic_shutdown_super+0x6a/0xf0
Cc: Jens Axboe <axboe at kernel.dk>
Cc: Keith Busch <keith.busch at intel.com>
Cc: Ross Zwisler <ross.zwisler at linux.intel.com>
Suggested-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Christoph Hellwig <hch at lst.de>
Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
block/blk-core.c | 71 +++++++++++++++++++++++++++++++++++++------
block/blk-mq-sysfs.c | 6 ----
block/blk-mq.c | 80 ++++++++++++++----------------------------------
block/blk-sysfs.c | 3 +-
block/blk.h | 14 ++++++++
include/linux/blk-mq.h | 1 -
include/linux/blkdev.h | 2 +
7 files changed, 102 insertions(+), 75 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 2eb722d48773..9b4d735cb5b8 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -554,13 +554,10 @@ void blk_cleanup_queue(struct request_queue *q)
* Drain all requests queued before DYING marking. Set DEAD flag to
* prevent that q->request_fn() gets invoked after draining finished.
*/
- if (q->mq_ops) {
- blk_mq_freeze_queue(q);
- spin_lock_irq(lock);
- } else {
- spin_lock_irq(lock);
+ blk_freeze_queue(q);
+ spin_lock_irq(lock);
+ if (!q->mq_ops)
__blk_drain_queue(q, true);
- }
queue_flag_set(QUEUE_FLAG_DEAD, q);
spin_unlock_irq(lock);
@@ -570,6 +567,7 @@ void blk_cleanup_queue(struct request_queue *q)
if (q->mq_ops)
blk_mq_free_queue(q);
+ percpu_ref_exit(&q->q_usage_counter);
spin_lock_irq(lock);
if (q->queue_lock != &q->__queue_lock)
@@ -629,6 +627,40 @@ struct request_queue *blk_alloc_queue(gfp_t gfp_mask)
}
EXPORT_SYMBOL(blk_alloc_queue);
+int blk_queue_enter(struct request_queue *q, gfp_t gfp)
+{
+ while (true) {
+ int ret;
+
+ if (percpu_ref_tryget_live(&q->q_usage_counter))
+ return 0;
+
+ if (!(gfp & __GFP_WAIT))
+ return -EBUSY;
+
+ ret = wait_event_interruptible(q->mq_freeze_wq,
+ !atomic_read(&q->mq_freeze_depth) ||
+ blk_queue_dying(q));
+ if (blk_queue_dying(q))
+ return -ENODEV;
+ if (ret)
+ return ret;
+ }
+}
+
+void blk_queue_exit(struct request_queue *q)
+{
+ percpu_ref_put(&q->q_usage_counter);
+}
+
+static void blk_queue_usage_counter_release(struct percpu_ref *ref)
+{
+ struct request_queue *q =
+ container_of(ref, struct request_queue, q_usage_counter);
+
+ wake_up_all(&q->mq_freeze_wq);
+}
+
struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
{
struct request_queue *q;
@@ -690,11 +722,22 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
init_waitqueue_head(&q->mq_freeze_wq);
- if (blkcg_init_queue(q))
+ /*
+ * Init percpu_ref in atomic mode so that it's faster to shutdown.
+ * See blk_register_queue() for details.
+ */
+ if (percpu_ref_init(&q->q_usage_counter,
+ blk_queue_usage_counter_release,
+ PERCPU_REF_INIT_ATOMIC, GFP_KERNEL))
goto fail_bdi;
+ if (blkcg_init_queue(q))
+ goto fail_ref;
+
return q;
+fail_ref:
+ percpu_ref_exit(&q->q_usage_counter);
fail_bdi:
bdi_destroy(&q->backing_dev_info);
fail_split:
@@ -1966,9 +2009,19 @@ void generic_make_request(struct bio *bio)
do {
struct request_queue *q = bdev_get_queue(bio->bi_bdev);
- q->make_request_fn(q, bio);
+ if (likely(blk_queue_enter(q, __GFP_WAIT) == 0)) {
+
+ q->make_request_fn(q, bio);
+
+ blk_queue_exit(q);
- bio = bio_list_pop(current->bio_list);
+ bio = bio_list_pop(current->bio_list);
+ } else {
+ struct bio *bio_next = bio_list_pop(current->bio_list);
+
+ bio_io_error(bio);
+ bio = bio_next;
+ }
} while (bio);
current->bio_list = NULL; /* deactivate */
}
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 788fffd9b409..6f57a110289c 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -413,12 +413,6 @@ static void blk_mq_sysfs_init(struct request_queue *q)
kobject_init(&ctx->kobj, &blk_mq_ctx_ktype);
}
-/* see blk_register_queue() */
-void blk_mq_finish_init(struct request_queue *q)
-{
- percpu_ref_switch_to_percpu(&q->mq_usage_counter);
-}
-
int blk_mq_register_disk(struct gendisk *disk)
{
struct device *dev = disk_to_dev(disk);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index d921cd5177f5..6c240712553a 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -78,47 +78,13 @@ static void blk_mq_hctx_clear_pending(struct blk_mq_hw_ctx *hctx,
clear_bit(CTX_TO_BIT(hctx, ctx), &bm->word);
}
-static int blk_mq_queue_enter(struct request_queue *q, gfp_t gfp)
-{
- while (true) {
- int ret;
-
- if (percpu_ref_tryget_live(&q->mq_usage_counter))
- return 0;
-
- if (!(gfp & __GFP_WAIT))
- return -EBUSY;
-
- ret = wait_event_interruptible(q->mq_freeze_wq,
- !atomic_read(&q->mq_freeze_depth) ||
- blk_queue_dying(q));
- if (blk_queue_dying(q))
- return -ENODEV;
- if (ret)
- return ret;
- }
-}
-
-static void blk_mq_queue_exit(struct request_queue *q)
-{
- percpu_ref_put(&q->mq_usage_counter);
-}
-
-static void blk_mq_usage_counter_release(struct percpu_ref *ref)
-{
- struct request_queue *q =
- container_of(ref, struct request_queue, mq_usage_counter);
-
- wake_up_all(&q->mq_freeze_wq);
-}
-
void blk_mq_freeze_queue_start(struct request_queue *q)
{
int freeze_depth;
freeze_depth = atomic_inc_return(&q->mq_freeze_depth);
if (freeze_depth == 1) {
- percpu_ref_kill(&q->mq_usage_counter);
+ percpu_ref_kill(&q->q_usage_counter);
blk_mq_run_hw_queues(q, false);
}
}
@@ -126,18 +92,34 @@ EXPORT_SYMBOL_GPL(blk_mq_freeze_queue_start);
static void blk_mq_freeze_queue_wait(struct request_queue *q)
{
- wait_event(q->mq_freeze_wq, percpu_ref_is_zero(&q->mq_usage_counter));
+ wait_event(q->mq_freeze_wq, percpu_ref_is_zero(&q->q_usage_counter));
}
/*
* Guarantee no request is in use, so we can change any data structure of
* the queue afterward.
*/
-void blk_mq_freeze_queue(struct request_queue *q)
+void blk_freeze_queue(struct request_queue *q)
{
+ /*
+ * In the !blk_mq case we are only calling this to kill the
+ * q_usage_counter, otherwise this increases the freeze depth
+ * and waits for it to return to zero. For this reason there is
+ * no blk_unfreeze_queue(), and blk_freeze_queue() is not
+ * exported to drivers as the only user for unfreeze is blk_mq.
+ */
blk_mq_freeze_queue_start(q);
blk_mq_freeze_queue_wait(q);
}
+
+void blk_mq_freeze_queue(struct request_queue *q)
+{
+ /*
+ * ...just an alias to keep freeze and unfreeze actions balanced
+ * in the blk_mq_* namespace
+ */
+ blk_freeze_queue(q);
+}
EXPORT_SYMBOL_GPL(blk_mq_freeze_queue);
void blk_mq_unfreeze_queue(struct request_queue *q)
@@ -147,7 +129,7 @@ void blk_mq_unfreeze_queue(struct request_queue *q)
freeze_depth = atomic_dec_return(&q->mq_freeze_depth);
WARN_ON_ONCE(freeze_depth < 0);
if (!freeze_depth) {
- percpu_ref_reinit(&q->mq_usage_counter);
+ percpu_ref_reinit(&q->q_usage_counter);
wake_up_all(&q->mq_freeze_wq);
}
}
@@ -256,7 +238,7 @@ struct request *blk_mq_alloc_request(struct request_queue *q, int rw, gfp_t gfp,
struct blk_mq_alloc_data alloc_data;
int ret;
- ret = blk_mq_queue_enter(q, gfp);
+ ret = blk_queue_enter(q, gfp);
if (ret)
return ERR_PTR(ret);
@@ -279,7 +261,7 @@ struct request *blk_mq_alloc_request(struct request_queue *q, int rw, gfp_t gfp,
}
blk_mq_put_ctx(ctx);
if (!rq) {
- blk_mq_queue_exit(q);
+ blk_queue_exit(q);
return ERR_PTR(-EWOULDBLOCK);
}
return rq;
@@ -298,7 +280,7 @@ static void __blk_mq_free_request(struct blk_mq_hw_ctx *hctx,
clear_bit(REQ_ATOM_STARTED, &rq->atomic_flags);
blk_mq_put_tag(hctx, tag, &ctx->last_tag);
- blk_mq_queue_exit(q);
+ blk_queue_exit(q);
}
void blk_mq_free_hctx_request(struct blk_mq_hw_ctx *hctx, struct request *rq)
@@ -1177,11 +1159,7 @@ static struct request *blk_mq_map_request(struct request_queue *q,
int rw = bio_data_dir(bio);
struct blk_mq_alloc_data alloc_data;
- if (unlikely(blk_mq_queue_enter(q, GFP_KERNEL))) {
- bio_io_error(bio);
- return NULL;
- }
-
+ blk_queue_enter_live(q);
ctx = blk_mq_get_ctx(q);
hctx = q->mq_ops->map_queue(q, ctx->cpu);
@@ -2000,14 +1978,6 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
hctxs[i]->queue_num = i;
}
- /*
- * Init percpu_ref in atomic mode so that it's faster to shutdown.
- * See blk_register_queue() for details.
- */
- if (percpu_ref_init(&q->mq_usage_counter, blk_mq_usage_counter_release,
- PERCPU_REF_INIT_ATOMIC, GFP_KERNEL))
- goto err_hctxs;
-
setup_timer(&q->timeout, blk_mq_rq_timer, (unsigned long) q);
blk_queue_rq_timeout(q, set->timeout ? set->timeout : 30 * HZ);
@@ -2088,8 +2058,6 @@ void blk_mq_free_queue(struct request_queue *q)
blk_mq_exit_hw_queues(q, set, set->nr_hw_queues);
blk_mq_free_hw_queues(q, set);
-
- percpu_ref_exit(&q->mq_usage_counter);
}
/* Basically redo blk_mq_init_queue with queue frozen */
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 3e44a9da2a13..61fc2633bbea 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -599,9 +599,8 @@ int blk_register_queue(struct gendisk *disk)
*/
if (!blk_queue_init_done(q)) {
queue_flag_set_unlocked(QUEUE_FLAG_INIT_DONE, q);
+ percpu_ref_switch_to_percpu(&q->q_usage_counter);
blk_queue_bypass_end(q);
- if (q->mq_ops)
- blk_mq_finish_init(q);
}
ret = blk_trace_init_sysfs(dev);
diff --git a/block/blk.h b/block/blk.h
index 98614ad37c81..5b2cd393afbe 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -72,6 +72,20 @@ void blk_dequeue_request(struct request *rq);
void __blk_queue_free_tags(struct request_queue *q);
bool __blk_end_bidi_request(struct request *rq, int error,
unsigned int nr_bytes, unsigned int bidi_bytes);
+int blk_queue_enter(struct request_queue *q, gfp_t gfp);
+void blk_queue_exit(struct request_queue *q);
+void blk_freeze_queue(struct request_queue *q);
+
+static inline void blk_queue_enter_live(struct request_queue *q)
+{
+ /*
+ * Given that running in generic_make_request() context
+ * guarantees that a live reference against q_usage_counter has
+ * been established, further references under that same context
+ * need not check that the queue has been frozen (marked dead).
+ */
+ percpu_ref_get(&q->q_usage_counter);
+}
void blk_rq_timed_out_timer(unsigned long data);
unsigned long blk_rq_timeout(unsigned long timeout);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 5e7d43ab61c0..83cc9d4e5455 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -166,7 +166,6 @@ enum {
struct request_queue *blk_mq_init_queue(struct blk_mq_tag_set *);
struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
struct request_queue *q);
-void blk_mq_finish_init(struct request_queue *q);
int blk_mq_register_disk(struct gendisk *);
void blk_mq_unregister_disk(struct gendisk *);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 60669c20190f..3e0465257d68 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -450,7 +450,7 @@ struct request_queue {
#endif
struct rcu_head rcu_head;
wait_queue_head_t mq_freeze_wq;
- struct percpu_ref mq_usage_counter;
+ struct percpu_ref q_usage_counter;
struct list_head all_q_node;
struct blk_mq_tag_set *tag_set;
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 10/12] block: move blk_integrity to request_queue
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
` (8 preceding siblings ...)
2015-10-15 20:00 ` [PATCH v2 09/12] block: generic request_queue reference counting Dan Williams
@ 2015-10-15 20:00 ` Dan Williams
2015-10-16 0:42 ` Williams, Dan J
2015-10-15 20:00 ` [PATCH v2 11/12] block: blk_flush_integrity() for bio-based drivers Dan Williams
` (2 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: Dan Williams @ 2015-10-15 20:00 UTC (permalink / raw)
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: [<ffff8800d31b10d8>] 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:
[<ffffffff8144e0f9>] ? bio_integrity_process+0x159/0x2d0
[<ffffffff8144e4f6>] bio_integrity_verify_fn+0x36/0x60
[<ffffffff810bd2dc>] 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 <hch at lst.de>
Cc: Martin K. Petersen <martin.petersen at oracle.com>
[martin: fix the CONFIG_BLK_DEV_INTEGRITY=n case]
Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
Documentation/ABI/testing/sysfs-block | 12 +++---
block/blk-integrity.c | 63 +++++++++++++++++----------------
block/blk-sysfs.c | 4 ++
block/genhd.c | 2 -
block/partition-generic.c | 2 +
drivers/md/dm-table.c | 4 +-
drivers/md/md.c | 4 +-
drivers/nvdimm/core.c | 2 +
drivers/nvme/host/pci.c | 8 +++-
drivers/scsi/sd_dif.c | 2 +
fs/block_dev.c | 2 +
include/linux/blkdev.h | 16 ++++++--
include/linux/genhd.h | 16 +++-----
13 files changed, 72 insertions(+), 65 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index 71d184dbb70d..9d3c9aa67d67 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -28,7 +28,7 @@ Description:
format.
-What: /sys/block/<disk>/integrity/format
+What: /sys/block/<disk>/queue/integrity/format
Date: June 2008
Contact: Martin K. Petersen <martin.petersen at oracle.com>
Description:
@@ -36,7 +36,7 @@ Description:
E.g. T10-DIF-TYPE1-CRC.
-What: /sys/block/<disk>/integrity/read_verify
+What: /sys/block/<disk>/queue/integrity/read_verify
Date: June 2008
Contact: Martin K. Petersen <martin.petersen at oracle.com>
Description:
@@ -45,7 +45,7 @@ Description:
support sending integrity metadata.
-What: /sys/block/<disk>/integrity/tag_size
+What: /sys/block/<disk>/queue/integrity/tag_size
Date: June 2008
Contact: Martin K. Petersen <martin.petersen at oracle.com>
Description:
@@ -53,14 +53,14 @@ Description:
512 bytes of data.
-What: /sys/block/<disk>/integrity/device_is_integrity_capable
+What: /sys/block/<disk>/queue/integrity/device_is_integrity_capable
Date: July 2014
Contact: Martin K. Petersen <martin.petersen at oracle.com>
Description:
Indicates whether a storage device is capable of storing
integrity metadata. Set if the device is T10 PI-capable.
-What: /sys/block/<disk>/integrity/protection_interval_bytes
+What: /sys/block/<disk>/queue/integrity/protection_interval_bytes
Date: July 2015
Contact: Martin K. Petersen <martin.petersen at oracle.com>
Description:
@@ -68,7 +68,7 @@ Description:
by one integrity tuple. Typically the device's logical
block size.
-What: /sys/block/<disk>/integrity/write_generate
+What: /sys/block/<disk>/queue/integrity/write_generate
Date: June 2008
Contact: Martin K. Petersen <martin.petersen at oracle.com>
Description:
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 4615a3386798..dc4dea7b8a93 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -142,8 +142,8 @@ EXPORT_SYMBOL(blk_rq_map_integrity_sg);
*/
int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
{
- struct blk_integrity *b1 = &gd1->integrity;
- struct blk_integrity *b2 = &gd2->integrity;
+ struct blk_integrity *b1 = blk_get_integrity(gd1);
+ struct blk_integrity *b2 = blk_get_integrity(gd2);
if (!b1->profile && !b2->profile)
return 0;
@@ -245,8 +245,8 @@ struct integrity_sysfs_entry {
static ssize_t integrity_attr_show(struct kobject *kobj, struct attribute *attr,
char *page)
{
- struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
- struct blk_integrity *bi = &disk->integrity;
+ struct request_queue *q = container_of(kobj, struct request_queue, integrity_kobj);
+ struct blk_integrity *bi = &q->integrity;
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
@@ -257,8 +257,8 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
struct attribute *attr, const char *page,
size_t count)
{
- struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
- struct blk_integrity *bi = &disk->integrity;
+ struct request_queue *q = container_of(kobj, struct request_queue, integrity_kobj);
+ struct blk_integrity *bi = &q->integrity;
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
ssize_t ret = 0;
@@ -385,8 +385,8 @@ static struct kobj_type integrity_ktype = {
};
/**
- * blk_integrity_register - Register a gendisk as being integrity-capable
- * @disk: struct gendisk pointer to make integrity-aware
+ * blk_integrity_register - Register a request_queue as being integrity-capable
+ * @disk: struct request_queue pointer to make integrity-aware
* @template: block integrity profile to register
*
* Description: When a device needs to advertise itself as being able to
@@ -395,62 +395,63 @@ static struct kobj_type integrity_ktype = {
* struct with values appropriate for the underlying hardware. See
* Documentation/block/data-integrity.txt.
*/
-void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
+void blk_integrity_register(struct request_queue *q, struct blk_integrity *template)
{
- struct blk_integrity *bi = &disk->integrity;
+ struct blk_integrity *bi = &q->integrity;
bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
template->flags;
- bi->interval_exp = ilog2(queue_logical_block_size(disk->queue));
+ bi->interval_exp = ilog2(queue_logical_block_size(q));
bi->profile = template->profile;
bi->tuple_size = template->tuple_size;
bi->tag_size = template->tag_size;
- blk_integrity_revalidate(disk);
+ blk_integrity_revalidate(q);
}
EXPORT_SYMBOL(blk_integrity_register);
/**
* blk_integrity_unregister - Unregister block integrity profile
- * @disk: disk whose integrity profile to unregister
+ * @disk: queue whose integrity profile to unregister
*
* Description: This function unregisters the integrity capability from
* a block device.
*/
-void blk_integrity_unregister(struct gendisk *disk)
+void blk_integrity_unregister(struct request_queue *q)
{
- blk_integrity_revalidate(disk);
- memset(&disk->integrity, 0, sizeof(struct blk_integrity));
+ blk_integrity_revalidate(q);
+ memset(&q->integrity, 0, sizeof(struct blk_integrity));
}
EXPORT_SYMBOL(blk_integrity_unregister);
-void blk_integrity_revalidate(struct gendisk *disk)
+void blk_integrity_revalidate(struct request_queue *q)
{
- struct blk_integrity *bi = &disk->integrity;
+ struct blk_integrity *bi = &q->integrity;
+ int rc;
- if (!(disk->flags & GENHD_FL_UP))
+ rc = blk_queue_enter(q, GFP_NOWAIT);
+ if (rc)
return;
if (bi->profile)
- disk->queue->backing_dev_info.capabilities |=
- BDI_CAP_STABLE_WRITES;
+ q->backing_dev_info.capabilities |= BDI_CAP_STABLE_WRITES;
else
- disk->queue->backing_dev_info.capabilities &=
- ~BDI_CAP_STABLE_WRITES;
+ q->backing_dev_info.capabilities &= ~BDI_CAP_STABLE_WRITES;
+ blk_queue_exit(q);
}
-void blk_integrity_add(struct gendisk *disk)
+void blk_integrity_add(struct request_queue *q)
{
- if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
- &disk_to_dev(disk)->kobj, "%s", "integrity"))
+ if (kobject_init_and_add(&q->integrity_kobj, &integrity_ktype,
+ &q->kobj, "%s", "integrity"))
return;
- kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
+ kobject_uevent(&q->integrity_kobj, KOBJ_ADD);
}
-void blk_integrity_del(struct gendisk *disk)
+void blk_integrity_del(struct request_queue *q)
{
- kobject_uevent(&disk->integrity_kobj, KOBJ_REMOVE);
- kobject_del(&disk->integrity_kobj);
- kobject_put(&disk->integrity_kobj);
+ kobject_uevent(&q->integrity_kobj, KOBJ_REMOVE);
+ kobject_del(&q->integrity_kobj);
+ kobject_put(&q->integrity_kobj);
}
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 61fc2633bbea..ea8b84d35a53 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -613,6 +613,8 @@ int blk_register_queue(struct gendisk *disk)
return ret;
}
+ blk_integrity_add(q);
+
kobject_uevent(&q->kobj, KOBJ_ADD);
if (q->mq_ops)
@@ -623,6 +625,7 @@ int blk_register_queue(struct gendisk *disk)
ret = elv_register_queue(q);
if (ret) {
+ blk_integrity_del(q);
kobject_uevent(&q->kobj, KOBJ_REMOVE);
kobject_del(&q->kobj);
blk_trace_remove_sysfs(dev);
@@ -646,6 +649,7 @@ void blk_unregister_queue(struct gendisk *disk)
if (q->request_fn)
elv_unregister_queue(q);
+ blk_integrity_del(q);
kobject_uevent(&q->kobj, KOBJ_REMOVE);
kobject_del(&q->kobj);
blk_trace_remove_sysfs(disk_to_dev(disk));
diff --git a/block/genhd.c b/block/genhd.c
index e5cafa51567c..0c706f33a599 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -630,7 +630,6 @@ void add_disk(struct gendisk *disk)
WARN_ON(retval);
disk_add_events(disk);
- blk_integrity_add(disk);
}
EXPORT_SYMBOL(add_disk);
@@ -639,7 +638,6 @@ void del_gendisk(struct gendisk *disk)
struct disk_part_iter piter;
struct hd_struct *part;
- blk_integrity_del(disk);
disk_del_events(disk);
/* invalidate stuff */
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 3b030157ec85..47ad1953e365 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -428,7 +428,7 @@ rescan:
if (disk->fops->revalidate_disk)
disk->fops->revalidate_disk(disk);
- blk_integrity_revalidate(disk);
+ blk_integrity_revalidate(disk->queue);
check_disk_size_change(disk, bdev);
bdev->bd_invalidated = 0;
if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 061152a43730..cd074c4df85e 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1075,7 +1075,7 @@ static int dm_table_register_integrity(struct dm_table *t)
* 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_integrity_register(dm_disk(md)->queue,
blk_get_integrity(template_disk));
return 0;
}
@@ -1305,7 +1305,7 @@ static void dm_table_verify_integrity(struct dm_table *t)
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));
+ blk_integrity_unregister(dm_disk(t->md)->queue);
}
}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 714aa92db174..4feff233d453 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1962,7 +1962,7 @@ 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.
*/
- blk_integrity_register(mddev->gendisk,
+ blk_integrity_register(mddev->gendisk->queue,
bdev_get_integrity(reference->bdev));
printk(KERN_NOTICE "md: data integrity enabled on %s\n", mdname(mddev));
@@ -1996,7 +1996,7 @@ void md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev)
return;
WARN_ON_ONCE(!mddev->suspended);
printk(KERN_NOTICE "disabling data integrity on %s\n", mdname(mddev));
- blk_integrity_unregister(mddev->gendisk);
+ blk_integrity_unregister(mddev->gendisk->queue);
}
EXPORT_SYMBOL(md_integrity_add_rdev);
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index e85848caf8d2..eeedd58bbcad 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -413,7 +413,7 @@ int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
bi.tuple_size = meta_size;
bi.tag_size = meta_size;
- blk_integrity_register(disk, &bi);
+ blk_integrity_register(disk->queue, &bi);
blk_queue_max_integrity_segments(disk->queue, 1);
return 0;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 5578de67f406..e4a0cc7fb421 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -521,6 +521,7 @@ static void nvme_dif_remap(struct request *req,
{
struct nvme_ns *ns = req->rq_disk->private_data;
struct bio_integrity_payload *bip;
+ struct blk_integrity *bi;
struct t10_pi_tuple *pi;
void *p, *pmap;
u32 i, nlb, ts, phys, virt;
@@ -538,7 +539,8 @@ 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;
+ bi = blk_get_integrity(ns->disk);
+ ts = bi->tuple_size;
for (i = 0; i < nlb; i++, virt++, phys++) {
pi = (struct t10_pi_tuple *)p;
@@ -581,7 +583,7 @@ static void nvme_init_integrity(struct nvme_ns *ns)
break;
}
integrity.tuple_size = ns->ms;
- blk_integrity_register(ns->disk, &integrity);
+ blk_integrity_register(ns->disk->queue, &integrity);
blk_queue_max_integrity_segments(ns->queue, 1);
}
#else /* CONFIG_BLK_DEV_INTEGRITY */
@@ -2038,7 +2040,7 @@ static int nvme_revalidate_disk(struct gendisk *disk)
ns->ms != old_ms ||
bs != queue_logical_block_size(disk->queue) ||
(ns->ms && ns->ext)))
- blk_integrity_unregister(disk);
+ blk_integrity_unregister(disk->queue);
ns->pi_type = pi_type;
blk_queue_logical_block_size(ns->queue, bs);
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 5a5ec9aa26b3..60ba4d9def6c 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -90,7 +90,7 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
}
out:
- blk_integrity_register(disk, &bi);
+ blk_integrity_register(disk->queue, &bi);
}
/*
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 0a793c7930eb..e3528c8c48ae 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1075,7 +1075,7 @@ int revalidate_disk(struct gendisk *disk)
if (disk->fops->revalidate_disk)
ret = disk->fops->revalidate_disk(disk);
- blk_integrity_revalidate(disk);
+ blk_integrity_revalidate(disk->queue);
bdev = bdget_disk(disk, 0);
if (!bdev)
return ret;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 3e0465257d68..5f55b1f4215e 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -369,6 +369,11 @@ struct request_queue {
*/
struct kobject mq_kobj;
+#ifdef CONFIG_BLK_DEV_INTEGRITY
+ struct blk_integrity integrity;
+ struct kobject integrity_kobj;
+#endif /* CONFIG_BLK_DEV_INTEGRITY */
+
#ifdef CONFIG_PM
struct device *dev;
int rpm_status;
@@ -1468,8 +1473,8 @@ struct blk_integrity_profile {
const char *name;
};
-extern void blk_integrity_register(struct gendisk *, struct blk_integrity *);
-extern void blk_integrity_unregister(struct gendisk *);
+extern void blk_integrity_register(struct request_queue *, struct blk_integrity *);
+extern void blk_integrity_unregister(struct request_queue *);
extern int blk_integrity_compare(struct gendisk *, struct gendisk *);
extern int blk_rq_map_integrity_sg(struct request_queue *, struct bio *,
struct scatterlist *);
@@ -1481,7 +1486,7 @@ extern bool blk_integrity_merge_bio(struct request_queue *, struct request *,
static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
{
- struct blk_integrity *bi = &disk->integrity;
+ struct blk_integrity *bi = &disk->queue->integrity;
if (!bi->profile)
return NULL;
@@ -1537,6 +1542,7 @@ static inline bool integrity_req_gap_front_merge(struct request *req,
struct bio;
struct block_device;
struct gendisk;
+struct request_queue;
struct blk_integrity;
static inline int blk_integrity_rq(struct request *rq)
@@ -1566,11 +1572,11 @@ static inline int blk_integrity_compare(struct gendisk *a, struct gendisk *b)
{
return 0;
}
-static inline void blk_integrity_register(struct gendisk *d,
+static inline void blk_integrity_register(struct request_queue *q,
struct blk_integrity *b)
{
}
-static inline void blk_integrity_unregister(struct gendisk *d)
+static inline void blk_integrity_unregister(struct request_queue *q)
{
}
static inline void blk_queue_max_integrity_segments(struct request_queue *q,
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 82f4911e0ad8..f842a85c71a0 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -209,10 +209,6 @@ struct gendisk {
struct timer_rand_state *random;
atomic_t sync_io; /* RAID */
struct disk_events *ev;
-#ifdef CONFIG_BLK_DEV_INTEGRITY
- struct blk_integrity integrity;
- struct kobject integrity_kobj;
-#endif /* CONFIG_BLK_DEV_INTEGRITY */
int node_id;
};
@@ -741,13 +737,13 @@ static inline void part_nr_sects_write(struct hd_struct *part, sector_t size)
}
#if defined(CONFIG_BLK_DEV_INTEGRITY)
-extern void blk_integrity_add(struct gendisk *);
-extern void blk_integrity_del(struct gendisk *);
-extern void blk_integrity_revalidate(struct gendisk *);
+extern void blk_integrity_add(struct request_queue *);
+extern void blk_integrity_del(struct request_queue *);
+extern void blk_integrity_revalidate(struct request_queue *);
#else /* CONFIG_BLK_DEV_INTEGRITY */
-static inline void blk_integrity_add(struct gendisk *disk) { }
-static inline void blk_integrity_del(struct gendisk *disk) { }
-static inline void blk_integrity_revalidate(struct gendisk *disk) { }
+static inline void blk_integrity_add(struct request_queue *q) { }
+static inline void blk_integrity_del(struct request_queue *q) { }
+static inline void blk_integrity_revalidate(struct request_queue *q) { }
#endif /* CONFIG_BLK_DEV_INTEGRITY */
#else /* CONFIG_BLOCK */
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 10/12] block: move blk_integrity to request_queue
2015-10-15 20:00 ` [PATCH v2 10/12] block: move blk_integrity to request_queue Dan Williams
@ 2015-10-16 0:42 ` Williams, Dan J
2015-10-20 2:24 ` Martin K. Petersen
0 siblings, 1 reply; 33+ messages in thread
From: Williams, Dan J @ 2015-10-16 0:42 UTC (permalink / raw)
On Thu, 2015-10-15@16:00 -0400, Dan Williams wrote:
> 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: [<ffff8800d31b10d8>] 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:
> [<ffffffff8144e0f9>] ? bio_integrity_process+0x159/0x2d0
> [<ffffffff8144e4f6>] bio_integrity_verify_fn+0x36/0x60
> [<ffffffff810bd2dc>] 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 <hch at lst.de>
> Cc: Martin K. Petersen <martin.petersen at oracle.com>
> [martin: fix the CONFIG_BLK_DEV_INTEGRITY=n case]
> Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams at intel.com>
> ---
> Documentation/ABI/testing/sysfs-block | 12 +++---
> block/blk-integrity.c | 63 +++++++++++++++++----------------
> block/blk-sysfs.c | 4 ++
> block/genhd.c | 2 -
> block/partition-generic.c | 2 +
> drivers/md/dm-table.c | 4 +-
> drivers/md/md.c | 4 +-
> drivers/nvdimm/core.c | 2 +
> drivers/nvme/host/pci.c | 8 +++-
> drivers/scsi/sd_dif.c | 2 +
> fs/block_dev.c | 2 +
> include/linux/blkdev.h | 16 ++++++--
> include/linux/genhd.h | 16 +++-----
> 13 files changed, 72 insertions(+), 65 deletions(-)
>
Martin pointed out that I broke compatibility by changing the sysfs
layout of the integrity attributes. Rather than add a sysfs-link we can
make this patch simpler by only moving struct blk_integrity to
request_queue and leave integrity_kobj where it is in gendisk.
Also undo the conversion of blk_integrity apis take a request_queue.
Through this simplification I also noticed that I had broken
blk_integrity_compare()
Here's the much smaller v3, and I also refreshed for-4.4/blk-integrity
8<-----
Subject: block: move blk_integrity to request_queue
From: Dan Williams <dan.j.williams@intel.com>
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: [<ffff8800d31b10d8>] 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:
[<ffffffff8144e0f9>] ? bio_integrity_process+0x159/0x2d0
[<ffffffff8144e4f6>] bio_integrity_verify_fn+0x36/0x60
[<ffffffff810bd2dc>] 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 <hch at lst.de>
Cc: Martin K. Petersen <martin.petersen at oracle.com>
[martin: fix the CONFIG_BLK_DEV_INTEGRITY=n case]
Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
block/blk-integrity.c | 14 +++++++-------
drivers/nvme/host/pci.c | 2 +-
include/linux/blkdev.h | 6 +++++-
include/linux/genhd.h | 1 -
4 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 4615a3386798..5d339ae64d56 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -142,8 +142,8 @@ EXPORT_SYMBOL(blk_rq_map_integrity_sg);
*/
int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
{
- struct blk_integrity *b1 = &gd1->integrity;
- struct blk_integrity *b2 = &gd2->integrity;
+ struct blk_integrity *b1 = &gd1->queue->integrity;
+ struct blk_integrity *b2 = &gd2->queue->integrity;
if (!b1->profile && !b2->profile)
return 0;
@@ -246,7 +246,7 @@ static ssize_t integrity_attr_show(struct kobject *kobj, struct attribute *attr,
char *page)
{
struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
- struct blk_integrity *bi = &disk->integrity;
+ struct blk_integrity *bi = &disk->queue->integrity;
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
@@ -258,7 +258,7 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
size_t count)
{
struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
- struct blk_integrity *bi = &disk->integrity;
+ struct blk_integrity *bi = &disk->queue->integrity;
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
ssize_t ret = 0;
@@ -397,7 +397,7 @@ static struct kobj_type integrity_ktype = {
*/
void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
{
- struct blk_integrity *bi = &disk->integrity;
+ struct blk_integrity *bi = &disk->queue->integrity;
bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
template->flags;
@@ -420,13 +420,13 @@ EXPORT_SYMBOL(blk_integrity_register);
void blk_integrity_unregister(struct gendisk *disk)
{
blk_integrity_revalidate(disk);
- memset(&disk->integrity, 0, sizeof(struct blk_integrity));
+ memset(&disk->queue->integrity, 0, sizeof(struct blk_integrity));
}
EXPORT_SYMBOL(blk_integrity_unregister);
void blk_integrity_revalidate(struct gendisk *disk)
{
- struct blk_integrity *bi = &disk->integrity;
+ struct blk_integrity *bi = &disk->queue->integrity;
if (!(disk->flags & GENHD_FL_UP))
return;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 5578de67f406..2a39314fc61a 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;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 3e0465257d68..cf57884db4b7 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -369,6 +369,10 @@ struct request_queue {
*/
struct kobject mq_kobj;
+#ifdef CONFIG_BLK_DEV_INTEGRITY
+ struct blk_integrity integrity;
+#endif /* CONFIG_BLK_DEV_INTEGRITY */
+
#ifdef CONFIG_PM
struct device *dev;
int rpm_status;
@@ -1481,7 +1485,7 @@ extern bool blk_integrity_merge_bio(struct request_queue *, struct request *,
static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
{
- struct blk_integrity *bi = &disk->integrity;
+ struct blk_integrity *bi = &disk->queue->integrity;
if (!bi->profile)
return NULL;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 82f4911e0ad8..847cc1d91634 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -210,7 +210,6 @@ struct gendisk {
atomic_t sync_io; /* RAID */
struct disk_events *ev;
#ifdef CONFIG_BLK_DEV_INTEGRITY
- struct blk_integrity integrity;
struct kobject integrity_kobj;
#endif /* CONFIG_BLK_DEV_INTEGRITY */
int node_id;
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 10/12] block: move blk_integrity to request_queue
2015-10-16 0:42 ` Williams, Dan J
@ 2015-10-20 2:24 ` Martin K. Petersen
2015-10-20 2:45 ` Simplify block integrity registration v2 Martin K. Petersen
0 siblings, 1 reply; 33+ messages in thread
From: Martin K. Petersen @ 2015-10-20 2:24 UTC (permalink / raw)
>>>>> "Dan" == Williams, Dan J <dan.j.williams at intel.com> writes:
Dan> Martin pointed out that I broke compatibility by changing the sysfs
Dan> layout of the integrity attributes. Rather than add a sysfs-link
Dan> we can make this patch simpler by only moving struct blk_integrity
Dan> to request_queue and leave integrity_kobj where it is in gendisk.
Dan> Also undo the conversion of blk_integrity apis take a
Dan> request_queue. Through this simplification I also noticed that I
Dan> had broken blk_integrity_compare()
Dan> Here's the much smaller v3, and I also refreshed
Dan> for-4.4/blk-integrity
Much better.
I ran some test on our combined trees over the weekend and tripped a bug
on some SCSI configs. Turned out to be my fault so I'll repost with a
fixed patch 2.
You can either rebase on top of the new series or maybe Jens will merge
my patches first. Either way works for me.
In any case I'm OK with your latest changes. Feel free to add my
Acked-by.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 33+ messages in thread
* Simplify block integrity registration v2
2015-10-20 2:24 ` Martin K. Petersen
@ 2015-10-20 2:45 ` Martin K. Petersen
2015-10-20 2:45 ` [PATCH 1/5] block: Move integrity kobject to struct gendisk Martin K. Petersen
` (5 more replies)
0 siblings, 6 replies; 33+ messages in thread
From: Martin K. Petersen @ 2015-10-20 2:45 UTC (permalink / raw)
Changes since last version:
- Patch 2/5: Zero template to prevent problems on SCSI devices w/ ATO=0
- Patch 5/5: Add Mike's SoB
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/5] block: Move integrity kobject to struct gendisk
2015-10-20 2:45 ` Simplify block integrity registration v2 Martin K. Petersen
@ 2015-10-20 2:45 ` Martin K. Petersen
2015-10-20 2:45 ` [PATCH 2/5] block: Consolidate static integrity profile properties Martin K. Petersen
` (4 subsequent siblings)
5 siblings, 0 replies; 33+ messages in thread
From: Martin K. Petersen @ 2015-10-20 2:45 UTC (permalink / raw)
The integrity kobject purely exists to support the integrity
subdirectory in sysfs and doesn't really have anything to do with the
blk_integrity data structure. Move the kobject to struct gendisk where
it belongs.
Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
Reported-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
---
block/blk-integrity.c | 22 +++++++++++-----------
include/linux/blkdev.h | 2 --
include/linux/genhd.h | 1 +
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 75f29cf70188..182bfd2383ea 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -249,8 +249,8 @@ struct integrity_sysfs_entry {
static ssize_t integrity_attr_show(struct kobject *kobj, struct attribute *attr,
char *page)
{
- struct blk_integrity *bi =
- container_of(kobj, struct blk_integrity, kobj);
+ struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
+ struct blk_integrity *bi = blk_get_integrity(disk);
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
@@ -261,8 +261,8 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
struct attribute *attr, const char *page,
size_t count)
{
- struct blk_integrity *bi =
- container_of(kobj, struct blk_integrity, kobj);
+ struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
+ struct blk_integrity *bi = blk_get_integrity(disk);
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
ssize_t ret = 0;
@@ -385,8 +385,8 @@ subsys_initcall(blk_dev_integrity_init);
static void blk_integrity_release(struct kobject *kobj)
{
- struct blk_integrity *bi =
- container_of(kobj, struct blk_integrity, kobj);
+ struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
+ struct blk_integrity *bi = blk_get_integrity(disk);
kmem_cache_free(integrity_cachep, bi);
}
@@ -429,14 +429,14 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
if (!bi)
return -1;
- if (kobject_init_and_add(&bi->kobj, &integrity_ktype,
+ if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
&disk_to_dev(disk)->kobj,
"%s", "integrity")) {
kmem_cache_free(integrity_cachep, bi);
return -1;
}
- kobject_uevent(&bi->kobj, KOBJ_ADD);
+ kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
bi->flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
bi->interval = queue_logical_block_size(disk->queue);
@@ -479,9 +479,9 @@ void blk_integrity_unregister(struct gendisk *disk)
bi = disk->integrity;
- kobject_uevent(&bi->kobj, KOBJ_REMOVE);
- kobject_del(&bi->kobj);
- kobject_put(&bi->kobj);
+ kobject_uevent(&disk->integrity_kobj, KOBJ_REMOVE);
+ kobject_del(&disk->integrity_kobj);
+ kobject_put(&disk->integrity_kobj);
disk->integrity = NULL;
}
EXPORT_SYMBOL(blk_integrity_unregister);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 19c2e947d4d1..830f9c07d4bb 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1472,8 +1472,6 @@ struct blk_integrity {
unsigned short tag_size;
const char *name;
-
- struct kobject kobj;
};
extern bool blk_integrity_is_initialized(struct gendisk *);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 2adbfa6d02bc..9e6e0dfa97ad 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -199,6 +199,7 @@ struct gendisk {
struct disk_events *ev;
#ifdef CONFIG_BLK_DEV_INTEGRITY
struct blk_integrity *integrity;
+ struct kobject integrity_kobj;
#endif
int node_id;
};
--
2.4.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 2/5] block: Consolidate static integrity profile properties
2015-10-20 2:45 ` Simplify block integrity registration v2 Martin K. Petersen
2015-10-20 2:45 ` [PATCH 1/5] block: Move integrity kobject to struct gendisk Martin K. Petersen
@ 2015-10-20 2:45 ` Martin K. Petersen
2015-10-20 18:20 ` Dan Williams
2015-10-20 2:45 ` [PATCH 3/5] block: Reduce the size of struct blk_integrity Martin K. Petersen
` (3 subsequent siblings)
5 siblings, 1 reply; 33+ messages in thread
From: Martin K. Petersen @ 2015-10-20 2:45 UTC (permalink / raw)
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 <martin.petersen at oracle.com>
Reported-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
Cc: Dan Williams <dan.j.williams at intel.com>
---
block/bio-integrity.c | 8 ++++----
block/blk-integrity.c | 17 ++++++++---------
block/t10-pi.c | 16 ++++------------
drivers/block/nvme-core.c | 8 ++++----
drivers/nvdimm/core.c | 11 +++++++----
drivers/scsi/sd_dif.c | 31 ++++++++++++++++++-------------
drivers/target/target_core_iblock.c | 10 +++++-----
include/linux/blkdev.h | 20 +++++++++++---------
include/linux/t10-pi.h | 8 ++++----
9 files changed, 65 insertions(+), 64 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 14b8faf8b09d..a10ffe19a8dd 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -177,11 +177,11 @@ bool bio_integrity_enabled(struct bio *bio)
if (bi == NULL)
return false;
- if (bio_data_dir(bio) == READ && bi->verify_fn != NULL &&
+ if (bio_data_dir(bio) == READ && bi->profile->verify_fn != NULL &&
(bi->flags & BLK_INTEGRITY_VERIFY))
return true;
- if (bio_data_dir(bio) == WRITE && bi->generate_fn != NULL &&
+ if (bio_data_dir(bio) == WRITE && bi->profile->generate_fn != NULL &&
(bi->flags & BLK_INTEGRITY_GENERATE))
return true;
@@ -340,7 +340,7 @@ int bio_integrity_prep(struct bio *bio)
/* Auto-generate integrity metadata if this is a write */
if (bio_data_dir(bio) == WRITE)
- bio_integrity_process(bio, bi->generate_fn);
+ bio_integrity_process(bio, bi->profile->generate_fn);
return 0;
}
@@ -361,7 +361,7 @@ static void bio_integrity_verify_fn(struct work_struct *work)
struct bio *bio = bip->bip_bio;
struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
- bio->bi_error = bio_integrity_process(bio, bi->verify_fn);
+ bio->bi_error = bio_integrity_process(bio, bi->profile->verify_fn);
/* Restore original bio completion handler */
bio->bi_end_io = bip->bip_end_io;
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 182bfd2383ea..daf590ab3b46 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -176,10 +176,10 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
return -1;
}
- if (strcmp(b1->name, b2->name)) {
+ if (b1->profile != b2->profile) {
printk(KERN_ERR "%s: %s/%s type %s != %s\n", __func__,
gd1->disk_name, gd2->disk_name,
- b1->name, b2->name);
+ b1->profile->name, b2->profile->name);
return -1;
}
@@ -275,8 +275,8 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
static ssize_t integrity_format_show(struct blk_integrity *bi, char *page)
{
- if (bi != NULL && bi->name != NULL)
- return sprintf(page, "%s\n", bi->name);
+ if (bi != NULL && bi->profile->name != NULL)
+ return sprintf(page, "%s\n", bi->profile->name);
else
return sprintf(page, "none\n");
}
@@ -401,7 +401,8 @@ bool blk_integrity_is_initialized(struct gendisk *disk)
{
struct blk_integrity *bi = blk_get_integrity(disk);
- return (bi && bi->name && strcmp(bi->name, bi_unsupported_name) != 0);
+ return (bi && bi->profile->name && strcmp(bi->profile->name,
+ bi_unsupported_name) != 0);
}
EXPORT_SYMBOL(blk_integrity_is_initialized);
@@ -446,14 +447,12 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
/* Use the provided profile as template */
if (template != NULL) {
- bi->name = template->name;
- bi->generate_fn = template->generate_fn;
- bi->verify_fn = template->verify_fn;
+ bi->profile = template->profile;
bi->tuple_size = template->tuple_size;
bi->tag_size = template->tag_size;
bi->flags |= template->flags;
} else
- bi->name = bi_unsupported_name;
+ bi->profile->name = bi_unsupported_name;
disk->queue->backing_dev_info.capabilities |= BDI_CAP_STABLE_WRITES;
diff --git a/block/t10-pi.c b/block/t10-pi.c
index 24d6e9715318..2c97912335a9 100644
--- a/block/t10-pi.c
+++ b/block/t10-pi.c
@@ -160,38 +160,30 @@ static int t10_pi_type3_verify_ip(struct blk_integrity_iter *iter)
return t10_pi_verify(iter, t10_pi_ip_fn, 3);
}
-struct blk_integrity t10_pi_type1_crc = {
+struct blk_integrity_profile t10_pi_type1_crc = {
.name = "T10-DIF-TYPE1-CRC",
.generate_fn = t10_pi_type1_generate_crc,
.verify_fn = t10_pi_type1_verify_crc,
- .tuple_size = sizeof(struct t10_pi_tuple),
- .tag_size = 0,
};
EXPORT_SYMBOL(t10_pi_type1_crc);
-struct blk_integrity t10_pi_type1_ip = {
+struct blk_integrity_profile t10_pi_type1_ip = {
.name = "T10-DIF-TYPE1-IP",
.generate_fn = t10_pi_type1_generate_ip,
.verify_fn = t10_pi_type1_verify_ip,
- .tuple_size = sizeof(struct t10_pi_tuple),
- .tag_size = 0,
};
EXPORT_SYMBOL(t10_pi_type1_ip);
-struct blk_integrity t10_pi_type3_crc = {
+struct blk_integrity_profile t10_pi_type3_crc = {
.name = "T10-DIF-TYPE3-CRC",
.generate_fn = t10_pi_type3_generate_crc,
.verify_fn = t10_pi_type3_verify_crc,
- .tuple_size = sizeof(struct t10_pi_tuple),
- .tag_size = 0,
};
EXPORT_SYMBOL(t10_pi_type3_crc);
-struct blk_integrity t10_pi_type3_ip = {
+struct blk_integrity_profile t10_pi_type3_ip = {
.name = "T10-DIF-TYPE3-IP",
.generate_fn = t10_pi_type3_generate_ip,
.verify_fn = t10_pi_type3_verify_ip,
- .tuple_size = sizeof(struct t10_pi_tuple),
- .tag_size = 0,
};
EXPORT_SYMBOL(t10_pi_type3_ip);
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 6f04771f1019..3ea1616814d8 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -555,7 +555,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,
@@ -567,14 +567,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;
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index cb62ec6a12d0..9e1b0f656a9b 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -399,19 +399,22 @@ static int nd_pi_nop_generate_verify(struct blk_integrity_iter *iter)
int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
{
- struct blk_integrity integrity = {
+ struct blk_integrity bi;
+ struct blk_integrity_profile profile = {
.name = "ND-PI-NOP",
.generate_fn = nd_pi_nop_generate_verify,
.verify_fn = nd_pi_nop_generate_verify,
- .tuple_size = meta_size,
- .tag_size = meta_size,
};
int ret;
if (meta_size == 0)
return 0;
- ret = blk_integrity_register(disk, &integrity);
+ bi.profile = &profile;
+ bi.tuple_size = meta_size;
+ bi.tag_size = meta_size;
+
+ ret = blk_integrity_register(disk, &bi);
if (ret)
return ret;
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 5c06d292b94c..987bf392c336 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -43,6 +43,7 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
struct scsi_device *sdp = sdkp->device;
struct gendisk *disk = sdkp->disk;
u8 type = sdkp->protection_type;
+ struct blk_integrity bi;
int dif, dix;
dif = scsi_host_dif_capable(sdp->host, type);
@@ -55,39 +56,43 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
if (!dix)
return;
+ memset(&bi, 0, sizeof(bi));
+
/* Enable DMA of protection information */
if (scsi_host_get_guard(sdkp->device->host) & SHOST_DIX_GUARD_IP) {
if (type == SD_DIF_TYPE3_PROTECTION)
- blk_integrity_register(disk, &t10_pi_type3_ip);
+ bi.profile = &t10_pi_type3_ip;
else
- blk_integrity_register(disk, &t10_pi_type1_ip);
+ bi.profile = &t10_pi_type1_ip;
- disk->integrity->flags |= BLK_INTEGRITY_IP_CHECKSUM;
+ bi.flags |= BLK_INTEGRITY_IP_CHECKSUM;
} else
if (type == SD_DIF_TYPE3_PROTECTION)
- blk_integrity_register(disk, &t10_pi_type3_crc);
+ bi.profile = &t10_pi_type3_crc;
else
- blk_integrity_register(disk, &t10_pi_type1_crc);
+ bi.profile = &t10_pi_type1_crc;
+ bi.tuple_size = sizeof(struct t10_pi_tuple);
sd_printk(KERN_NOTICE, sdkp,
- "Enabling DIX %s protection\n", disk->integrity->name);
+ "Enabling DIX %s protection\n", bi.profile->name);
- /* Signal to block layer that we support sector tagging */
if (dif && type) {
-
- disk->integrity->flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+ bi.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
if (!sdkp->ATO)
- return;
+ goto out;
if (type == SD_DIF_TYPE3_PROTECTION)
- disk->integrity->tag_size = sizeof(u16) + sizeof(u32);
+ bi.tag_size = sizeof(u16) + sizeof(u32);
else
- disk->integrity->tag_size = sizeof(u16);
+ bi.tag_size = sizeof(u16);
sd_printk(KERN_NOTICE, sdkp, "DIF application tag size %u\n",
- disk->integrity->tag_size);
+ bi.tag_size);
}
+
+out:
+ blk_integrity_register(disk, &bi);
}
/*
diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index 0f19e11acac2..f29c69120054 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -155,17 +155,17 @@ static int iblock_configure_device(struct se_device *dev)
if (bi) {
struct bio_set *bs = ib_dev->ibd_bio_set;
- if (!strcmp(bi->name, "T10-DIF-TYPE3-IP") ||
- !strcmp(bi->name, "T10-DIF-TYPE1-IP")) {
+ if (!strcmp(bi->profile->name, "T10-DIF-TYPE3-IP") ||
+ !strcmp(bi->profile->name, "T10-DIF-TYPE1-IP")) {
pr_err("IBLOCK export of blk_integrity: %s not"
- " supported\n", bi->name);
+ " supported\n", bi->profile->name);
ret = -ENOSYS;
goto out_blkdev_put;
}
- if (!strcmp(bi->name, "T10-DIF-TYPE3-CRC")) {
+ if (!strcmp(bi->profile->name, "T10-DIF-TYPE3-CRC")) {
dev->dev_attrib.pi_prot_type = TARGET_DIF_TYPE3_PROT;
- } else if (!strcmp(bi->name, "T10-DIF-TYPE1-CRC")) {
+ } else if (!strcmp(bi->profile->name, "T10-DIF-TYPE1-CRC")) {
dev->dev_attrib.pi_prot_type = TARGET_DIF_TYPE1_PROT;
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 830f9c07d4bb..f36c6476f1c7 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1462,16 +1462,18 @@ struct blk_integrity_iter {
typedef int (integrity_processing_fn) (struct blk_integrity_iter *);
-struct blk_integrity {
- integrity_processing_fn *generate_fn;
- integrity_processing_fn *verify_fn;
-
- unsigned short flags;
- unsigned short tuple_size;
- unsigned short interval;
- unsigned short tag_size;
+struct blk_integrity_profile {
+ integrity_processing_fn *generate_fn;
+ integrity_processing_fn *verify_fn;
+ const char *name;
+};
- const char *name;
+struct blk_integrity {
+ struct blk_integrity_profile *profile;
+ unsigned short flags;
+ unsigned short tuple_size;
+ unsigned short interval;
+ unsigned short tag_size;
};
extern bool blk_integrity_is_initialized(struct gendisk *);
diff --git a/include/linux/t10-pi.h b/include/linux/t10-pi.h
index 6a8b9942632d..dd8de82cf5b5 100644
--- a/include/linux/t10-pi.h
+++ b/include/linux/t10-pi.h
@@ -14,9 +14,9 @@ struct t10_pi_tuple {
};
-extern struct blk_integrity t10_pi_type1_crc;
-extern struct blk_integrity t10_pi_type1_ip;
-extern struct blk_integrity t10_pi_type3_crc;
-extern struct blk_integrity t10_pi_type3_ip;
+extern struct blk_integrity_profile t10_pi_type1_crc;
+extern struct blk_integrity_profile t10_pi_type1_ip;
+extern struct blk_integrity_profile t10_pi_type3_crc;
+extern struct blk_integrity_profile t10_pi_type3_ip;
#endif
--
2.4.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 2/5] block: Consolidate static integrity profile properties
2015-10-20 2:45 ` [PATCH 2/5] block: Consolidate static integrity profile properties Martin K. Petersen
@ 2015-10-20 18:20 ` Dan Williams
0 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-20 18:20 UTC (permalink / raw)
On Mon, Oct 19, 2015 at 7:45 PM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
> 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 <martin.petersen at oracle.com>
> Reported-by: Christoph Hellwig <hch at lst.de>
> Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
> Cc: Dan Williams <dan.j.williams at intel.com>
> ---
[..]
> diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
> index cb62ec6a12d0..9e1b0f656a9b 100644
> --- a/drivers/nvdimm/core.c
> +++ b/drivers/nvdimm/core.c
> @@ -399,19 +399,22 @@ static int nd_pi_nop_generate_verify(struct blk_integrity_iter *iter)
>
> int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
> {
> - struct blk_integrity integrity = {
> + struct blk_integrity bi;
> + struct blk_integrity_profile profile = {
Oops, this version is missing the fix to make this static, I'll fold
that in for my local series and refresh
nvdimm.git/for-4.4/blk-intetgrity.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 3/5] block: Reduce the size of struct blk_integrity
2015-10-20 2:45 ` Simplify block integrity registration v2 Martin K. Petersen
2015-10-20 2:45 ` [PATCH 1/5] block: Move integrity kobject to struct gendisk Martin K. Petersen
2015-10-20 2:45 ` [PATCH 2/5] block: Consolidate static integrity profile properties Martin K. Petersen
@ 2015-10-20 2:45 ` Martin K. Petersen
2015-10-20 2:45 ` [PATCH 4/5] block: Export integrity data interval size in sysfs Martin K. Petersen
` (2 subsequent siblings)
5 siblings, 0 replies; 33+ messages in thread
From: Martin K. Petersen @ 2015-10-20 2:45 UTC (permalink / raw)
The per-device properties in the blk_integrity structure were previously
unsigned short. However, most of the values fit inside a char. The only
exception is the data interval size and we can work around that by
storing it as a power of two.
This cuts the size of the dynamic portion of blk_integrity in half.
Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
Reported-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
---
block/bio-integrity.c | 4 ++--
block/blk-integrity.c | 6 +++---
include/linux/blkdev.h | 8 ++++----
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index a10ffe19a8dd..6a90eca9cea1 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -202,7 +202,7 @@ EXPORT_SYMBOL(bio_integrity_enabled);
static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
unsigned int sectors)
{
- return sectors >> (ilog2(bi->interval) - 9);
+ return sectors >> (bi->interval_exp - 9);
}
static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
@@ -229,7 +229,7 @@ static int bio_integrity_process(struct bio *bio,
bip->bip_vec->bv_offset;
iter.disk_name = bio->bi_bdev->bd_disk->disk_name;
- iter.interval = bi->interval;
+ iter.interval = 1 << bi->interval_exp;
iter.seed = bip_get_seed(bip);
iter.prot_buf = prot_buf;
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index daf590ab3b46..c7508654faff 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -155,10 +155,10 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
if (!b1 || !b2)
return -1;
- if (b1->interval != b2->interval) {
+ if (b1->interval_exp != b2->interval_exp) {
pr_err("%s: %s/%s protection interval %u != %u\n",
__func__, gd1->disk_name, gd2->disk_name,
- b1->interval, b2->interval);
+ 1 << b1->interval_exp, 1 << b2->interval_exp);
return -1;
}
@@ -440,7 +440,7 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
bi->flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
- bi->interval = queue_logical_block_size(disk->queue);
+ bi->interval_exp = ilog2(queue_logical_block_size(disk->queue));
disk->integrity = bi;
} else
bi = disk->integrity;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index f36c6476f1c7..4f1968f15e30 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1470,10 +1470,10 @@ struct blk_integrity_profile {
struct blk_integrity {
struct blk_integrity_profile *profile;
- unsigned short flags;
- unsigned short tuple_size;
- unsigned short interval;
- unsigned short tag_size;
+ unsigned char flags;
+ unsigned char tuple_size;
+ unsigned char interval_exp;
+ unsigned char tag_size;
};
extern bool blk_integrity_is_initialized(struct gendisk *);
--
2.4.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 4/5] block: Export integrity data interval size in sysfs
2015-10-20 2:45 ` Simplify block integrity registration v2 Martin K. Petersen
` (2 preceding siblings ...)
2015-10-20 2:45 ` [PATCH 3/5] block: Reduce the size of struct blk_integrity Martin K. Petersen
@ 2015-10-20 2:45 ` Martin K. Petersen
2015-10-20 2:45 ` [PATCH 5/5] block: Inline blk_integrity in struct gendisk Martin K. Petersen
2015-10-21 7:22 ` Simplify block integrity registration v2 Christoph Hellwig
5 siblings, 0 replies; 33+ messages in thread
From: Martin K. Petersen @ 2015-10-20 2:45 UTC (permalink / raw)
The size of the data interval was not exported in the sysfs integrity
directory. Export it so that userland apps can tell whether the interval
is different from the device's logical block size.
Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
---
Documentation/ABI/testing/sysfs-block | 7 +++++++
block/blk-integrity.c | 14 ++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index 8df003963d99..71d184dbb70d 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -60,6 +60,13 @@ Description:
Indicates whether a storage device is capable of storing
integrity metadata. Set if the device is T10 PI-capable.
+What: /sys/block/<disk>/integrity/protection_interval_bytes
+Date: July 2015
+Contact: Martin K. Petersen <martin.petersen at oracle.com>
+Description:
+ Describes the number of data bytes which are protected
+ by one integrity tuple. Typically the device's logical
+ block size.
What: /sys/block/<disk>/integrity/write_generate
Date: June 2008
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index c7508654faff..7a96f57ed195 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -289,6 +289,14 @@ static ssize_t integrity_tag_size_show(struct blk_integrity *bi, char *page)
return sprintf(page, "0\n");
}
+static ssize_t integrity_interval_show(struct blk_integrity *bi, char *page)
+{
+ if (bi != NULL)
+ return sprintf(page, "%u\n", 1 << bi->interval_exp);
+ else
+ return sprintf(page, "0\n");
+}
+
static ssize_t integrity_verify_store(struct blk_integrity *bi,
const char *page, size_t count)
{
@@ -343,6 +351,11 @@ static struct integrity_sysfs_entry integrity_tag_size_entry = {
.show = integrity_tag_size_show,
};
+static struct integrity_sysfs_entry integrity_interval_entry = {
+ .attr = { .name = "protection_interval_bytes", .mode = S_IRUGO },
+ .show = integrity_interval_show,
+};
+
static struct integrity_sysfs_entry integrity_verify_entry = {
.attr = { .name = "read_verify", .mode = S_IRUGO | S_IWUSR },
.show = integrity_verify_show,
@@ -363,6 +376,7 @@ static struct integrity_sysfs_entry integrity_device_entry = {
static struct attribute *integrity_attrs[] = {
&integrity_format_entry.attr,
&integrity_tag_size_entry.attr,
+ &integrity_interval_entry.attr,
&integrity_verify_entry.attr,
&integrity_generate_entry.attr,
&integrity_device_entry.attr,
--
2.4.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 5/5] block: Inline blk_integrity in struct gendisk
2015-10-20 2:45 ` Simplify block integrity registration v2 Martin K. Petersen
` (3 preceding siblings ...)
2015-10-20 2:45 ` [PATCH 4/5] block: Export integrity data interval size in sysfs Martin K. Petersen
@ 2015-10-20 2:45 ` Martin K. Petersen
2015-10-21 7:22 ` Simplify block integrity registration v2 Christoph Hellwig
5 siblings, 0 replies; 33+ messages in thread
From: Martin K. Petersen @ 2015-10-20 2:45 UTC (permalink / raw)
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 <martin.petersen at oracle.com>
Reported-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Sagi Grimberg <sagig at mellanox.com>
Signed-off-by: Mike Snitzer <snitzer at redhat.com>
Cc: Dan Williams <dan.j.williams at intel.com>
---
block/blk-integrity.c | 160 +++++++++++++++++-----------------------------
block/genhd.c | 2 +
block/partition-generic.c | 1 +
drivers/block/nvme-core.c | 5 +-
drivers/md/dm-table.c | 88 +++++++++++++------------
drivers/md/md.c | 9 +--
drivers/nvdimm/core.c | 6 +-
fs/block_dev.c | 2 +-
include/linux/blkdev.h | 34 ++++------
include/linux/genhd.h | 26 +++++++-
10 files changed, 152 insertions(+), 181 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 7a96f57ed195..4615a3386798 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -30,10 +30,6 @@
#include "blk.h"
-static struct kmem_cache *integrity_cachep;
-
-static const char *bi_unsupported_name = "unsupported";
-
/**
* blk_rq_count_integrity_sg - Count number of integrity scatterlist elements
* @q: request queue
@@ -146,13 +142,13 @@ EXPORT_SYMBOL(blk_rq_map_integrity_sg);
*/
int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
{
- struct blk_integrity *b1 = gd1->integrity;
- struct blk_integrity *b2 = gd2->integrity;
+ struct blk_integrity *b1 = &gd1->integrity;
+ struct blk_integrity *b2 = &gd2->integrity;
- if (!b1 && !b2)
+ if (!b1->profile && !b2->profile)
return 0;
- if (!b1 || !b2)
+ if (!b1->profile || !b2->profile)
return -1;
if (b1->interval_exp != b2->interval_exp) {
@@ -163,21 +159,21 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
}
if (b1->tuple_size != b2->tuple_size) {
- printk(KERN_ERR "%s: %s/%s tuple sz %u != %u\n", __func__,
+ pr_err("%s: %s/%s tuple sz %u != %u\n", __func__,
gd1->disk_name, gd2->disk_name,
b1->tuple_size, b2->tuple_size);
return -1;
}
if (b1->tag_size && b2->tag_size && (b1->tag_size != b2->tag_size)) {
- printk(KERN_ERR "%s: %s/%s tag sz %u != %u\n", __func__,
+ pr_err("%s: %s/%s tag sz %u != %u\n", __func__,
gd1->disk_name, gd2->disk_name,
b1->tag_size, b2->tag_size);
return -1;
}
if (b1->profile != b2->profile) {
- printk(KERN_ERR "%s: %s/%s type %s != %s\n", __func__,
+ pr_err("%s: %s/%s type %s != %s\n", __func__,
gd1->disk_name, gd2->disk_name,
b1->profile->name, b2->profile->name);
return -1;
@@ -250,7 +246,7 @@ static ssize_t integrity_attr_show(struct kobject *kobj, struct attribute *attr,
char *page)
{
struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
- struct blk_integrity *bi = blk_get_integrity(disk);
+ struct blk_integrity *bi = &disk->integrity;
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
@@ -262,7 +258,7 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
size_t count)
{
struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
- struct blk_integrity *bi = blk_get_integrity(disk);
+ struct blk_integrity *bi = &disk->integrity;
struct integrity_sysfs_entry *entry =
container_of(attr, struct integrity_sysfs_entry, attr);
ssize_t ret = 0;
@@ -275,7 +271,7 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
static ssize_t integrity_format_show(struct blk_integrity *bi, char *page)
{
- if (bi != NULL && bi->profile->name != NULL)
+ if (bi->profile && bi->profile->name)
return sprintf(page, "%s\n", bi->profile->name);
else
return sprintf(page, "none\n");
@@ -283,18 +279,13 @@ static ssize_t integrity_format_show(struct blk_integrity *bi, char *page)
static ssize_t integrity_tag_size_show(struct blk_integrity *bi, char *page)
{
- if (bi != NULL)
- return sprintf(page, "%u\n", bi->tag_size);
- else
- return sprintf(page, "0\n");
+ return sprintf(page, "%u\n", bi->tag_size);
}
static ssize_t integrity_interval_show(struct blk_integrity *bi, char *page)
{
- if (bi != NULL)
- return sprintf(page, "%u\n", 1 << bi->interval_exp);
- else
- return sprintf(page, "0\n");
+ return sprintf(page, "%u\n",
+ bi->interval_exp ? 1 << bi->interval_exp : 0);
}
static ssize_t integrity_verify_store(struct blk_integrity *bi,
@@ -388,113 +379,78 @@ static const struct sysfs_ops integrity_ops = {
.store = &integrity_attr_store,
};
-static int __init blk_dev_integrity_init(void)
-{
- integrity_cachep = kmem_cache_create("blkdev_integrity",
- sizeof(struct blk_integrity),
- 0, SLAB_PANIC, NULL);
- return 0;
-}
-subsys_initcall(blk_dev_integrity_init);
-
-static void blk_integrity_release(struct kobject *kobj)
-{
- struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
- struct blk_integrity *bi = blk_get_integrity(disk);
-
- kmem_cache_free(integrity_cachep, bi);
-}
-
static struct kobj_type integrity_ktype = {
.default_attrs = integrity_attrs,
.sysfs_ops = &integrity_ops,
- .release = blk_integrity_release,
};
-bool blk_integrity_is_initialized(struct gendisk *disk)
-{
- struct blk_integrity *bi = blk_get_integrity(disk);
-
- return (bi && bi->profile->name && strcmp(bi->profile->name,
- bi_unsupported_name) != 0);
-}
-EXPORT_SYMBOL(blk_integrity_is_initialized);
-
/**
* blk_integrity_register - Register a gendisk as being integrity-capable
* @disk: struct gendisk pointer to make integrity-aware
- * @template: optional integrity profile to register
+ * @template: block integrity profile to register
*
- * Description: When a device needs to advertise itself as being able
- * to send/receive integrity metadata it must use this function to
- * register the capability with the block layer. The template is a
- * blk_integrity struct with values appropriate for the underlying
- * hardware. If template is NULL the new profile is allocated but
- * not filled out. See Documentation/block/data-integrity.txt.
+ * Description: When a device needs to advertise itself as being able to
+ * send/receive integrity metadata it must use this function to register
+ * the capability with the block layer. The template is a blk_integrity
+ * struct with values appropriate for the underlying hardware. See
+ * Documentation/block/data-integrity.txt.
*/
-int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
+void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
{
- struct blk_integrity *bi;
-
- BUG_ON(disk == NULL);
+ struct blk_integrity *bi = &disk->integrity;
- if (disk->integrity == NULL) {
- bi = kmem_cache_alloc(integrity_cachep,
- GFP_KERNEL | __GFP_ZERO);
- if (!bi)
- return -1;
+ bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
+ template->flags;
+ bi->interval_exp = ilog2(queue_logical_block_size(disk->queue));
+ bi->profile = template->profile;
+ bi->tuple_size = template->tuple_size;
+ bi->tag_size = template->tag_size;
- if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
- &disk_to_dev(disk)->kobj,
- "%s", "integrity")) {
- kmem_cache_free(integrity_cachep, bi);
- return -1;
- }
-
- kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
-
- bi->flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
- bi->interval_exp = ilog2(queue_logical_block_size(disk->queue));
- disk->integrity = bi;
- } else
- bi = disk->integrity;
-
- /* Use the provided profile as template */
- if (template != NULL) {
- bi->profile = template->profile;
- bi->tuple_size = template->tuple_size;
- bi->tag_size = template->tag_size;
- bi->flags |= template->flags;
- } else
- bi->profile->name = bi_unsupported_name;
-
- disk->queue->backing_dev_info.capabilities |= BDI_CAP_STABLE_WRITES;
-
- return 0;
+ blk_integrity_revalidate(disk);
}
EXPORT_SYMBOL(blk_integrity_register);
/**
- * blk_integrity_unregister - Remove block integrity profile
- * @disk: disk whose integrity profile to deallocate
+ * blk_integrity_unregister - Unregister block integrity profile
+ * @disk: disk whose integrity profile to unregister
*
- * Description: This function frees all memory used by the block
- * integrity profile. To be called at device teardown.
+ * Description: This function unregisters the integrity capability from
+ * a block device.
*/
void blk_integrity_unregister(struct gendisk *disk)
{
- struct blk_integrity *bi;
+ blk_integrity_revalidate(disk);
+ memset(&disk->integrity, 0, sizeof(struct blk_integrity));
+}
+EXPORT_SYMBOL(blk_integrity_unregister);
- if (!disk || !disk->integrity)
+void blk_integrity_revalidate(struct gendisk *disk)
+{
+ struct blk_integrity *bi = &disk->integrity;
+
+ if (!(disk->flags & GENHD_FL_UP))
return;
- disk->queue->backing_dev_info.capabilities &= ~BDI_CAP_STABLE_WRITES;
+ if (bi->profile)
+ disk->queue->backing_dev_info.capabilities |=
+ BDI_CAP_STABLE_WRITES;
+ else
+ disk->queue->backing_dev_info.capabilities &=
+ ~BDI_CAP_STABLE_WRITES;
+}
- bi = disk->integrity;
+void blk_integrity_add(struct gendisk *disk)
+{
+ if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
+ &disk_to_dev(disk)->kobj, "%s", "integrity"))
+ return;
+ kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
+}
+
+void blk_integrity_del(struct gendisk *disk)
+{
kobject_uevent(&disk->integrity_kobj, KOBJ_REMOVE);
kobject_del(&disk->integrity_kobj);
kobject_put(&disk->integrity_kobj);
- disk->integrity = NULL;
}
-EXPORT_SYMBOL(blk_integrity_unregister);
diff --git a/block/genhd.c b/block/genhd.c
index 0c706f33a599..e5cafa51567c 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -630,6 +630,7 @@ void add_disk(struct gendisk *disk)
WARN_ON(retval);
disk_add_events(disk);
+ blk_integrity_add(disk);
}
EXPORT_SYMBOL(add_disk);
@@ -638,6 +639,7 @@ void del_gendisk(struct gendisk *disk)
struct disk_part_iter piter;
struct hd_struct *part;
+ blk_integrity_del(disk);
disk_del_events(disk);
/* invalidate stuff */
diff --git a/block/partition-generic.c b/block/partition-generic.c
index e7711133284e..3b030157ec85 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -428,6 +428,7 @@ rescan:
if (disk->fops->revalidate_disk)
disk->fops->revalidate_disk(disk);
+ blk_integrity_revalidate(disk);
check_disk_size_change(disk, bdev);
bdev->bd_invalidated = 0;
if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 3ea1616814d8..01197f0d3c01 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -535,7 +535,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;
@@ -2034,8 +2034,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))
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",
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index 9e1b0f656a9b..9b6ac57c6e73 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -405,7 +405,6 @@ int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
.generate_fn = nd_pi_nop_generate_verify,
.verify_fn = nd_pi_nop_generate_verify,
};
- int ret;
if (meta_size == 0)
return 0;
@@ -414,10 +413,7 @@ int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
bi.tuple_size = meta_size;
bi.tag_size = meta_size;
- ret = blk_integrity_register(disk, &bi);
- if (ret)
- return ret;
-
+ blk_integrity_register(disk, &bi);
blk_queue_max_integrity_segments(disk->queue, 1);
return 0;
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 073bb57adab1..0a793c7930eb 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1075,7 +1075,7 @@ int revalidate_disk(struct gendisk *disk)
if (disk->fops->revalidate_disk)
ret = disk->fops->revalidate_disk(disk);
-
+ blk_integrity_revalidate(disk);
bdev = bdget_disk(disk, 0);
if (!bdev)
return ret;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4f1968f15e30..60669c20190f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1468,16 +1468,7 @@ struct blk_integrity_profile {
const char *name;
};
-struct blk_integrity {
- struct blk_integrity_profile *profile;
- unsigned char flags;
- unsigned char tuple_size;
- unsigned char interval_exp;
- unsigned char tag_size;
-};
-
-extern bool blk_integrity_is_initialized(struct gendisk *);
-extern int blk_integrity_register(struct gendisk *, struct blk_integrity *);
+extern void blk_integrity_register(struct gendisk *, struct blk_integrity *);
extern void blk_integrity_unregister(struct gendisk *);
extern int blk_integrity_compare(struct gendisk *, struct gendisk *);
extern int blk_rq_map_integrity_sg(struct request_queue *, struct bio *,
@@ -1488,15 +1479,20 @@ extern bool blk_integrity_merge_rq(struct request_queue *, struct request *,
extern bool blk_integrity_merge_bio(struct request_queue *, struct request *,
struct bio *);
-static inline
-struct blk_integrity *bdev_get_integrity(struct block_device *bdev)
+static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
{
- return bdev->bd_disk->integrity;
+ struct blk_integrity *bi = &disk->integrity;
+
+ if (!bi->profile)
+ return NULL;
+
+ return bi;
}
-static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
+static inline
+struct blk_integrity *bdev_get_integrity(struct block_device *bdev)
{
- return disk->integrity;
+ return blk_get_integrity(bdev->bd_disk);
}
static inline bool blk_integrity_rq(struct request *rq)
@@ -1570,10 +1566,9 @@ static inline int blk_integrity_compare(struct gendisk *a, struct gendisk *b)
{
return 0;
}
-static inline int blk_integrity_register(struct gendisk *d,
+static inline void blk_integrity_register(struct gendisk *d,
struct blk_integrity *b)
{
- return 0;
}
static inline void blk_integrity_unregister(struct gendisk *d)
{
@@ -1598,10 +1593,7 @@ static inline bool blk_integrity_merge_bio(struct request_queue *rq,
{
return true;
}
-static inline bool blk_integrity_is_initialized(struct gendisk *g)
-{
- return 0;
-}
+
static inline bool integrity_req_gap_back_merge(struct request *req,
struct bio *next)
{
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 9e6e0dfa97ad..82f4911e0ad8 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -163,6 +163,18 @@ struct disk_part_tbl {
struct disk_events;
+#if defined(CONFIG_BLK_DEV_INTEGRITY)
+
+struct blk_integrity {
+ struct blk_integrity_profile *profile;
+ unsigned char flags;
+ unsigned char tuple_size;
+ unsigned char interval_exp;
+ unsigned char tag_size;
+};
+
+#endif /* CONFIG_BLK_DEV_INTEGRITY */
+
struct gendisk {
/* major, first_minor and minors are input parameters only,
* don't use directly. Use disk_devt() and disk_max_parts().
@@ -198,9 +210,9 @@ struct gendisk {
atomic_t sync_io; /* RAID */
struct disk_events *ev;
#ifdef CONFIG_BLK_DEV_INTEGRITY
- struct blk_integrity *integrity;
+ struct blk_integrity integrity;
struct kobject integrity_kobj;
-#endif
+#endif /* CONFIG_BLK_DEV_INTEGRITY */
int node_id;
};
@@ -728,6 +740,16 @@ static inline void part_nr_sects_write(struct hd_struct *part, sector_t size)
#endif
}
+#if defined(CONFIG_BLK_DEV_INTEGRITY)
+extern void blk_integrity_add(struct gendisk *);
+extern void blk_integrity_del(struct gendisk *);
+extern void blk_integrity_revalidate(struct gendisk *);
+#else /* CONFIG_BLK_DEV_INTEGRITY */
+static inline void blk_integrity_add(struct gendisk *disk) { }
+static inline void blk_integrity_del(struct gendisk *disk) { }
+static inline void blk_integrity_revalidate(struct gendisk *disk) { }
+#endif /* CONFIG_BLK_DEV_INTEGRITY */
+
#else /* CONFIG_BLOCK */
static inline void printk_all_partitions(void) { }
--
2.4.3
^ permalink raw reply related [flat|nested] 33+ messages in thread* Simplify block integrity registration v2
2015-10-20 2:45 ` Simplify block integrity registration v2 Martin K. Petersen
` (4 preceding siblings ...)
2015-10-20 2:45 ` [PATCH 5/5] block: Inline blk_integrity in struct gendisk Martin K. Petersen
@ 2015-10-21 7:22 ` Christoph Hellwig
2015-10-21 15:45 ` Dan Williams
5 siblings, 1 reply; 33+ messages in thread
From: Christoph Hellwig @ 2015-10-21 7:22 UTC (permalink / raw)
On Mon, Oct 19, 2015@10:45:27PM -0400, Martin K. Petersen wrote:
> Changes since last version:
>
> - Patch 2/5: Zero template to prevent problems on SCSI devices w/ ATO=0
> - Patch 5/5: Add Mike's SoB
Can you post a series that also includes Dan series so that we have one
coherent batch of both?
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v2 11/12] block: blk_flush_integrity() for bio-based drivers
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
` (9 preceding siblings ...)
2015-10-15 20:00 ` [PATCH v2 10/12] block: move blk_integrity to request_queue Dan Williams
@ 2015-10-15 20:00 ` Dan Williams
2015-10-15 20:00 ` [PATCH v2 12/12] block, libnvdimm, nvme: provide a built-in blk_integrity nop profile Dan Williams
2015-10-15 23:53 ` [PATCH v2 00/12] blk-integrity lifetime fixes Martin K. Petersen
12 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-15 20:00 UTC (permalink / raw)
Since they lack requests to pin the request_queue active, synchronous
bio-based drivers may have in-flight integrity work from
bio_integrity_endio() that is not flushed by blk_freeze_queue(). Flush
that work to prevent races to free the queue and the final usage of the
blk_integrity profile.
This is temporary unless/until bio-based drivers start to generically
take a q_usage_counter reference while a bio is in-flight.
Cc: Martin K. Petersen <martin.petersen at oracle.com>
[martin: fix the CONFIG_BLK_DEV_INTEGRITY=n case]
Tested-by: Ross Zwisler <ross.zwisler at linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
block/bio-integrity.c | 5 +++++
block/blk-core.c | 3 +++
block/blk.h | 8 ++++++++
3 files changed, 16 insertions(+)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 6a90eca9cea1..f6325d573c10 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -32,6 +32,11 @@
static struct kmem_cache *bip_slab;
static struct workqueue_struct *kintegrityd_wq;
+void blk_flush_integrity(void)
+{
+ flush_workqueue(kintegrityd_wq);
+}
+
/**
* bio_integrity_alloc - Allocate integrity payload and attach it to bio
* @bio: bio to attach integrity metadata to
diff --git a/block/blk-core.c b/block/blk-core.c
index 9b4d735cb5b8..6ebe33ed5154 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -561,6 +561,9 @@ void blk_cleanup_queue(struct request_queue *q)
queue_flag_set(QUEUE_FLAG_DEAD, q);
spin_unlock_irq(lock);
+ /* for synchronous bio-based driver finish in-flight integrity i/o */
+ blk_flush_integrity();
+
/* @q won't process any more request, flush async actions */
del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer);
blk_sync_queue(q);
diff --git a/block/blk.h b/block/blk.h
index 5b2cd393afbe..157c93d54dc9 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -87,6 +87,14 @@ static inline void blk_queue_enter_live(struct request_queue *q)
percpu_ref_get(&q->q_usage_counter);
}
+#ifdef CONFIG_BLK_DEV_INTEGRITY
+void blk_flush_integrity(void);
+#else
+static inline void blk_flush_integrity(void)
+{
+}
+#endif
+
void blk_rq_timed_out_timer(unsigned long data);
unsigned long blk_rq_timeout(unsigned long timeout);
void blk_add_timer(struct request *req);
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 12/12] block, libnvdimm, nvme: provide a built-in blk_integrity nop profile
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
` (10 preceding siblings ...)
2015-10-15 20:00 ` [PATCH v2 11/12] block: blk_flush_integrity() for bio-based drivers Dan Williams
@ 2015-10-15 20:00 ` Dan Williams
2015-10-15 23:53 ` [PATCH v2 00/12] blk-integrity lifetime fixes Martin K. Petersen
12 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-15 20:00 UTC (permalink / raw)
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 <keith.busch at intel.com>
Cc: Matthew Wilcox <willy at linux.intel.com>
Suggested-by: Christoph Hellwig <hch at lst.de>
[hch: kill the local nvme nop profile as well]
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
block/blk-integrity.c | 13 ++++++++++++-
drivers/nvdimm/core.c | 12 +-----------
drivers/nvme/host/pci.c | 18 +-----------------
3 files changed, 14 insertions(+), 29 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index dc4dea7b8a93..506cc16c1a92 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -384,6 +384,17 @@ static struct kobj_type integrity_ktype = {
.sysfs_ops = &integrity_ops,
};
+static int blk_integrity_nop_fn(struct blk_integrity_iter *iter)
+{
+ return 0;
+}
+
+static struct blk_integrity_profile nop_profile = {
+ .name = "nop",
+ .generate_fn = blk_integrity_nop_fn,
+ .verify_fn = blk_integrity_nop_fn,
+};
+
/**
* blk_integrity_register - Register a request_queue as being integrity-capable
* @disk: struct request_queue pointer to make integrity-aware
@@ -402,7 +413,7 @@ void blk_integrity_register(struct request_queue *q, struct blk_integrity *templ
bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
template->flags;
bi->interval_exp = ilog2(queue_logical_block_size(q));
- bi->profile = template->profile;
+ bi->profile = template->profile ? template->profile : &nop_profile;
bi->tuple_size = template->tuple_size;
bi->tag_size = template->tag_size;
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index eeedd58bbcad..2ed3c934256f 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -392,24 +392,14 @@ void nvdimm_bus_unregister(struct nvdimm_bus *nvdimm_bus)
EXPORT_SYMBOL_GPL(nvdimm_bus_unregister);
#ifdef CONFIG_BLK_DEV_INTEGRITY
-static int nd_pi_nop_generate_verify(struct blk_integrity_iter *iter)
-{
- return 0;
-}
-
int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
{
struct blk_integrity bi;
- static struct blk_integrity_profile profile = {
- .name = "ND-PI-NOP",
- .generate_fn = nd_pi_nop_generate_verify,
- .verify_fn = nd_pi_nop_generate_verify,
- };
if (meta_size == 0)
return 0;
- bi.profile = &profile;
+ bi.profile = NULL;
bi.tuple_size = meta_size;
bi.tag_size = meta_size;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index e4a0cc7fb421..532b6a491fca 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -550,22 +550,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;
@@ -579,7 +563,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;
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 00/12] blk-integrity lifetime fixes
2015-10-15 19:59 [PATCH v2 00/12] blk-integrity lifetime fixes Dan Williams
` (11 preceding siblings ...)
2015-10-15 20:00 ` [PATCH v2 12/12] block, libnvdimm, nvme: provide a built-in blk_integrity nop profile Dan Williams
@ 2015-10-15 23:53 ` Martin K. Petersen
2015-10-16 0:06 ` Dan Williams
12 siblings, 1 reply; 33+ messages in thread
From: Martin K. Petersen @ 2015-10-15 23:53 UTC (permalink / raw)
>>>>> "Dan" == Dan Williams <dan.j.williams at intel.com> writes:
Dan,
Dan> 5/ Fixed up Documentation/ABI/testing/sysfs-block to reflect moving
Dan> "integrity" sysfs attributes from "disk" to "disk/queue".
You can't change an exported interface. Several apps are depending on
it.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH v2 00/12] blk-integrity lifetime fixes
2015-10-15 23:53 ` [PATCH v2 00/12] blk-integrity lifetime fixes Martin K. Petersen
@ 2015-10-16 0:06 ` Dan Williams
0 siblings, 0 replies; 33+ messages in thread
From: Dan Williams @ 2015-10-16 0:06 UTC (permalink / raw)
On Thu, Oct 15, 2015 at 4:53 PM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>>>>>> "Dan" == Dan Williams <dan.j.williams at intel.com> writes:
>
> Dan,
>
> Dan> 5/ Fixed up Documentation/ABI/testing/sysfs-block to reflect moving
> Dan> "integrity" sysfs attributes from "disk" to "disk/queue".
>
> You can't change an exported interface. Several apps are depending on
> it.
Hmm, add an "integrity" sysfs link for backwards compatibility, or
leave it as a child of the gendisk kobject? Probably the latter
because it seems fine if the integrity_kobj has follow the gendisk
lifetime even if the struct blk_integrity info follows the
request_queue.
^ permalink raw reply [flat|nested] 33+ messages in thread