From 193700b9b218a77222a1b5cd53206c17c40f786b Mon Sep 17 00:00:00 2001 From: liujing Date: Mon, 9 Dec 2024 11:10:55 +0800 Subject: [PATCH 01/17] dm raid: fix spelling errors in raid_ctr() Fix the respective spelling errors in raid_ctr() function. Signed-off-by: liujing Signed-off-by: Mikulas Patocka --- drivers/md/dm-raid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c index 1e0d3b9b75d6..6adc55fd90d3 100644 --- a/drivers/md/dm-raid.c +++ b/drivers/md/dm-raid.c @@ -3196,7 +3196,7 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv) if (reshape_sectors || rs_is_raid1(rs)) { /* * We can only prepare for a reshape here, because the - * raid set needs to run to provide the repective reshape + * raid set needs to run to provide the respective reshape * check functions via its MD personality instance. * * So do the reshape check after md_run() succeeded. From c3a1f2ef72a97a79410c566022c5ea962f815ed9 Mon Sep 17 00:00:00 2001 From: Ethan Carter Edwards Date: Tue, 24 Dec 2024 22:13:21 +0000 Subject: [PATCH 02/17] dm: change kzalloc to kcalloc Use 2-factor multiplication argument form kcalloc() instead of instead of the deprecated kzalloc() [1]. [1] https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments Link: https://github.com/KSPP/linux/issues/162 Signed-off-by: Ethan Carter Edwards Reviewed-by: Gustavo A. R. Silva Signed-off-by: Mikulas Patocka --- drivers/md/dm-ps-io-affinity.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/dm-ps-io-affinity.c b/drivers/md/dm-ps-io-affinity.c index 461ee6b2044d..716807e511ee 100644 --- a/drivers/md/dm-ps-io-affinity.c +++ b/drivers/md/dm-ps-io-affinity.c @@ -116,7 +116,7 @@ static int ioa_create(struct path_selector *ps, unsigned int argc, char **argv) if (!s) return -ENOMEM; - s->path_map = kzalloc(nr_cpu_ids * sizeof(struct path_info *), + s->path_map = kcalloc(nr_cpu_ids, sizeof(struct path_info *), GFP_KERNEL); if (!s->path_map) goto free_selector; From 6942348d1bbdb612bc14025cab467fd1f012abe8 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Thu, 9 Jan 2025 14:47:20 +0100 Subject: [PATCH 03/17] dm: remove useless test in alloc_multiple_bios The test gfp_flag & GFP_NOWAIT was always true, because both GFP_NOIO and GFP_NOWAIT include the flag __GFP_KSWAPD_RECLAIM. Luckily, this oversight didn't result in any harm; the loop always started with "try = 0". This patch removes the faulty test and explicitly starts the loop with "try = 0" (so that we don't hold the mutex in the first iteration). Signed-off-by: Mikulas Patocka --- drivers/md/dm.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 12ecf07a3841..4817ef1206a9 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1479,12 +1479,12 @@ static void setup_split_accounting(struct clone_info *ci, unsigned int len) static void alloc_multiple_bios(struct bio_list *blist, struct clone_info *ci, struct dm_target *ti, unsigned int num_bios, - unsigned *len, gfp_t gfp_flag) + unsigned *len) { struct bio *bio; - int try = (gfp_flag & GFP_NOWAIT) ? 0 : 1; + int try; - for (; try < 2; try++) { + for (try = 0; try < 2; try++) { int bio_nr; if (try && num_bios > 1) @@ -1508,8 +1508,7 @@ static void alloc_multiple_bios(struct bio_list *blist, struct clone_info *ci, } static unsigned int __send_duplicate_bios(struct clone_info *ci, struct dm_target *ti, - unsigned int num_bios, unsigned int *len, - gfp_t gfp_flag) + unsigned int num_bios, unsigned int *len) { struct bio_list blist = BIO_EMPTY_LIST; struct bio *clone; @@ -1526,7 +1525,7 @@ static unsigned int __send_duplicate_bios(struct clone_info *ci, struct dm_targe * Using alloc_multiple_bios(), even if num_bios is 1, to consistently * support allocating using GFP_NOWAIT with GFP_NOIO fallback. */ - alloc_multiple_bios(&blist, ci, ti, num_bios, len, gfp_flag); + alloc_multiple_bios(&blist, ci, ti, num_bios, len); while ((clone = bio_list_pop(&blist))) { if (num_bios > 1) dm_tio_set_flag(clone_to_tio(clone), DM_TIO_IS_DUPLICATE_BIO); @@ -1564,7 +1563,7 @@ static void __send_empty_flush(struct clone_info *ci) atomic_add(ti->num_flush_bios, &ci->io->io_count); bios = __send_duplicate_bios(ci, ti, ti->num_flush_bios, - NULL, GFP_NOWAIT); + NULL); atomic_sub(ti->num_flush_bios - bios, &ci->io->io_count); } } else { @@ -1612,7 +1611,7 @@ static void __send_abnormal_io(struct clone_info *ci, struct dm_target *ti, __max_io_len(ti, ci->sector, max_granularity, max_sectors)); atomic_add(num_bios, &ci->io->io_count); - bios = __send_duplicate_bios(ci, ti, num_bios, &len, GFP_NOIO); + bios = __send_duplicate_bios(ci, ti, num_bios, &len); /* * alloc_io() takes one extra reference for submission, so the * reference won't reach 0 without the following (+1) subtraction @@ -1849,7 +1848,7 @@ static blk_status_t __send_zone_reset_all_emulated(struct clone_info *ci, * not go crazy with the clone allocation. */ alloc_multiple_bios(&blist, ci, ti, min(nr_reset, 32), - NULL, GFP_NOIO); + NULL); } /* Get a clone and change it to a regular reset operation. */ @@ -1881,7 +1880,7 @@ static void __send_zone_reset_all_native(struct clone_info *ci, unsigned int bios; atomic_add(1, &ci->io->io_count); - bios = __send_duplicate_bios(ci, ti, 1, NULL, GFP_NOIO); + bios = __send_duplicate_bios(ci, ti, 1, NULL); atomic_sub(1 - bios, &ci->io->io_count); ci->sector_count = 0; From cd6521d03f6a722f18fc027f0d289c39d164dc4f Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Thu, 9 Jan 2025 14:49:11 +0100 Subject: [PATCH 04/17] dm: disable REQ_NOWAIT for flushes REQ_NOWAIT for flushes cannot be easily supported by device mapper because it may allocate multiple bios and its impossible to undo if one of those allocations wants to wait. So, this patch disables REQ_NOWAIT flushes in device mapper and we always return EAGAIN. Previously, the code accepted REQ_NOWAIT flushes, but the non-blocking execution was not guaranteed. Signed-off-by: Mikulas Patocka --- drivers/md/dm.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 4817ef1206a9..ee86cc60d4b8 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1968,6 +1968,15 @@ static void dm_split_and_process_bio(struct mapped_device *md, /* Only support nowait for normal IO */ if (unlikely(bio->bi_opf & REQ_NOWAIT) && !is_abnormal) { + /* + * Don't support NOWAIT for FLUSH because it may allocate + * multiple bios and there's no easy way how to undo the + * allocations. + */ + if (bio->bi_opf & REQ_PREFLUSH) { + bio_wouldblock_error(bio); + return; + } io = alloc_io(md, bio, GFP_NOWAIT); if (unlikely(!io)) { /* Unable to do anything without dm_io. */ From a38425935f7886cef5fe04c796f36715e9d0ef3f Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Wed, 15 Jan 2025 18:33:30 +0100 Subject: [PATCH 05/17] dm-transaction-manager: use red-black trees instead of linear lists There was reported performance degradation when the shadow map contained too many entries [1]. The shadow map uses 256-bucket hash with linear lists - when there are too many entries, it has quadratic complexity. Meir Elisha proposed to add a module parameter that could configure the size of the hash array - however, this is not ideal because users don't know that they should increase the parameter when they get bad performance. This commit replaces the linear lists with rb-trees (so that there's a hash of rb-trees), they have logarithmic complexity, so it solves the performance degradation. Link: https://patchwork.kernel.org/project/dm-devel/patch/20241014134944.1264991-1-meir.elisha@volumez.com/ [1] Reported-by: Meir Elisha Signed-off-by: Mikulas Patocka --- .../persistent-data/dm-transaction-manager.c | 54 +++++++++++++------ 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/drivers/md/persistent-data/dm-transaction-manager.c b/drivers/md/persistent-data/dm-transaction-manager.c index c7ba4e6cbbc7..98c745d90f48 100644 --- a/drivers/md/persistent-data/dm-transaction-manager.c +++ b/drivers/md/persistent-data/dm-transaction-manager.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -77,7 +78,7 @@ static void prefetch_issue(struct prefetch_set *p, struct dm_block_manager *bm) /*----------------------------------------------------------------*/ struct shadow_info { - struct hlist_node hlist; + struct rb_node node; dm_block_t where; }; @@ -95,7 +96,7 @@ struct dm_transaction_manager { struct dm_space_map *sm; spinlock_t lock; - struct hlist_head buckets[DM_HASH_SIZE]; + struct rb_root buckets[DM_HASH_SIZE]; struct prefetch_set prefetches; }; @@ -106,14 +107,22 @@ static int is_shadow(struct dm_transaction_manager *tm, dm_block_t b) { int r = 0; unsigned int bucket = dm_hash_block(b, DM_HASH_MASK); - struct shadow_info *si; + struct rb_node **node; spin_lock(&tm->lock); - hlist_for_each_entry(si, tm->buckets + bucket, hlist) - if (si->where == b) { + node = &tm->buckets[bucket].rb_node; + while (*node) { + struct shadow_info *si = + rb_entry(*node, struct shadow_info, node); + if (b == si->where) { r = 1; break; } + if (b < si->where) + node = &si->node.rb_left; + else + node = &si->node.rb_right; + } spin_unlock(&tm->lock); return r; @@ -130,30 +139,41 @@ static void insert_shadow(struct dm_transaction_manager *tm, dm_block_t b) si = kmalloc(sizeof(*si), GFP_NOIO); if (si) { + struct rb_node **node, *parent; si->where = b; bucket = dm_hash_block(b, DM_HASH_MASK); + spin_lock(&tm->lock); - hlist_add_head(&si->hlist, tm->buckets + bucket); + node = &tm->buckets[bucket].rb_node; + parent = NULL; + while (*node) { + struct shadow_info *si = + rb_entry(*node, struct shadow_info, node); + parent = *node; + if (b < si->where) + node = &si->node.rb_left; + else + node = &si->node.rb_right; + } + rb_link_node(&si->node, parent, node); + rb_insert_color(&si->node, &tm->buckets[bucket]); spin_unlock(&tm->lock); } } static void wipe_shadow_table(struct dm_transaction_manager *tm) { - struct shadow_info *si; - struct hlist_node *tmp; - struct hlist_head *bucket; - int i; + unsigned int i; spin_lock(&tm->lock); for (i = 0; i < DM_HASH_SIZE; i++) { - bucket = tm->buckets + i; - hlist_for_each_entry_safe(si, tmp, bucket, hlist) + while (!RB_EMPTY_ROOT(&tm->buckets[i])) { + struct shadow_info *si = + rb_entry(tm->buckets[i].rb_node, struct shadow_info, node); + rb_erase(&si->node, &tm->buckets[i]); kfree(si); - - INIT_HLIST_HEAD(bucket); + } } - spin_unlock(&tm->lock); } @@ -162,7 +182,7 @@ static void wipe_shadow_table(struct dm_transaction_manager *tm) static struct dm_transaction_manager *dm_tm_create(struct dm_block_manager *bm, struct dm_space_map *sm) { - int i; + unsigned int i; struct dm_transaction_manager *tm; tm = kmalloc(sizeof(*tm), GFP_KERNEL); @@ -176,7 +196,7 @@ static struct dm_transaction_manager *dm_tm_create(struct dm_block_manager *bm, spin_lock_init(&tm->lock); for (i = 0; i < DM_HASH_SIZE; i++) - INIT_HLIST_HEAD(tm->buckets + i); + tm->buckets[i] = RB_ROOT; prefetch_init(&tm->prefetches); From 3194e36488e2dae05f9a822c73eaa367e47b2e3d Mon Sep 17 00:00:00 2001 From: John Garry Date: Thu, 16 Jan 2025 17:02:56 +0000 Subject: [PATCH 06/17] dm-table: atomic writes support Support stacking atomic write limits for DM devices. All the pre-existing code in blk_stack_atomic_writes_limits() already takes care of finding the aggregrate limits from the bottom devices. Feature flag DM_TARGET_ATOMIC_WRITES is introduced so that atomic writes can be enabled on personalities selectively. This is to ensure that atomic writes are only enabled when verified to be working properly (for a specific personality). In addition, it just may not make sense to enable atomic writes on some personalities (so this flag also helps there). Signed-off-by: John Garry Reviewed-by: Mike Snitzer Signed-off-by: Mikulas Patocka --- drivers/md/dm-table.c | 29 +++++++++++++++++++++++++++++ include/linux/device-mapper.h | 3 +++ include/uapi/linux/dm-ioctl.h | 4 ++-- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index bd8b796ae683..0ef5203387b2 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c @@ -1806,6 +1806,32 @@ static bool dm_table_supports_secure_erase(struct dm_table *t) return true; } +static int device_not_atomic_write_capable(struct dm_target *ti, + struct dm_dev *dev, sector_t start, + sector_t len, void *data) +{ + return !bdev_can_atomic_write(dev->bdev); +} + +static bool dm_table_supports_atomic_writes(struct dm_table *t) +{ + for (unsigned int i = 0; i < t->num_targets; i++) { + struct dm_target *ti = dm_table_get_target(t, i); + + if (!dm_target_supports_atomic_writes(ti->type)) + return false; + + if (!ti->type->iterate_devices) + return false; + + if (ti->type->iterate_devices(ti, + device_not_atomic_write_capable, NULL)) { + return false; + } + } + return true; +} + int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q, struct queue_limits *limits) { @@ -1854,6 +1880,9 @@ int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q, return r; } + if (dm_table_supports_atomic_writes(t)) + limits->features |= BLK_FEAT_ATOMIC_WRITES; + r = queue_limits_set(q, limits); if (r) return r; diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h index 8321f65897f3..bcc6d7b69470 100644 --- a/include/linux/device-mapper.h +++ b/include/linux/device-mapper.h @@ -299,6 +299,9 @@ struct target_type { #define dm_target_supports_mixed_zoned_model(type) (false) #endif +#define DM_TARGET_ATOMIC_WRITES 0x00000400 +#define dm_target_supports_atomic_writes(type) ((type)->features & DM_TARGET_ATOMIC_WRITES) + struct dm_target { struct dm_table *table; struct target_type *type; diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h index 1990b5700f69..b08c7378164d 100644 --- a/include/uapi/linux/dm-ioctl.h +++ b/include/uapi/linux/dm-ioctl.h @@ -286,9 +286,9 @@ enum { #define DM_DEV_SET_GEOMETRY _IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD, struct dm_ioctl) #define DM_VERSION_MAJOR 4 -#define DM_VERSION_MINOR 48 +#define DM_VERSION_MINOR 49 #define DM_VERSION_PATCHLEVEL 0 -#define DM_VERSION_EXTRA "-ioctl (2023-03-01)" +#define DM_VERSION_EXTRA "-ioctl (2025-01-17)" /* Status bits */ #define DM_READONLY_FLAG (1 << 0) /* In/Out */ From 5f430e34087b4882545baa3d31962f1f7d735421 Mon Sep 17 00:00:00 2001 From: John Garry Date: Thu, 16 Jan 2025 17:02:57 +0000 Subject: [PATCH 07/17] dm: Ensure cloned bio is same length for atomic write For an atomic write, a cloned bio must be same length as the original bio, i.e. no splitting. Error in case it is not. Per-dm device queue limits should be setup to ensure that this does not happen, but error this case as an insurance policy. Signed-off-by: John Garry Reviewed-by: Mike Snitzer Signed-off-by: Mikulas Patocka --- drivers/md/dm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index ee86cc60d4b8..4d1e42891d24 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1745,6 +1745,9 @@ static blk_status_t __split_and_process_bio(struct clone_info *ci) ci->submit_as_polled = !!(ci->bio->bi_opf & REQ_POLLED); len = min_t(sector_t, max_io_len(ti, ci->sector), ci->sector_count); + if (ci->bio->bi_opf & REQ_ATOMIC && len != ci->sector_count) + return BLK_STS_IOERR; + setup_split_accounting(ci, len); if (unlikely(ci->bio->bi_opf & REQ_NOWAIT)) { From 487d1a9cb586746c03190310056e292e8552539e Mon Sep 17 00:00:00 2001 From: John Garry Date: Thu, 16 Jan 2025 17:02:58 +0000 Subject: [PATCH 08/17] dm-linear: Enable atomic writes Set feature flag DM_TARGET_ATOMIC_WRITES. Signed-off-by: John Garry Reviewed-by: Mike Snitzer Signed-off-by: Mikulas Patocka --- drivers/md/dm-linear.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c index 49fb0f684193..66318aba4bdb 100644 --- a/drivers/md/dm-linear.c +++ b/drivers/md/dm-linear.c @@ -199,9 +199,10 @@ static size_t linear_dax_recovery_write(struct dm_target *ti, pgoff_t pgoff, static struct target_type linear_target = { .name = "linear", - .version = {1, 4, 0}, + .version = {1, 5, 0}, .features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT | - DM_TARGET_ZONED_HM | DM_TARGET_PASSES_CRYPTO, + DM_TARGET_ZONED_HM | DM_TARGET_PASSES_CRYPTO | + DM_TARGET_ATOMIC_WRITES, .report_zones = linear_report_zones, .module = THIS_MODULE, .ctr = linear_ctr, From 30b88ed06f8018b33e034c86d30d67b93f6aca12 Mon Sep 17 00:00:00 2001 From: John Garry Date: Thu, 16 Jan 2025 17:02:59 +0000 Subject: [PATCH 09/17] dm-stripe: Enable atomic writes Set feature flag DM_TARGET_ATOMIC_WRITES. Similar to md raid0, the chunk size set in stripe_io_hints() limits the atomic write unit max. Signed-off-by: John Garry Reviewed-by: Mike Snitzer Signed-off-by: Mikulas Patocka --- drivers/md/dm-stripe.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c index 4112071de0be..3786ac67cefe 100644 --- a/drivers/md/dm-stripe.c +++ b/drivers/md/dm-stripe.c @@ -465,8 +465,9 @@ static void stripe_io_hints(struct dm_target *ti, static struct target_type stripe_target = { .name = "striped", - .version = {1, 6, 0}, - .features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT, + .version = {1, 7, 0}, + .features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT | + DM_TARGET_ATOMIC_WRITES, .module = THIS_MODULE, .ctr = stripe_ctr, .dtr = stripe_dtr, From c6a657d9683def5588aee6ed920cef61415a6a52 Mon Sep 17 00:00:00 2001 From: John Garry Date: Thu, 16 Jan 2025 17:03:00 +0000 Subject: [PATCH 10/17] dm-io: Warn on creating multiple atomic write bios for a region A region should just be for a single atomic write, so warn when we are creating many. This should not occur if request queue limits are properly configured. Signed-off-by: John Garry Reviewed-by: Mike Snitzer Signed-off-by: Mikulas Patocka --- drivers/md/dm-io.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/md/dm-io.c b/drivers/md/dm-io.c index d7a8e2f40db3..c37668790577 100644 --- a/drivers/md/dm-io.c +++ b/drivers/md/dm-io.c @@ -379,6 +379,7 @@ static void do_region(const blk_opf_t opf, unsigned int region, atomic_inc(&io->count); submit_bio(bio); + WARN_ON_ONCE(opf & REQ_ATOMIC && remaining); } while (remaining); } From 6845de78ae3701ed19619cf786695072c73b8e92 Mon Sep 17 00:00:00 2001 From: John Garry Date: Thu, 16 Jan 2025 17:03:01 +0000 Subject: [PATCH 11/17] dm-mirror: Support atomic writes Support atomic writes by setting DM_TARGET_ATOMIC_WRITES for the target type, and also unmasking REQ_ATOMIC from the submitted bio op flags. Signed-off-by: John Garry Reviewed-by: Mike Snitzer Signed-off-by: Mikulas Patocka --- drivers/md/dm-raid1.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c index 9511dae5b556..8c6f1f7e6456 100644 --- a/drivers/md/dm-raid1.c +++ b/drivers/md/dm-raid1.c @@ -656,7 +656,7 @@ static void do_write(struct mirror_set *ms, struct bio *bio) unsigned int i; struct dm_io_region io[MAX_NR_MIRRORS], *dest = io; struct mirror *m; - blk_opf_t op_flags = bio->bi_opf & (REQ_FUA | REQ_PREFLUSH); + blk_opf_t op_flags = bio->bi_opf & (REQ_FUA | REQ_PREFLUSH | REQ_ATOMIC); struct dm_io_request io_req = { .bi_opf = REQ_OP_WRITE | op_flags, .mem.type = DM_IO_BIO, @@ -1483,8 +1483,9 @@ static int mirror_iterate_devices(struct dm_target *ti, static struct target_type mirror_target = { .name = "mirror", - .version = {1, 14, 0}, + .version = {1, 15, 0}, .module = THIS_MODULE, + .features = DM_TARGET_ATOMIC_WRITES, .ctr = mirror_ctr, .dtr = mirror_dtr, .map = mirror_map, From 6fd2cb38c006cbdc4653e7e15e37ab23af59c2de Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Mon, 20 Jan 2025 16:29:46 +0800 Subject: [PATCH 12/17] dm-crypt: set atomic as false when calling crypt_convert() in kworker Both kcryptd_crypt_write_continue() and kcryptd_crypt_read_continue() are running in the kworker context, it is OK to call cond_resched(), Therefore, set atomic as false when invoking crypt_convert() under kworker context. Signed-off-by: Hou Tao Reviewed-by: Ignat Korchagin Signed-off-by: Mikulas Patocka --- drivers/md/dm-crypt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 1ae2c71bb383..424c8fc3f0ad 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -2098,7 +2098,7 @@ static void kcryptd_crypt_write_continue(struct work_struct *work) wait_for_completion(&ctx->restart); reinit_completion(&ctx->restart); - r = crypt_convert(cc, &io->ctx, true, false); + r = crypt_convert(cc, &io->ctx, false, false); if (r) io->error = r; crypt_finished = atomic_dec_and_test(&ctx->cc_pending); @@ -2203,7 +2203,7 @@ static void kcryptd_crypt_read_continue(struct work_struct *work) wait_for_completion(&io->ctx.restart); reinit_completion(&io->ctx.restart); - r = crypt_convert(cc, &io->ctx, true, false); + r = crypt_convert(cc, &io->ctx, false, false); if (r) io->error = r; From 7c88f7cfab553016a1b02a38ba261d9ac3750b6a Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Mon, 20 Jan 2025 16:29:47 +0800 Subject: [PATCH 13/17] dm-crypt: fully initialize clone->bi_iter in crypt_alloc_buffer() Both kcryptd_io_read() and kcryptd_crypt_write_convert() will invoke crypt_alloc_buffer() to allocate a new bio. Both of these two callers initialize bi_iter.bi_sector for the new bio separatedly after crypt_alloc_buffer() returns. However, kcryptd_crypt_write_convert() will copy the bi_iter of the new bio into ctx.iter_out or ctx.iter_in. Although it doesn't incur any harm now, it is better to fully initialize bi_iter before it is used. Therefore, initialize bi_iter.bi_sector in crypt_alloc_buffer() instead. Signed-off-by: Hou Tao Signed-off-by: Mikulas Patocka --- drivers/md/dm-crypt.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 424c8fc3f0ad..551c934bfc50 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1719,6 +1719,7 @@ retry: clone->bi_private = io; clone->bi_end_io = crypt_endio; clone->bi_ioprio = io->base_bio->bi_ioprio; + clone->bi_iter.bi_sector = cc->start + io->sector; remaining_size = size; @@ -1909,7 +1910,6 @@ static int kcryptd_io_read(struct dm_crypt_io *io, gfp_t gfp) crypt_dec_pending(io); return 1; } - clone->bi_iter.bi_sector = cc->start + io->sector; crypt_convert_init(cc, &io->ctx, clone, clone, io->sector); io->saved_bi_iter = clone->bi_iter; dm_submit_bio_remap(io->base_bio, clone); @@ -1925,13 +1925,13 @@ static int kcryptd_io_read(struct dm_crypt_io *io, gfp_t gfp) clone = bio_alloc_clone(cc->dev->bdev, io->base_bio, gfp, &cc->bs); if (!clone) return 1; + + clone->bi_iter.bi_sector = cc->start + io->sector; clone->bi_private = io; clone->bi_end_io = crypt_endio; crypt_inc_pending(io); - clone->bi_iter.bi_sector = cc->start + io->sector; - if (dm_crypt_integrity_io_alloc(io, clone)) { crypt_dec_pending(io); bio_put(clone); @@ -2039,8 +2039,6 @@ static void kcryptd_crypt_write_io_submit(struct dm_crypt_io *io, int async) /* crypt_convert should have filled the clone bio */ BUG_ON(io->ctx.iter_out.bi_size); - clone->bi_iter.bi_sector = cc->start + io->sector; - if ((likely(!async) && test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags)) || test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags)) { dm_submit_bio_remap(io->base_bio, clone); From 2f8c28d0d97313edc36d62cbd505019f36111fd5 Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Mon, 20 Jan 2025 16:29:48 +0800 Subject: [PATCH 14/17] dm-crypt: use bi_sector in bio when initialize integrity seed bio->bi_iter.bi_sector has already been initialized when initialize the integrity seed in dm_crypt_integrity_io_alloc(). There is no need to calculate it again. Therefore, use the helper bip_set_seed() to initialize the seed and pass bi_iter.bi_sector to it instead. Mikulas: We can't use bip_set_seed because it doesn't compile without CONFIG_BLK_DEV_INTEGRITY. Signed-off-by: Hou Tao Signed-off-by: Mikulas Patocka --- drivers/md/dm-crypt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 551c934bfc50..8a4c24d4ee02 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1187,7 +1187,7 @@ static int dm_crypt_integrity_io_alloc(struct dm_crypt_io *io, struct bio *bio) tag_len = io->cc->tuple_size * (bio_sectors(bio) >> io->cc->sector_shift); - bip->bip_iter.bi_sector = io->cc->start + io->sector; + bip->bip_iter.bi_sector = bio->bi_iter.bi_sector; ret = bio_integrity_add_page(bio, virt_to_page(io->integrity_metadata), tag_len, offset_in_page(io->integrity_metadata)); From 9fdbbdbbc92b1474a87b89f8b964892a63734492 Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Mon, 20 Jan 2025 16:29:49 +0800 Subject: [PATCH 15/17] dm-crypt: don't update io->sector after kcryptd_crypt_write_io_submit() The updates of io->sector are the leftovers when dm-crypt allocated pages for partial write request. However, since commit cf2f1abfbd0db ("dm crypt: don't allocate pages for a partial request"), there is no partial request anymore. After the introduction of write request rb-tree, the updates of io->sectors may interfere the insertion procedure, because ->sectors of these write requests which have already been added in the rb-tree may be changed during the insertion of new write request. Fix it by removing these buggy updates of io->sectors. Considering these updates only effect the write request rb-tree, the commit which introduces the write request rb-tree is used as the fix tag. Fixes: b3c5fd305249 ("dm crypt: sort writes") Cc: stable@vger.kernel.org Signed-off-by: Hou Tao Signed-off-by: Mikulas Patocka --- drivers/md/dm-crypt.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 8a4c24d4ee02..605859ef01b5 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -2090,7 +2090,6 @@ static void kcryptd_crypt_write_continue(struct work_struct *work) struct crypt_config *cc = io->cc; struct convert_context *ctx = &io->ctx; int crypt_finished; - sector_t sector = io->sector; blk_status_t r; wait_for_completion(&ctx->restart); @@ -2107,10 +2106,8 @@ static void kcryptd_crypt_write_continue(struct work_struct *work) } /* Encryption was already finished, submit io now */ - if (crypt_finished) { + if (crypt_finished) kcryptd_crypt_write_io_submit(io, 0); - io->sector = sector; - } crypt_dec_pending(io); } @@ -2121,14 +2118,13 @@ static void kcryptd_crypt_write_convert(struct dm_crypt_io *io) struct convert_context *ctx = &io->ctx; struct bio *clone; int crypt_finished; - sector_t sector = io->sector; blk_status_t r; /* * Prevent io from disappearing until this function completes. */ crypt_inc_pending(io); - crypt_convert_init(cc, ctx, NULL, io->base_bio, sector); + crypt_convert_init(cc, ctx, NULL, io->base_bio, io->sector); clone = crypt_alloc_buffer(io, io->base_bio->bi_iter.bi_size); if (unlikely(!clone)) { @@ -2145,8 +2141,6 @@ static void kcryptd_crypt_write_convert(struct dm_crypt_io *io) io->ctx.iter_in = clone->bi_iter; } - sector += bio_sectors(clone); - crypt_inc_pending(io); r = crypt_convert(cc, ctx, test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags), true); @@ -2170,10 +2164,8 @@ static void kcryptd_crypt_write_convert(struct dm_crypt_io *io) } /* Encryption was already finished, submit io now */ - if (crypt_finished) { + if (crypt_finished) kcryptd_crypt_write_io_submit(io, 0); - io->sector = sector; - } dec: crypt_dec_pending(io); From 996c451d982d1f3f110a4639d822abcd433336e7 Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Mon, 20 Jan 2025 16:29:50 +0800 Subject: [PATCH 16/17] dm-crypt: don't initialize cc_sector again For aead_recheck case, cc_sector has already been initialized in crypt_convert_init() when trying to re-read the read. Therefore, remove the duplicated initialization. Signed-off-by: Hou Tao Signed-off-by: Mikulas Patocka --- drivers/md/dm-crypt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 605859ef01b5..09f5c1a04fae 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -2211,7 +2211,6 @@ static void kcryptd_crypt_read_convert(struct dm_crypt_io *io) crypt_inc_pending(io); if (io->ctx.aead_recheck) { - io->ctx.cc_sector = io->sector + cc->iv_offset; r = crypt_convert(cc, &io->ctx, test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags), true); } else { From 8b8f8037765757861f899ed3a2bfb34525b5c065 Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Mon, 20 Jan 2025 16:29:51 +0800 Subject: [PATCH 17/17] dm-crypt: track tag_offset in convert_context dm-crypt uses tag_offset to index the integrity metadata for each crypt sector. When the initial crypt_convert() returns BLK_STS_DEV_RESOURCE, dm-crypt will try to continue the crypt/decrypt procedure in a kworker. However, it resets tag_offset as zero instead of using the tag_offset related with current sector. It may return unexpected data when using random IV or return unexpected integrity related error. Fix the problem by tracking tag_offset in per-IO convert_context. Therefore, when the crypt/decrypt procedure continues in a kworker, it could use the next tag_offset saved in convert_context. Fixes: 8abec36d1274 ("dm crypt: do not wait for backlogged crypto request completion in softirq") Cc: stable@vger.kernel.org Signed-off-by: Hou Tao Signed-off-by: Mikulas Patocka --- drivers/md/dm-crypt.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 09f5c1a04fae..02a2919f4e5a 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -59,6 +59,7 @@ struct convert_context { struct bio *bio_out; struct bvec_iter iter_out; atomic_t cc_pending; + unsigned int tag_offset; u64 cc_sector; union { struct skcipher_request *req; @@ -1256,6 +1257,7 @@ static void crypt_convert_init(struct crypt_config *cc, if (bio_out) ctx->iter_out = bio_out->bi_iter; ctx->cc_sector = sector + cc->iv_offset; + ctx->tag_offset = 0; init_completion(&ctx->restart); } @@ -1588,7 +1590,6 @@ static void crypt_free_req(struct crypt_config *cc, void *req, struct bio *base_ static blk_status_t crypt_convert(struct crypt_config *cc, struct convert_context *ctx, bool atomic, bool reset_pending) { - unsigned int tag_offset = 0; unsigned int sector_step = cc->sector_size >> SECTOR_SHIFT; int r; @@ -1611,9 +1612,9 @@ static blk_status_t crypt_convert(struct crypt_config *cc, atomic_inc(&ctx->cc_pending); if (crypt_integrity_aead(cc)) - r = crypt_convert_block_aead(cc, ctx, ctx->r.req_aead, tag_offset); + r = crypt_convert_block_aead(cc, ctx, ctx->r.req_aead, ctx->tag_offset); else - r = crypt_convert_block_skcipher(cc, ctx, ctx->r.req, tag_offset); + r = crypt_convert_block_skcipher(cc, ctx, ctx->r.req, ctx->tag_offset); switch (r) { /* @@ -1633,8 +1634,8 @@ static blk_status_t crypt_convert(struct crypt_config *cc, * exit and continue processing in a workqueue */ ctx->r.req = NULL; + ctx->tag_offset++; ctx->cc_sector += sector_step; - tag_offset++; return BLK_STS_DEV_RESOURCE; } } else { @@ -1648,8 +1649,8 @@ static blk_status_t crypt_convert(struct crypt_config *cc, */ case -EINPROGRESS: ctx->r.req = NULL; + ctx->tag_offset++; ctx->cc_sector += sector_step; - tag_offset++; continue; /* * The request was already processed (synchronously). @@ -1657,7 +1658,7 @@ static blk_status_t crypt_convert(struct crypt_config *cc, case 0: atomic_dec(&ctx->cc_pending); ctx->cc_sector += sector_step; - tag_offset++; + ctx->tag_offset++; if (!atomic) cond_resched(); continue;