Simplify block integrity registration v3
Subsystems (10)
v3 cover Simplify block integrity registration v3
Changes since v2 [1]:
1/ Fold in static declaration fix to "block: Consolidate static
integrity profile properties"
2/ Rebase on latest for-4.4/drivers from block.git
3/ Merge the pending lifetime fixes
[1]: http://lists.infradead.org/pipermail/linux-nvme/2015-October/002783.html
---
Dan Williams (7):
md, dm, scsi, nvme, libnvdimm: drop blk_integrity_unregister() at shutdown
md: suspend i/o during runtime blk_integrity_unregister
nvme: suspend i/o during runtime blk_integrity_unregister
block: generic request_queue reference counting
block: move blk_integrity to request_queue
block: blk_flush_integrity() for bio-based drivers
block, libnvdimm, nvme: provide a built-in blk_integrity nop profile
Martin K. Petersen (5):
block: Move integrity kobject to struct gendisk
block: Consolidate static integrity profile properties
block: Reduce the size of struct blk_integrity
block: Export integrity data interval size in sysfs
block: Inline blk_integrity in struct gendisk
Documentation/ABI/testing/sysfs-block | 7 + block/bio-integrity.c | 17 ++- block/blk-core.c | 74 +++++++++++-- block/blk-integrity.c | 192 +++++++++++++++------------------ block/blk-mq-sysfs.c | 6 - block/blk-mq.c | 80 ++++---------- block/blk-sysfs.c | 3 - block/blk.h | 22 ++++ block/genhd.c | 2 block/partition-generic.c | 1 block/t10-pi.c | 16 +-- drivers/md/dm-table.c | 88 ++++++++------- drivers/md/dm.c | 2 drivers/md/md.c | 11 +- drivers/md/multipath.c | 2 drivers/md/raid1.c | 2 drivers/md/raid10.c | 2 drivers/nvdimm/btt.c | 1 drivers/nvdimm/core.c | 21 +--- drivers/nvme/host/pci.c | 34 +----- drivers/scsi/sd.c | 1 drivers/scsi/sd_dif.c | 31 +++-- drivers/target/target_core_iblock.c | 10 +- fs/block_dev.c | 2 include/linux/blk-mq.h | 1 include/linux/blkdev.h | 48 ++++---- include/linux/genhd.h | 26 ++++ include/linux/t10-pi.h | 8 + 28 files changed, 367 insertions(+), 343 deletions(-)
PATCH v3 1/12 block: Move integrity kobject to struct gendisk
From: Martin K. Petersen <[email protected]>
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.
--- block/blk-integrity.c | 22 +++++++++++----------- include/linux/blkdev.h | 2 -- include/linux/genhd.h | 1 + 3 files changed, 12 insertions(+), 13 deletions(-)
block/blk-integrity.c +11 -11
@@ -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);
include/linux/blkdev.h +0 -2
@@ -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 *);
include/linux/genhd.h +1 -0
@@ -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; };
PATCH v3 2/12 block: Consolidate static integrity profile properties
In reply to Dan WilliamsFrom: Martin K. Petersen <[email protected]>
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.
--- 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 | 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(-)
block/bio-integrity.c +4 -4
@@ -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;
block/blk-integrity.c +8 -9
@@ -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 ;
block/t10-pi.c +4 -12
@@ -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);
drivers/nvdimm/core.c +7 -4
@@ -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;
drivers/nvme/host/pci.c +4 -4
@@ -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;
drivers/scsi/sd_dif.c +18 -13
@@ -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); }/*
drivers/target/target_core_iblock.c +5 -5
@@ -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 ; }
include/linux/blkdev.h +11 -9
@@ -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 *);
include/linux/t10-pi.h +4 -4
@@ -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
PATCH v3 3/12 block: Reduce the size of struct blk_integrity
In reply to Dan WilliamsFrom: Martin K. Petersen <[email protected]>
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.
--- block/bio-integrity.c | 4 ++-- block/blk-integrity.c | 6 +++--- include/linux/blkdev.h | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-)
block/bio-integrity.c +2 -2
@@ -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;
block/blk-integrity.c +3 -3
@@ -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;
include/linux/blkdev.h +4 -4
@@ -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 *);
PATCH v3 4/12 block: Export integrity data interval size in sysfs
In reply to Dan WilliamsFrom: Martin K. Petersen <[email protected]>
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.
--- Documentation/ABI/testing/sysfs-block | 7 +++++++ block/blk-integrity.c | 14 ++++++++++++++ 2 files changed, 21 insertions(+)
Documentation/ABI/testing/sysfs-block +7 -0
@@ -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
block/blk-integrity.c +14 -0
@@ -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,
PATCH v3 5/12 block: Inline blk_integrity in struct gendisk
In reply to Dan WilliamsFrom: Martin K. Petersen <[email protected]>
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).
--- 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(-)
block/blk-integrity.c +58 -102
@@ -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);
block/genhd.c +2 -0
@@ -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 */
block/partition-generic.c +1 -0
@@ -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)))
drivers/md/dm-table.c +47 -41
156 lines not on this page · raw message
drivers/md/md.c +3 -6
@@ -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",
drivers/nvdimm/core.c +1 -5
@@ -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 ;
drivers/nvme/host/pci.c +2 -3
@@ -538,7 +538,7 @@ static void nvme_dif_remap(struct request *req, virt =bip_get_seed (bip); phys =nvme_block_nr (ns,blk_rq_pos (req)); nlb = (blk_rq_bytes (req) >> ns->lba_shift); - ts = ns->disk->integrity->tuple_size; + ts = ns->disk->integrity.tuple_size;for (i =0 ; i < nlb; i++, virt++, phys++) { pi = (struct t10_pi_tuple *)p;
@@ -2044,8 +2044,7 @@ static int nvme_revalidate_disk(struct gendisk *disk) ns->pi_type = pi_type;blk_queue_logical_block_size (ns->queue, bs); -if (ns->ms && !blk_get_integrity (disk) && (disk->flags &GENHD_FL_UP ) && - !ns->ext) +if (ns->ms && !ns->ext)nvme_init_integrity (ns);if (ns->ms && !(ns->ms ==8 && ns->pi_type) && !blk_get_integrity (disk))
fs/block_dev.c +1 -1
@@ -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;
include/linux/blkdev.h +13 -21
@@ -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) {
include/linux/genhd.h +24 -2
@@ -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 ) { }
PATCH v3 6/12 md, dm, scsi, nvme, libnvdimm: drop blk_integrity_unregister() at shutdown
In reply to Dan WilliamsNow that the integrity profile is statically allocated there is no work to do when shutting down an integrity enabled block device.
--- 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(-)
drivers/md/dm.c +0 -2
@@ -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); }
drivers/md/md.c +0 -1
@@ -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 ;
drivers/nvdimm/btt.c +0 -1
@@ -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);
drivers/nvme/host/pci.c +1 -4
@@ -2407,11 +2407,8 @@ static void nvme_ns_remove(struct nvme_ns *ns)if (kill)blk_set_queue_dying (ns->queue); -if (ns->disk->flags &GENHD_FL_UP ) { -if (blk_get_integrity (ns->disk)) -blk_integrity_unregister (ns->disk); +if (ns->disk->flags &GENHD_FL_UP )del_gendisk (ns->disk); - }if (kill || !blk_queue_dying (ns->queue)) {blk_mq_abort_requeue_list (ns->queue);blk_cleanup_queue (ns->queue);
drivers/scsi/sd.c +0 -1
@@ -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);
PATCH v3 6/12 md, dm, scsi, nvme, libnvdimm: drop blk_integrity_unregister() at shutdown
For the nvme, scsi bits
PATCH v3 7/12 md: suspend i/o during runtime blk_integrity_unregister
In reply to Dan WilliamsSynchronize 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.
--- 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(+)
drivers/md/md.c +1 -0
@@ -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); }
drivers/md/multipath.c +2 -0
@@ -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 ; }
drivers/md/raid1.c +2 -0
@@ -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);
drivers/md/raid10.c +2 -0
@@ -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);
PATCH v3 7/12 md: suspend i/o during runtime blk_integrity_unregister
Quoted 20 lines · Dan Williams
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(+)drivers/md/raid10.c +0
-0
drivers/md/raid10.c static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev) · line 1744
Quoted 9 lines
drivers/md/raid10.c +2
-0@@ -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);
Hi Dan,
Somewhat belatedly I am testing this code, and it deadlocks. Which is obvious once I think about it.
raid10_add_disk() is called from
raid10d -> md_check_recovery -> remove_and_add_spares
so this call to mddev_suspend() will block raid10d until all pending IO completes. But raid10d might be needed to complete some IO - particularly in the case of errors. I don't know exactly what is deadlocking in my test, but it doesn't surprise me that something does.
Can you explain in more detail what needs to be synchronised here? If the synchronisation doesn't happen, what can do wrong? Because I cannot imagine what this might actually be protecting against.
Thanks,
NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20160113/219e785c/attachment-0001.sig>
PATCH v3 7/12 md: suspend i/o during runtime blk_integrity_unregister
Quoted 37 lines · NeilBrown
Quoted 20 lines · Dan Williams
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(+)drivers/md/raid10.c +0
-0drivers/md/raid10.c static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev) · line 1744
Quoted 9 lines
drivers/md/raid10.c +2
-0@@ -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);Hi Dan,
Somewhat belatedly I am testing this code, and it deadlocks. Which is obvious once I think about it.
Quoted 7 lines
raid10_add_disk()is called from
raid10d ->md_check_recovery->remove_and_add_sparesso this call to
mddev_suspend()will block raid10d until all pending IO completes. But raid10d might be needed to complete some IO - particularly in the case of errors. I don't know exactly what is deadlocking in my test, but it doesn't surprise me that something does.
Ugh.
Quoted 3 lines
Can you explain in more detail what needs to be synchronised here? If the synchronisation doesn't happen, what can do wrong? Because I cannot imagine what this might actually be protecting against.
My thinking is that if the integrity profile changes while i/o is in flight we can get spurious errors. For example when enabling integrity a checksum for in-flight commands will be missing, so wait for those to complete.
PATCH v3 7/12 md: suspend i/o during runtime blk_integrity_unregister
Quoted 17 lines · Dan Williams
Quoted 6 lines · NeilBrown
Hi Dan,
Somewhat belatedly I am testing this code, and it deadlocks. Which is obvious once I think about it.
Quoted 7 lines
raid10_add_disk()is called from
raid10d ->md_check_recovery->remove_and_add_sparesso this call to
mddev_suspend()will block raid10d until all pending IO completes. But raid10d might be needed to complete some IO - particularly in the case of errors. I don't know exactly what is deadlocking in my test, but it doesn't surprise me that something does.Ugh.
Quoted 8 lines
Quoted 3 lines
Can you explain in more detail what needs to be synchronised here? If the synchronisation doesn't happen, what can do wrong? Because I cannot imagine what this might actually be protecting against.
My thinking is that if the integrity profile changes while i/o is in flight we can get spurious errors. For example when enabling integrity a checksum for in-flight commands will be missing, so wait for those to complete.
DM avoids this because it doesn't allow changing the integrity profile once one is established.
PATCH v3 7/12 md: suspend i/o during runtime blk_integrity_unregister
In reply to Dan WilliamsQuoted 48 lines · Dan Williams
Quoted 37 lines · NeilBrown
Quoted 20 lines · Dan Williams
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(+)drivers/md/raid10.c +0
-0drivers/md/raid10.c static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev) · line 1744
Quoted 9 lines
drivers/md/raid10.c +2
-0@@ -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);Hi Dan,
Somewhat belatedly I am testing this code, and it deadlocks. Which is obvious once I think about it.
Quoted 7 lines
raid10_add_disk()is called from
raid10d ->md_check_recovery->remove_and_add_sparesso this call to
mddev_suspend()will block raid10d until all pending IO completes. But raid10d might be needed to complete some IO - particularly in the case of errors. I don't know exactly what is deadlocking in my test, but it doesn't surprise me that something does.Ugh.
Quoted 8 lines
Quoted 3 lines
Can you explain in more detail what needs to be synchronised here? If the synchronisation doesn't happen, what can do wrong? Because I cannot imagine what this might actually be protecting against.
My thinking is that if the integrity profile changes while i/o is in flight we can get spurious errors. For example when enabling integrity a checksum for in-flight commands will be missing, so wait for those to complete.
Who/what adds that checksum? The filesystem? mddev_suspend() doesn't stop the filesystem from submitting requests. It just stops them from getting into md. So when mddev_resume() completes, old requests can arrive.
If stability of the integrity profile is important, then we presumably need to make sure it never changes (while the array is active - or at least while it is mounted). So don't accept drives which don't support the current profile. Also don't upgrade the profile just because all current devices support some new feature.
Maybe the array should have a persistent knowledge of what profile it has and only accept devices which match?
Or maybe integrity should be completely disabled on arrays which can hot-add devices (so support it on RAID0 and LINEAR, but not on RAID1/4/5/6/10).
Seems like a bit of a mess.
NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20160113/f6e4f388/attachment.sig>
PATCH v3 7/12 md: suspend i/o during runtime blk_integrity_unregister
Quoted 59 lines · NeilBrown
Quoted 48 lines · Dan Williams
Quoted 37 lines · NeilBrown
Quoted 20 lines · Dan Williams
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(+)drivers/md/raid10.c +0
-0drivers/md/raid10.c static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev) · line 1744
Quoted 9 lines
drivers/md/raid10.c +2
-0@@ -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);Hi Dan,
Somewhat belatedly I am testing this code, and it deadlocks. Which is obvious once I think about it.
Quoted 7 lines
raid10_add_disk()is called from
raid10d ->md_check_recovery->remove_and_add_sparesso this call to
mddev_suspend()will block raid10d until all pending IO completes. But raid10d might be needed to complete some IO - particularly in the case of errors. I don't know exactly what is deadlocking in my test, but it doesn't surprise me that something does.Ugh.
Quoted 8 lines
Quoted 3 lines
Can you explain in more detail what needs to be synchronised here? If the synchronisation doesn't happen, what can do wrong? Because I cannot imagine what this might actually be protecting against.
My thinking is that if the integrity profile changes while i/o is in flight we can get spurious errors. For example when enabling integrity a checksum for in-flight commands will be missing, so wait for those to complete.
Quoted 4 lines
Who/what adds that checksum? The filesystem?
mddev_suspend()doesn't stop the filesystem from submitting requests. It just stops them from getting into md. So whenmddev_resume()completes, old requests can arrive.
The block layer generates the checksum when the bio is queued.
You're right the suspend does nothing to protect against the integrity generated for the array.
Quoted 7 lines
If stability of the integrity profile is important, then we presumably need to make sure it never changes (while the array is active - or at least while it is mounted). So don't accept drives which don't support the current profile. Also don't upgrade the profile just because all current devices support some new feature.
Quoted 5 lines
Maybe the array should have a persistent knowledge of what profile it has and only accept devices which match?
Or maybe integrity should be completely disabled on arrays which can hot-add devices (so support it on RAID0 and LINEAR, but not on RAID1/4/5/6/10).
For -stable I'll develop an immediate fix that disallows live changes of the integrity similar to DM.
However, given no filesystem is sending down app tags, I'm wondering why md has integrity enabled at the array level in the first place? Is this just future preparation for eventual filesystem enabling? Otherwise, the integrity handling for each component device is sufficient. I'm probably missing something.
PATCH v3 7/12 md: suspend i/o during runtime blk_integrity_unregister
Quoted 65 lines · Dan Williams
Quoted 59 lines · NeilBrown
Quoted 48 lines · Dan Williams
Quoted 37 lines · NeilBrown
Quoted 20 lines · Dan Williams
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(+)drivers/md/raid10.c +0
-0drivers/md/raid10.c static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev) · line 1744
Quoted 9 lines
drivers/md/raid10.c +2
-0@@ -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);Hi Dan,
Somewhat belatedly I am testing this code, and it deadlocks. Which is obvious once I think about it.
Quoted 7 lines
raid10_add_disk()is called from
raid10d ->md_check_recovery->remove_and_add_sparesso this call to
mddev_suspend()will block raid10d until all pending IO completes. But raid10d might be needed to complete some IO - particularly in the case of errors. I don't know exactly what is deadlocking in my test, but it doesn't surprise me that something does.Ugh.
Quoted 8 lines
Quoted 3 lines
Can you explain in more detail what needs to be synchronised here? If the synchronisation doesn't happen, what can do wrong? Because I cannot imagine what this might actually be protecting against.
My thinking is that if the integrity profile changes while i/o is in flight we can get spurious errors. For example when enabling integrity a checksum for in-flight commands will be missing, so wait for those to complete.
Quoted 4 lines
Who/what adds that checksum? The filesystem?
mddev_suspend()doesn't stop the filesystem from submitting requests. It just stops them from getting into md. So whenmddev_resume()completes, old requests can arrive.
Quoted 4 lines
The block layer generates the checksum when the bio is queued.
You're right the suspend does nothing to protect against the integrity generated for the array.
MD and DM are _not_ using the Linux block layer's ability to generate protection information metadata (via bio_integrity_generate). As such they are only providing pass-through of upper layer application generated protection information (DIX).
A bit more explanation:
bio-based MD and DM's cloning of protection information causes their cloned bios to have bio->bi_integrity -- as such the underlying request-based driver skips bio_integrity_generate() via blk_integrity_prep() (because blk_queue_bio()'s call to bio_integrity_enabled() returns false for MD and DM's cloned bios).
Quoted 12 lines
Quoted 11 lines
If stability of the integrity profile is important, then we presumably need to make sure it never changes (while the array is active - or at least while it is mounted). So don't accept drives which don't support the current profile. Also don't upgrade the profile just because all current devices support some new feature.
Maybe the array should have a persistent knowledge of what profile it has and only accept devices which match?
Or maybe integrity should be completely disabled on arrays which can hot-add devices (so support it on RAID0 and LINEAR, but not on RAID1/4/5/6/10).
Quoted 8 lines
For -stable I'll develop an immediate fix that disallows live changes of the integrity similar to DM.
However, given no filesystem is sending down app tags, I'm wondering why md has integrity enabled at the array level in the first place? Is this just future preparation for eventual filesystem enabling? Otherwise, the integrity handling for each component device is sufficient. I'm probably missing something.
AFAIK, Oracle DB is the only application known to be using DIX.
PATCH 1/1 md/raid: only permit hot-add of compatible integrity profiles
In reply to NeilBrownIt is not safe for an integrity profile to be changed while i/o is in-flight in the queue. Prevent adding new disks or otherwise online spares to an array if the device has an incompatible integrity profile.
The original change to the blk_integrity_unregister implementation in md, commmit c7bfced9a671 "md: suspend i/o during runtime blk_integrity_unregister" introduced an immediate hang regression.
This policy of disallowing changes the integrity profile once one has been established is shared with DM.
Here is an abbreviated log from a test run that:
1/ Creates a degraded raid1 with an integrity-enabled device (pmem0s) [ 59.076127]
2/ Tries to add an integrity-disabled device (pmem1m) [ 90.489209]
3/ Retries with an integrity-enabled device (pmem1s) [ 205.671277]
[ 59.076127] md/raid1:md0: active with 1 out of 2 mirrors [ 59.078302] md: data integrity enabled on md0 [..] [ 90.489209] md0: incompatible integrity profile for pmem1m [..] [ 205.671277] md: super_written gets error=-5 [ 205.677386] md/raid1:md0: Disk failure on pmem1m, disabling device. [ 205.677386] md/raid1:md0: Operation continuing on 1 devices. [ 205.683037] RAID1 conf printout: [ 205.684699] --- wd:1 rd:2 [ 205.685972] disk 0, wo:0, o:1, dev:pmem0s [ 205.687562] disk 1, wo:1, o:1, dev:pmem1s [ 205.691717] md: recovery of RAID array md0
--- drivers/md/md.c | 28 ++++++++++++++++------------ drivers/md/md.h | 2 +- drivers/md/multipath.c | 6 +++--- drivers/md/raid1.c | 6 +++--- drivers/md/raid10.c | 6 +++--- 5 files changed, 26 insertions(+), 22 deletions(-)
drivers/md/md.c +16 -12
@@ -2017,28 +2017,32 @@ int md_integrity_register(struct mddev *mddev) }EXPORT_SYMBOL (md_integrity_register); -/* Disable data integrity if non-capable/non-matching disk is being added */ -void md_integrity_add_rdev (struct md_rdev *rdev,struct mddev *mddev) +/* +* Attempt to add an rdev, but only if it is consistent with the current +* integrity profile +*/ +int md_integrity_add_rdev (struct md_rdev *rdev,struct mddev *mddev) {struct blk_integrity *bi_rdev;struct blk_integrity *bi_mddev; +char name[BDEVNAME_SIZE ];if (!mddev->gendisk) -return ; +return 0 ; bi_rdev =bdev_get_integrity (rdev->bdev); bi_mddev =blk_get_integrity (mddev->gendisk);if (!bi_mddev)/* nothing to do */ -return ; -if (rdev->raid_disk <0 )/* skip spares */ -return ; -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); +return 0 ; + +if (blk_integrity_compare (mddev->gendisk, rdev->bdev->bd_disk) !=0 ) { +printk (KERN_NOTICE "%s: incompatible integrity profile for %s\n" , +mdname (mddev),bdevname (rdev->bdev, name)); +return -ENXIO ; + } + +return 0 ; }EXPORT_SYMBOL (md_integrity_add_rdev);
drivers/md/md.h +1 -1
@@ -657,7 +657,7 @@ extern void md_wait_for_blocked_rdev(struct md_rdev *rdev, struct mddev *mddev);extern void md_set_array_sectors (struct mddev *mddev,sector_t array_sectors);extern int md_check_no_bitmap (struct mddev *mddev);extern int md_integrity_register (struct mddev *mddev); -extern void md_integrity_add_rdev (struct md_rdev *rdev,struct mddev *mddev); +extern int md_integrity_add_rdev (struct md_rdev *rdev,struct mddev *mddev);extern int strict_strtoul_scaled (const char *cp,unsigned long *res,int scale);extern void mddev_init (struct mddev *mddev);
drivers/md/multipath.c +3 -3
@@ -257,6 +257,9 @@ static int multipath_add_disk(struct mddev *mddev, struct md_rdev *rdev)disk_stack_limits (mddev->gendisk, rdev->bdev, rdev->data_offset <<9 ); + err =md_integrity_add_rdev (rdev, mddev); +if (err) +break ;spin_lock_irq (&conf->device_lock); mddev->degraded--; rdev->raid_disk = path;
@@ -264,9 +267,6 @@ 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 ; }
drivers/md/raid1.c +3 -3
@@ -1589,6 +1589,9 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)if (mddev->recovery_disabled == conf->recovery_disabled)return -EBUSY ; +if (md_integrity_add_rdev (rdev, mddev)) +return -ENXIO ; +if (rdev->raid_disk >=0 ) first = last = rdev->raid_disk;
@@ -1632,9 +1635,6 @@ 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);
drivers/md/raid10.c +3 -3
@@ -1698,6 +1698,9 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)if (rdev->saved_raid_disk <0 && !_enough (conf,1 ,-1 ))return -EINVAL ; +if (md_integrity_add_rdev (rdev, mddev)) +return -ENXIO ; +if (rdev->raid_disk >=0 ) first = last = rdev->raid_disk;
@@ -1739,9 +1742,6 @@ 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);
Re: [PATCH] md/raid: only permit hot-add of compatible integrity profiles
Quoted 10 lines · Dan Williams
It is not safe for an integrity profile to be changed while i/o is in-flight in the queue. Prevent adding new disks or otherwise online spares to an array if the device has an incompatible integrity profile.
The original change to the
blk_integrity_unregisterimplementation in md, commmitc7bfced9a671"md: suspend i/o during runtimeblk_integrity_unregister" introduced an immediate hang regression.This policy of disallowing changes the integrity profile once one has been established is shared with DM.
Thanks Dan. That looks like it should address the issues and seems to make sense.
If it passes my smoke-testing I'll include it in my merge-window pull request tomorrow.
NeilBrown
Quoted 158 lines
Here is an abbreviated log from a test run that:
1/ Creates a degraded raid1 with an integrity-enabled device (pmem0s) [ 59.076127]
2/ Tries to add an integrity-disabled device (pmem1m) [ 90.489209]
3/ Retries with an integrity-enabled device (pmem1s) [ 205.671277][ 59.076127] md/raid1:md0: active with 1 out of 2 mirrors [ 59.078302] md: data integrity enabled on md0 [..] [ 90.489209] md0: incompatible integrity profile for pmem1m [..] [ 205.671277] md: super_written gets error=-5 [ 205.677386] md/raid1:md0: Disk failure on pmem1m, disabling device. [ 205.677386] md/raid1:md0: Operation continuing on 1 devices. [ 205.683037] RAID1 conf printout: [ 205.684699] --- wd:1 rd:2 [ 205.685972] disk 0, wo:0, o:1, dev:pmem0s [ 205.687562] disk 1, wo:1, o:1, dev:pmem1s [ 205.691717] md: recovery of RAID array md0Fixes c7bfced9a671 ("md: suspend i/o during runtime blk_integrity_unregister")Cc <stable at vger.kernel.org>Cc Mike Snitzer <snitzer at redhat.com>Reported-by NeilBrown <neilb at suse.com>Signed-off-by Dan Williams <dan.j.williams at intel.com>--- drivers/md/md.c | 28 ++++++++++++++++------------ drivers/md/md.h | 2 +- drivers/md/multipath.c | 6 +++--- drivers/md/raid1.c | 6 +++--- drivers/md/raid10.c | 6 +++--- 5 files changed, 26 insertions(+), 22 deletions(-)drivers/md/md.c +16
-12@@ -2017,28 +2017,32 @@ int md_integrity_register(struct mddev *mddev) }EXPORT_SYMBOL (md_integrity_register); -/* Disable data integrity if non-capable/non-matching disk is being added */ -void md_integrity_add_rdev (struct md_rdev *rdev,struct mddev *mddev) +/* +* Attempt to add an rdev, but only if it is consistent with the current +* integrity profile +*/ +int md_integrity_add_rdev (struct md_rdev *rdev,struct mddev *mddev) {struct blk_integrity *bi_rdev;struct blk_integrity *bi_mddev; +char name[BDEVNAME_SIZE ];if (!mddev->gendisk) -return ; +return 0 ; bi_rdev =bdev_get_integrity (rdev->bdev); bi_mddev =blk_get_integrity (mddev->gendisk);if (!bi_mddev)/* nothing to do */ -return ; -if (rdev->raid_disk <0 )/* skip spares */ -return ; -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); +return 0 ; + +if (blk_integrity_compare (mddev->gendisk, rdev->bdev->bd_disk) !=0 ) { +printk (KERN_NOTICE "%s: incompatible integrity profile for %s\n" , +mdname (mddev),bdevname (rdev->bdev, name)); +return -ENXIO ; + } + +return 0 ; }EXPORT_SYMBOL (md_integrity_add_rdev);drivers/md/md.h +1
-1@@ -657,7 +657,7 @@ extern void md_wait_for_blocked_rdev(struct md_rdev *rdev, struct mddev *mddev);extern void md_set_array_sectors (struct mddev *mddev,sector_t array_sectors);extern int md_check_no_bitmap (struct mddev *mddev);extern int md_integrity_register (struct mddev *mddev); -extern void md_integrity_add_rdev (struct md_rdev *rdev,struct mddev *mddev); +extern int md_integrity_add_rdev (struct md_rdev *rdev,struct mddev *mddev);extern int strict_strtoul_scaled (const char *cp,unsigned long *res,int scale);extern void mddev_init (struct mddev *mddev);drivers/md/multipath.c +3
-3@@ -257,6 +257,9 @@ static int multipath_add_disk(struct mddev *mddev, struct md_rdev *rdev)disk_stack_limits (mddev->gendisk, rdev->bdev, rdev->data_offset <<9 ); + err =md_integrity_add_rdev (rdev, mddev); +if (err) +break ;spin_lock_irq (&conf->device_lock); mddev->degraded--; rdev->raid_disk = path;@@ -264,9 +267,6 @@ 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 ; }drivers/md/raid1.c +3
-3@@ -1589,6 +1589,9 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)if (mddev->recovery_disabled == conf->recovery_disabled)return -EBUSY ; +if (md_integrity_add_rdev (rdev, mddev)) +return -ENXIO ; +if (rdev->raid_disk >=0 ) first = last = rdev->raid_disk;@@ -1632,9 +1635,6 @@ 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);drivers/md/raid10.c +3
-0@@ -1698,6 +1698,9 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)if (rdev->saved_raid_disk <0 && !_enough (conf,1 ,-1 ))return -EINVAL ; +if (md_integrity_add_rdev (rdev, mddev)) +return -ENXIO ; +if (rdev->raid_disk >=0 ) first = last = rdev->raid_disk;
drivers/md/raid10.c static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev) · line 1747
Quoted 9 lines
drivers/md/raid10.c +0
-3@@ -1739,9 +1742,6 @@ 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);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20160114/2ffd2f07/attachment.sig>
PATCH v3 8/12 nvme: suspend i/o during runtime blk_integrity_unregister
In reply to Dan WilliamsSynchronize pending i/o against a change in the integrity profile to avoid the possibility of spurious integrity errors.
--- drivers/nvme/host/pci.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/nvme/host/pci.c +2 -0
@@ -2035,6 +2035,7 @@ static int nvme_revalidate_disk(struct gendisk *disk) pi_type = ns->ms ==sizeof (struct t10_pi_tuple ) ? id->dps &NVME_NS_DPS_PI_MASK :0 ; +blk_mq_freeze_queue (disk->queue); if (blk_get_integrity(disk) && (ns->pi_type != pi_type || ns->ms != old_ms || bs != queue_logical_block_size(disk->queue) ||
@@ -2054,6 +2055,7 @@ static int nvme_revalidate_disk(struct gendisk *disk)if (dev->oncs &NVME_CTRL_ONCS_DSM )nvme_config_discard (ns); +blk_mq_unfreeze_queue (disk->queue);kfree (id);return 0 ;
PATCH v3 8/12 nvme: suspend i/o during runtime blk_integrity_unregister
Looks good,
PATCH v3 9/12 block: generic request_queue reference counting
In reply to Dan WilliamsAllow 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
--- 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(-)
block/blk-core.c +62 -9
@@ -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 */ }
block/blk-mq-sysfs.c +0 -6
@@ -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);
block/blk-mq.c +24 -56
150 lines not on this page · raw message
block/blk-sysfs.c +1 -2
@@ -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);
block/blk.h +14 -0
@@ -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);
include/linux/blk-mq.h +0 -1
@@ -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 *);
include/linux/blkdev.h +1 -1
@@ -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;
PATCH v3 9/12 block: generic request_queue reference counting
This looks good,
This is running in my setups for some time now so,
PATCH v3 10/12 block: move blk_integrity to request_queue
In reply to Dan WilliamsA 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/0x4e0Given 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.
--- 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(-)
block/blk-integrity.c +7 -7
@@ -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 ;
drivers/nvme/host/pci.c +1 -1
@@ -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;
include/linux/blkdev.h +5 -1
@@ -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 ;
include/linux/genhd.h +0 -1
@@ -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;
PATCH v3 10/12 block: move blk_integrity to request_queue
Looks good,
PATCH v3 11/12 block: blk_flush_integrity() for bio-based drivers
In reply to Dan WilliamsSince 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.
--- block/bio-integrity.c | 5 +++++ block/blk-core.c | 3 +++ block/blk.h | 8 ++++++++ 3 files changed, 16 insertions(+)
block/bio-integrity.c +5 -0
@@ -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
block/blk-core.c +3 -0
@@ -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);
block/blk.h +8 -0
@@ -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);
PATCH v3 11/12 block: blk_flush_integrity() for bio-based drivers
Looks good,
PATCH v3 12/12 block, libnvdimm, nvme: provide a built-in blk_integrity nop profile
In reply to Dan WilliamsThe 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.
--- block/blk-integrity.c | 13 ++++++++++++- drivers/nvdimm/core.c | 12 +----------- drivers/nvme/host/pci.c | 18 +----------------- 3 files changed, 14 insertions(+), 29 deletions(-)
block/blk-integrity.c +12 -1
@@ -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 gendisk as being integrity-capable * @disk: struct gendisk pointer to make integrity-aware
@@ -402,7 +413,7 @@ void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template 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->profile = template->profile ? template->profile : &nop_profile; bi->tuple_size = template->tuple_size; bi->tag_size = template->tag_size;
drivers/nvdimm/core.c +1 -11
@@ -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;
drivers/nvme/host/pci.c +1 -17
@@ -548,22 +548,6 @@ static void nvme_dif_remap(struct request *req,kunmap_atomic (pmap); } -static int nvme_noop_verify (struct blk_integrity_iter *iter) -{ -return 0 ; -} - -static int nvme_noop_generate (struct blk_integrity_iter *iter) -{ -return 0 ; -} - -struct blk_integrity_profile nvme_meta_noop = { - .name ="NVME_META_NOOP" , - .generate_fn = nvme_noop_generate, - .verify_fn = nvme_noop_verify, -}; -static void nvme_init_integrity (struct nvme_ns *ns) {struct blk_integrity integrity;
@@ -577,7 +561,7 @@ static void nvme_init_integrity(struct nvme_ns *ns) integrity.profile = &t10_pi_type1_crc;break ;default : - integrity.profile = &nvme_meta_noop; + integrity.profile =NULL ;break ; } integrity.tuple_size = ns->ms;
PATCH v3 12/12 block, libnvdimm, nvme: provide a built-in blk_integrity nop profile
For the block, nvme bits
Christoph Hellwig
maintainerv3 cover Simplify block integrity registration v3
In reply to Dan WilliamsHi Dan,
the series looks fine to me: