Commit e9f5f44a authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Jens Axboe
Browse files

block: remove the blk_integrity_profile structure



Block layer integrity configuration is a bit complex right now, as it
indirects through operation vectors for a simple two-dimensional
configuration:

 a) the checksum type of none, ip checksum, crc, crc64
 b) the presence or absence of a reference tag

Remove the integrity profile, and instead add a separate csum_type flag
which replaces the existing ip-checksum field and a new flag that
indicates the presence of the reference tag.

This removes up to two layers of indirect calls, remove the need to
offload the no-op verification of non-PI metadata to a workqueue and
generally simplifies the code. The downside is that block/t10-pi.c now
has to be built into the kernel when CONFIG_BLK_DEV_INTEGRITY is
supported.  Given that both nvme and SCSI require t10-pi.ko, it is loaded
for all usual configurations that enabled CONFIG_BLK_DEV_INTEGRITY
already, though.

Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarKanchan Joshi <joshi.k@samsung.com>
Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
Reviewed-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
Link: https://lore.kernel.org/r/20240613084839.1044015-6-hch@lst.de


Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 63e64959
Loading
Loading
Loading
Loading
+2 −6
Original line number Diff line number Diff line
@@ -62,6 +62,8 @@ config BLK_DEV_BSGLIB

config BLK_DEV_INTEGRITY
	bool "Block layer data integrity support"
	select CRC_T10DIF
	select CRC64_ROCKSOFT
	help
	Some storage devices allow extra information to be
	stored/retrieved to help protect the data.  The block layer
@@ -72,12 +74,6 @@ config BLK_DEV_INTEGRITY
	T10/SCSI Data Integrity Field or the T13/ATA External Path
	Protection.  If in doubt, say N.

config BLK_DEV_INTEGRITY_T10
	tristate
	depends on BLK_DEV_INTEGRITY
	select CRC_T10DIF
	select CRC64_ROCKSOFT

config BLK_DEV_WRITE_MOUNTED
	bool "Allow writing to mounted block devices"
	default y
+1 −2
Original line number Diff line number Diff line
@@ -26,8 +26,7 @@ obj-$(CONFIG_MQ_IOSCHED_KYBER) += kyber-iosched.o
bfq-y				:= bfq-iosched.o bfq-wf2q.o bfq-cgroup.o
obj-$(CONFIG_IOSCHED_BFQ)	+= bfq.o

obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o blk-integrity.o
obj-$(CONFIG_BLK_DEV_INTEGRITY_T10)	+= t10-pi.o
obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o blk-integrity.o t10-pi.o
obj-$(CONFIG_BLK_MQ_PCI)	+= blk-mq-pci.o
obj-$(CONFIG_BLK_MQ_VIRTIO)	+= blk-mq-virtio.o
obj-$(CONFIG_BLK_DEV_ZONED)	+= blk-zoned.o
+13 −19
Original line number Diff line number Diff line
@@ -378,10 +378,9 @@ EXPORT_SYMBOL_GPL(bio_integrity_map_user);
 * bio_integrity_process - Process integrity metadata for a bio
 * @bio:	bio to generate/verify integrity metadata for
 * @proc_iter:  iterator to process
 * @proc_fn:	Pointer to the relevant processing function
 */
static blk_status_t bio_integrity_process(struct bio *bio,
		struct bvec_iter *proc_iter, integrity_processing_fn *proc_fn)
		struct bvec_iter *proc_iter)
{
	struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
	struct blk_integrity_iter iter;
@@ -392,17 +391,18 @@ static blk_status_t bio_integrity_process(struct bio *bio,

	iter.disk_name = bio->bi_bdev->bd_disk->disk_name;
	iter.interval = 1 << bi->interval_exp;
	iter.tuple_size = bi->tuple_size;
	iter.seed = proc_iter->bi_sector;
	iter.prot_buf = bvec_virt(bip->bip_vec);
	iter.pi_offset = bi->pi_offset;

	__bio_for_each_segment(bv, bio, bviter, *proc_iter) {
		void *kaddr = bvec_kmap_local(&bv);

		iter.data_buf = kaddr;
		iter.data_size = bv.bv_len;
		ret = proc_fn(&iter);
		if (bio_data_dir(bio) == WRITE)
			blk_integrity_generate(&iter, bi);
		else
			ret = blk_integrity_verify(&iter, bi);
		kunmap_local(kaddr);

		if (ret)
@@ -448,12 +448,10 @@ bool bio_integrity_prep(struct bio *bio)
		return true;

	if (bio_data_dir(bio) == READ) {
		if (!bi->profile->verify_fn ||
		    !(bi->flags & BLK_INTEGRITY_VERIFY))
		if (!(bi->flags & BLK_INTEGRITY_VERIFY))
			return true;
	} else {
		if (!bi->profile->generate_fn ||
		    !(bi->flags & BLK_INTEGRITY_GENERATE))
		if (!(bi->flags & BLK_INTEGRITY_GENERATE))
			return true;

		/*
@@ -488,7 +486,7 @@ bool bio_integrity_prep(struct bio *bio)
	bip->bip_flags |= BIP_BLOCK_INTEGRITY;
	bip_set_seed(bip, bio->bi_iter.bi_sector);

	if (bi->flags & BLK_INTEGRITY_IP_CHECKSUM)
	if (bi->csum_type == BLK_INTEGRITY_CSUM_IP)
		bip->bip_flags |= BIP_IP_CHECKSUM;

	/* Map it */
@@ -511,12 +509,10 @@ bool 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, &bio->bi_iter,
				      bi->profile->generate_fn);
	} else {
	if (bio_data_dir(bio) == WRITE)
		bio_integrity_process(bio, &bio->bi_iter);
	else
		bip->bio_iter = bio->bi_iter;
	}
	return true;

err_end_io:
@@ -539,15 +535,13 @@ static void bio_integrity_verify_fn(struct work_struct *work)
	struct bio_integrity_payload *bip =
		container_of(work, struct bio_integrity_payload, bip_work);
	struct bio *bio = bip->bip_bio;
	struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);

	/*
	 * At the moment verify is called bio's iterator was advanced
	 * during split and completion, we need to rewind iterator to
	 * it's original position.
	 */
	bio->bi_status = bio_integrity_process(bio, &bip->bio_iter,
						bi->profile->verify_fn);
	bio->bi_status = bio_integrity_process(bio, &bip->bio_iter);
	bio_integrity_free(bio);
	bio_endio(bio);
}
@@ -569,7 +563,7 @@ bool __bio_integrity_endio(struct bio *bio)
	struct bio_integrity_payload *bip = bio_integrity(bio);

	if (bio_op(bio) == REQ_OP_READ && !bio->bi_status &&
	    (bip->bip_flags & BIP_BLOCK_INTEGRITY) && bi->profile->verify_fn) {
	    (bip->bip_flags & BIP_BLOCK_INTEGRITY) && bi->csum_type) {
		INIT_WORK(&bip->bip_work, bio_integrity_verify_fn);
		queue_work(kintegrityd_wq, &bip->bip_work);
		return false;
+35 −31
Original line number Diff line number Diff line
@@ -123,10 +123,10 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
	struct blk_integrity *b1 = &gd1->queue->integrity;
	struct blk_integrity *b2 = &gd2->queue->integrity;

	if (!b1->profile && !b2->profile)
	if (!b1->tuple_size && !b2->tuple_size)
		return 0;

	if (!b1->profile || !b2->profile)
	if (!b1->tuple_size || !b2->tuple_size)
		return -1;

	if (b1->interval_exp != b2->interval_exp) {
@@ -150,10 +150,13 @@ int blk_integrity_compare(struct gendisk *gd1, struct gendisk *gd2)
		return -1;
	}

	if (b1->profile != b2->profile) {
	if (b1->csum_type != b2->csum_type ||
	    (b1->flags & BLK_INTEGRITY_REF_TAG) !=
	    (b2->flags & BLK_INTEGRITY_REF_TAG)) {
		pr_err("%s: %s/%s type %s != %s\n", __func__,
		       gd1->disk_name, gd2->disk_name,
		       b1->profile->name, b2->profile->name);
		       blk_integrity_profile_name(b1),
		       blk_integrity_profile_name(b2));
		return -1;
	}

@@ -217,14 +220,37 @@ static inline struct blk_integrity *dev_to_bi(struct device *dev)
	return &dev_to_disk(dev)->queue->integrity;
}

const char *blk_integrity_profile_name(struct blk_integrity *bi)
{
	switch (bi->csum_type) {
	case BLK_INTEGRITY_CSUM_IP:
		if (bi->flags & BLK_INTEGRITY_REF_TAG)
			return "T10-DIF-TYPE1-IP";
		return "T10-DIF-TYPE3-IP";
	case BLK_INTEGRITY_CSUM_CRC:
		if (bi->flags & BLK_INTEGRITY_REF_TAG)
			return "T10-DIF-TYPE1-CRC";
		return "T10-DIF-TYPE3-CRC";
	case BLK_INTEGRITY_CSUM_CRC64:
		if (bi->flags & BLK_INTEGRITY_REF_TAG)
			return "EXT-DIF-TYPE1-CRC64";
		return "EXT-DIF-TYPE3-CRC64";
	case BLK_INTEGRITY_CSUM_NONE:
		break;
	}

	return "nop";
}
EXPORT_SYMBOL_GPL(blk_integrity_profile_name);

static ssize_t format_show(struct device *dev, struct device_attribute *attr,
			   char *page)
{
	struct blk_integrity *bi = dev_to_bi(dev);

	if (bi->profile && bi->profile->name)
		return sysfs_emit(page, "%s\n", bi->profile->name);
	if (!bi->tuple_size)
		return sysfs_emit(page, "none\n");
	return sysfs_emit(page, "%s\n", blk_integrity_profile_name(bi));
}

static ssize_t tag_size_show(struct device *dev, struct device_attribute *attr,
@@ -326,28 +352,6 @@ const struct attribute_group blk_integrity_attr_group = {
	.attrs = integrity_attrs,
};

static blk_status_t blk_integrity_nop_fn(struct blk_integrity_iter *iter)
{
	return BLK_STS_OK;
}

static void blk_integrity_nop_prepare(struct request *rq)
{
}

static void blk_integrity_nop_complete(struct request *rq,
		unsigned int nr_bytes)
{
}

static const struct blk_integrity_profile nop_profile = {
	.name = "nop",
	.generate_fn = blk_integrity_nop_fn,
	.verify_fn = blk_integrity_nop_fn,
	.prepare_fn = blk_integrity_nop_prepare,
	.complete_fn = blk_integrity_nop_complete,
};

/**
 * blk_integrity_register - Register a gendisk as being integrity-capable
 * @disk:	struct gendisk pointer to make integrity-aware
@@ -363,11 +367,11 @@ void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template
{
	struct blk_integrity *bi = &disk->queue->integrity;

	bi->csum_type = template->csum_type;
	bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
		template->flags;
	bi->interval_exp = template->interval_exp ? :
		ilog2(queue_logical_block_size(disk->queue));
	bi->profile = template->profile ? template->profile : &nop_profile;
	bi->tuple_size = template->tuple_size;
	bi->tag_size = template->tag_size;
	bi->pi_offset = template->pi_offset;
@@ -394,7 +398,7 @@ void blk_integrity_unregister(struct gendisk *disk)
{
	struct blk_integrity *bi = &disk->queue->integrity;

	if (!bi->profile)
	if (!bi->tuple_size)
		return;

	/* ensure all bios are off the integrity workqueue */
+4 −9
Original line number Diff line number Diff line
@@ -804,10 +804,8 @@ static void blk_complete_request(struct request *req)
	if (!bio)
		return;

#ifdef CONFIG_BLK_DEV_INTEGRITY
	if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ)
		req->q->integrity.profile->complete_fn(req, total_bytes);
#endif
		blk_integrity_complete(req, total_bytes);

	/*
	 * Upper layers may call blk_crypto_evict_key() anytime after the last
@@ -875,11 +873,9 @@ bool blk_update_request(struct request *req, blk_status_t error,
	if (!req->bio)
		return false;

#ifdef CONFIG_BLK_DEV_INTEGRITY
	if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ &&
	    error == BLK_STS_OK)
		req->q->integrity.profile->complete_fn(req, nr_bytes);
#endif
		blk_integrity_complete(req, nr_bytes);

	/*
	 * Upper layers may call blk_crypto_evict_key() anytime after the last
@@ -1264,10 +1260,9 @@ void blk_mq_start_request(struct request *rq)
	WRITE_ONCE(rq->state, MQ_RQ_IN_FLIGHT);
	rq->mq_hctx->tags->rqs[rq->tag] = rq;

#ifdef CONFIG_BLK_DEV_INTEGRITY
	if (blk_integrity_rq(rq) && req_op(rq) == REQ_OP_WRITE)
		q->integrity.profile->prepare_fn(rq);
#endif
		blk_integrity_prepare(rq);

	if (rq->bio && rq->bio->bi_opf & REQ_POLLED)
	        WRITE_ONCE(rq->bio->bi_cookie, rq->mq_hctx->queue_num);
}
Loading