From 0fcb100d50835d6823723ef0898cd565b3796e0a Mon Sep 17 00:00:00 2001 From: Nathan Huckleberry Date: Fri, 22 Jul 2022 09:38:21 +0000 Subject: dm bufio: Add flags argument to dm_bufio_client_create Add a flags argument to dm_bufio_client_create and update all the callers. This is in preparation to add the DM_BUFIO_NO_SLEEP flag. Signed-off-by: Nathan Huckleberry Signed-off-by: Mike Snitzer --- drivers/md/dm-bufio.c | 3 ++- drivers/md/dm-ebs-target.c | 3 ++- drivers/md/dm-integrity.c | 2 +- drivers/md/dm-snap-persistent.c | 2 +- drivers/md/dm-verity-fec.c | 4 ++-- drivers/md/dm-verity-target.c | 2 +- drivers/md/persistent-data/dm-block-manager.c | 3 ++- 7 files changed, 11 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index 5ffa1dcf84cf..ad5603eb12e3 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -1717,7 +1717,8 @@ static unsigned long dm_bufio_shrink_count(struct shrinker *shrink, struct shrin struct dm_bufio_client *dm_bufio_client_create(struct block_device *bdev, unsigned block_size, unsigned reserved_buffers, unsigned aux_size, void (*alloc_callback)(struct dm_buffer *), - void (*write_callback)(struct dm_buffer *)) + void (*write_callback)(struct dm_buffer *), + unsigned int flags) { int r; struct dm_bufio_client *c; diff --git a/drivers/md/dm-ebs-target.c b/drivers/md/dm-ebs-target.c index 0221fa63f888..04a1f56d6588 100644 --- a/drivers/md/dm-ebs-target.c +++ b/drivers/md/dm-ebs-target.c @@ -312,7 +312,8 @@ static int ebs_ctr(struct dm_target *ti, unsigned int argc, char **argv) goto bad; } - ec->bufio = dm_bufio_client_create(ec->dev->bdev, to_bytes(ec->u_bs), 1, 0, NULL, NULL); + ec->bufio = dm_bufio_client_create(ec->dev->bdev, to_bytes(ec->u_bs), 1, + 0, NULL, NULL, 0); if (IS_ERR(ec->bufio)) { ti->error = "Cannot create dm bufio client"; r = PTR_ERR(ec->bufio); diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c index 3d5a0ce123c9..a508073d8414 100644 --- a/drivers/md/dm-integrity.c +++ b/drivers/md/dm-integrity.c @@ -4439,7 +4439,7 @@ try_smaller_buffer: } ic->bufio = dm_bufio_client_create(ic->meta_dev ? ic->meta_dev->bdev : ic->dev->bdev, - 1U << (SECTOR_SHIFT + ic->log2_buffer_sectors), 1, 0, NULL, NULL); + 1U << (SECTOR_SHIFT + ic->log2_buffer_sectors), 1, 0, NULL, NULL, 0); if (IS_ERR(ic->bufio)) { r = PTR_ERR(ic->bufio); ti->error = "Cannot initialize dm-bufio"; diff --git a/drivers/md/dm-snap-persistent.c b/drivers/md/dm-snap-persistent.c index 3bb5cff5d6fc..aaa699749c3b 100644 --- a/drivers/md/dm-snap-persistent.c +++ b/drivers/md/dm-snap-persistent.c @@ -494,7 +494,7 @@ static int read_exceptions(struct pstore *ps, client = dm_bufio_client_create(dm_snap_cow(ps->store->snap)->bdev, ps->store->chunk_size << SECTOR_SHIFT, - 1, 0, NULL, NULL); + 1, 0, NULL, NULL, 0); if (IS_ERR(client)) return PTR_ERR(client); diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c index cea2b3789736..23cffce56403 100644 --- a/drivers/md/dm-verity-fec.c +++ b/drivers/md/dm-verity-fec.c @@ -749,7 +749,7 @@ int verity_fec_ctr(struct dm_verity *v) f->bufio = dm_bufio_client_create(f->dev->bdev, f->io_size, - 1, 0, NULL, NULL); + 1, 0, NULL, NULL, 0); if (IS_ERR(f->bufio)) { ti->error = "Cannot initialize FEC bufio client"; return PTR_ERR(f->bufio); @@ -765,7 +765,7 @@ int verity_fec_ctr(struct dm_verity *v) f->data_bufio = dm_bufio_client_create(v->data_dev->bdev, 1 << v->data_dev_block_bits, - 1, 0, NULL, NULL); + 1, 0, NULL, NULL, 0); if (IS_ERR(f->data_bufio)) { ti->error = "Cannot initialize FEC data bufio client"; return PTR_ERR(f->data_bufio); diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index 75b66dd67633..95db3a7ee3c7 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -1265,7 +1265,7 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) v->bufio = dm_bufio_client_create(v->hash_dev->bdev, 1 << v->hash_dev_block_bits, 1, sizeof(struct buffer_aux), - dm_bufio_alloc_callback, NULL); + dm_bufio_alloc_callback, NULL, 0); if (IS_ERR(v->bufio)) { ti->error = "Cannot initialize dm-bufio"; r = PTR_ERR(v->bufio); diff --git a/drivers/md/persistent-data/dm-block-manager.c b/drivers/md/persistent-data/dm-block-manager.c index 54c089a50b15..11935864f50f 100644 --- a/drivers/md/persistent-data/dm-block-manager.c +++ b/drivers/md/persistent-data/dm-block-manager.c @@ -391,7 +391,8 @@ struct dm_block_manager *dm_block_manager_create(struct block_device *bdev, bm->bufio = dm_bufio_client_create(bdev, block_size, max_held_per_thread, sizeof(struct buffer_aux), dm_block_manager_alloc_callback, - dm_block_manager_write_callback); + dm_block_manager_write_callback, + 0); if (IS_ERR(bm->bufio)) { r = PTR_ERR(bm->bufio); kfree(bm); -- cgit v1.2.3 From b32d45824aa7e07a0c3257a16e3a2a691b11b39a Mon Sep 17 00:00:00 2001 From: Nathan Huckleberry Date: Fri, 22 Jul 2022 09:38:22 +0000 Subject: dm bufio: Add DM_BUFIO_CLIENT_NO_SLEEP flag Add an optional flag that ensures dm_bufio_client does not sleep (primary focus is to service dm_bufio_get without sleeping). This allows the dm-bufio cache to be queried from interrupt context. To ensure that dm-bufio does not sleep, dm-bufio must use a spinlock instead of a mutex. Additionally, to avoid deadlocks, special care must be taken so that dm-bufio does not sleep while holding the spinlock. But again: the scope of this no_sleep is initially confined to dm_bufio_get, so __alloc_buffer_wait_no_callback is _not_ changed to avoid sleeping because __bufio_new avoids allocation for NF_GET. Signed-off-by: Nathan Huckleberry Signed-off-by: Mike Snitzer --- drivers/md/dm-bufio.c | 22 +++++++++++++++++++--- include/linux/dm-bufio.h | 5 +++++ 2 files changed, 24 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index ad5603eb12e3..486179d1831c 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -81,6 +81,8 @@ */ struct dm_bufio_client { struct mutex lock; + spinlock_t spinlock; + unsigned long spinlock_flags; struct list_head lru[LIST_SIZE]; unsigned long n_buffers[LIST_SIZE]; @@ -90,6 +92,7 @@ struct dm_bufio_client { s8 sectors_per_block_bits; void (*alloc_callback)(struct dm_buffer *); void (*write_callback)(struct dm_buffer *); + bool no_sleep; struct kmem_cache *slab_buffer; struct kmem_cache *slab_cache; @@ -167,17 +170,26 @@ struct dm_buffer { static void dm_bufio_lock(struct dm_bufio_client *c) { - mutex_lock_nested(&c->lock, dm_bufio_in_request()); + if (c->no_sleep) + spin_lock_irqsave_nested(&c->spinlock, c->spinlock_flags, dm_bufio_in_request()); + else + mutex_lock_nested(&c->lock, dm_bufio_in_request()); } static int dm_bufio_trylock(struct dm_bufio_client *c) { - return mutex_trylock(&c->lock); + if (c->no_sleep) + return spin_trylock_irqsave(&c->spinlock, c->spinlock_flags); + else + return mutex_trylock(&c->lock); } static void dm_bufio_unlock(struct dm_bufio_client *c) { - mutex_unlock(&c->lock); + if (c->no_sleep) + spin_unlock_irqrestore(&c->spinlock, c->spinlock_flags); + else + mutex_unlock(&c->lock); } /*----------------------------------------------------------------*/ @@ -1748,12 +1760,16 @@ struct dm_bufio_client *dm_bufio_client_create(struct block_device *bdev, unsign c->alloc_callback = alloc_callback; c->write_callback = write_callback; + if (flags & DM_BUFIO_CLIENT_NO_SLEEP) + c->no_sleep = true; + for (i = 0; i < LIST_SIZE; i++) { INIT_LIST_HEAD(&c->lru[i]); c->n_buffers[i] = 0; } mutex_init(&c->lock); + spin_lock_init(&c->spinlock); INIT_LIST_HEAD(&c->reserved_buffers); c->need_reserved_buffers = reserved_buffers; diff --git a/include/linux/dm-bufio.h b/include/linux/dm-bufio.h index e21480715255..15d9e15ca830 100644 --- a/include/linux/dm-bufio.h +++ b/include/linux/dm-bufio.h @@ -17,6 +17,11 @@ struct dm_bufio_client; struct dm_buffer; +/* + * Flags for dm_bufio_client_create + */ +#define DM_BUFIO_CLIENT_NO_SLEEP 0x1 + /* * Create a buffered IO cache on a given device */ -- cgit v1.2.3 From 5721d4e5a9cdb148f681a004ae5748890a0e2d90 Mon Sep 17 00:00:00 2001 From: Nathan Huckleberry Date: Fri, 22 Jul 2022 09:38:23 +0000 Subject: dm verity: Add optional "try_verify_in_tasklet" feature Using tasklets for disk verification can reduce IO latency. When there are accelerated hash instructions it is often better to compute the hash immediately using a tasklet rather than deferring verification to a work-queue. This reduces time spent waiting to schedule work-queue jobs, but requires spending slightly more time in interrupt context. If the dm-bufio cache does not have the required hashes we fallback to the work-queue implementation. FEC is only possible using work-queue because code to support the FEC feature may sleep. The following shows a speed comparison of random reads on a dm-verity device. The dm-verity device uses a 1G ramdisk for data and a 1G ramdisk for hashes. One test was run using tasklets and one test was run using the existing work-queue solution. Both tests were run when the dm-bufio cache was hot. The tasklet implementation performs significantly better since there is no time spent waiting for work-queue jobs to be scheduled. READ: bw=181MiB/s (190MB/s), 181MiB/s-181MiB/s (190MB/s-190MB/s), io=512MiB (537MB), run=2827-2827msec READ: bw=23.6MiB/s (24.8MB/s), 23.6MiB/s-23.6MiB/s (24.8MB/s-24.8MB/s), io=512MiB (537MB), run=21688-21688msec Signed-off-by: Nathan Huckleberry Signed-off-by: Mike Snitzer --- drivers/md/dm-verity-target.c | 103 +++++++++++++++++++++++++++++++++++------- drivers/md/dm-verity.h | 6 ++- 2 files changed, 91 insertions(+), 18 deletions(-) (limited to 'drivers') diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index 95db3a7ee3c7..44af4faa30ab 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -34,6 +34,7 @@ #define DM_VERITY_OPT_PANIC "panic_on_corruption" #define DM_VERITY_OPT_IGN_ZEROES "ignore_zero_blocks" #define DM_VERITY_OPT_AT_MOST_ONCE "check_at_most_once" +#define DM_VERITY_OPT_TASKLET_VERIFY "try_verify_in_tasklet" #define DM_VERITY_OPTS_MAX (3 + DM_VERITY_OPTS_FEC + \ DM_VERITY_ROOT_HASH_VERIFICATION_OPTS) @@ -220,7 +221,7 @@ static int verity_handle_err(struct dm_verity *v, enum verity_block_type type, struct mapped_device *md = dm_table_get_md(v->ti->table); /* Corruption should be visible in device status in all modes */ - v->hash_failed = 1; + v->hash_failed = true; if (v->corrupted_errs >= DM_VERITY_MAX_CORRUPTED_ERRS) goto out; @@ -286,7 +287,19 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io, verity_hash_at_level(v, block, level, &hash_block, &offset); - data = dm_bufio_read(v->bufio, hash_block, &buf); + if (io->in_tasklet) { + data = dm_bufio_get(v->bufio, hash_block, &buf); + if (data == NULL) { + /* + * In tasklet and the hash was not in the bufio cache. + * Return early and resume execution from a work-queue + * to read the hash from disk. + */ + return -EAGAIN; + } + } else + data = dm_bufio_read(v->bufio, hash_block, &buf); + if (IS_ERR(data)) return PTR_ERR(data); @@ -307,6 +320,14 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io, if (likely(memcmp(verity_io_real_digest(v, io), want_digest, v->digest_size) == 0)) aux->hash_verified = 1; + else if (io->in_tasklet) { + /* + * Error handling code (FEC included) cannot be run in a + * tasklet since it may sleep, so fallback to work-queue. + */ + r = -EAGAIN; + goto release_ret_r; + } else if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_METADATA, hash_block, data, NULL) == 0) @@ -474,9 +495,14 @@ static int verity_verify_io(struct dm_verity_io *io) bool is_zero; struct dm_verity *v = io->v; struct bvec_iter start; - unsigned b; + /* + * Copy the iterator in case we need to restart verification in a + * work-queue. + */ + struct bvec_iter iter_copy = io->iter; struct crypto_wait wait; struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size); + unsigned int b; for (b = 0; b < io->n_blocks; b++) { int r; @@ -485,7 +511,7 @@ static int verity_verify_io(struct dm_verity_io *io) if (v->validated_blocks && likely(test_bit(cur_block, v->validated_blocks))) { - verity_bv_skip_block(v, io, &io->iter); + verity_bv_skip_block(v, io, &iter_copy); continue; } @@ -500,7 +526,7 @@ static int verity_verify_io(struct dm_verity_io *io) * If we expect a zero block, don't validate, just * return zeros. */ - r = verity_for_bv_block(v, io, &io->iter, + r = verity_for_bv_block(v, io, &iter_copy, verity_bv_zero); if (unlikely(r < 0)) return r; @@ -512,8 +538,8 @@ static int verity_verify_io(struct dm_verity_io *io) if (unlikely(r < 0)) return r; - start = io->iter; - r = verity_for_io_block(v, io, &io->iter, &wait); + start = iter_copy; + r = verity_for_io_block(v, io, &iter_copy, &wait); if (unlikely(r < 0)) return r; @@ -527,8 +553,14 @@ static int verity_verify_io(struct dm_verity_io *io) if (v->validated_blocks) set_bit(cur_block, v->validated_blocks); continue; + } else if (io->in_tasklet) { + /* + * Error handling code (FEC included) cannot be run in a + * tasklet since it may sleep, so fallback to work-queue. + */ + return -EAGAIN; } else if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_DATA, - cur_block, NULL, &start) == 0) { + cur_block, NULL, &start) == 0) { continue; } else { if (bio->bi_status) { @@ -566,7 +598,8 @@ static void verity_finish_io(struct dm_verity_io *io, blk_status_t status) bio->bi_end_io = io->orig_bi_end_io; bio->bi_status = status; - verity_fec_finish_io(io); + if (!io->in_tasklet) + verity_fec_finish_io(io); bio_endio(bio); } @@ -575,9 +608,29 @@ static void verity_work(struct work_struct *w) { struct dm_verity_io *io = container_of(w, struct dm_verity_io, work); + io->in_tasklet = false; + + verity_fec_init_io(io); verity_finish_io(io, errno_to_blk_status(verity_verify_io(io))); } +static void verity_tasklet(unsigned long data) +{ + struct dm_verity_io *io = (struct dm_verity_io *)data; + int err; + + io->in_tasklet = true; + err = verity_verify_io(io); + if (err == -EAGAIN) { + /* fallback to retrying with work-queue */ + INIT_WORK(&io->work, verity_work); + queue_work(io->v->verify_wq, &io->work); + return; + } + + verity_finish_io(io, errno_to_blk_status(err)); +} + static void verity_end_io(struct bio *bio) { struct dm_verity_io *io = bio->bi_private; @@ -588,8 +641,13 @@ static void verity_end_io(struct bio *bio) return; } - INIT_WORK(&io->work, verity_work); - queue_work(io->v->verify_wq, &io->work); + if (io->v->use_tasklet) { + tasklet_init(&io->tasklet, verity_tasklet, (unsigned long)io); + tasklet_schedule(&io->tasklet); + } else { + INIT_WORK(&io->work, verity_work); + queue_work(io->v->verify_wq, &io->work); + } } /* @@ -700,8 +758,6 @@ static int verity_map(struct dm_target *ti, struct bio *bio) bio->bi_private = io; io->iter = bio->bi_iter; - verity_fec_init_io(io); - verity_submit_prefetch(v, io); submit_bio_noacct(bio); @@ -751,6 +807,8 @@ static void verity_status(struct dm_target *ti, status_type_t type, args++; if (v->validated_blocks) args++; + if (v->use_tasklet) + args++; if (v->signature_key_desc) args += DM_VERITY_ROOT_HASH_VERIFICATION_OPTS; if (!args) @@ -776,6 +834,8 @@ static void verity_status(struct dm_target *ti, status_type_t type, DMEMIT(" " DM_VERITY_OPT_IGN_ZEROES); if (v->validated_blocks) DMEMIT(" " DM_VERITY_OPT_AT_MOST_ONCE); + if (v->use_tasklet) + DMEMIT(" " DM_VERITY_OPT_TASKLET_VERIFY); sz = verity_fec_status_table(v, sz, result, maxlen); if (v->signature_key_desc) DMEMIT(" " DM_VERITY_ROOT_HASH_VERIFICATION_OPT_SIG_KEY @@ -1011,11 +1071,16 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, return r; continue; + } else if (!strcasecmp(arg_name, DM_VERITY_OPT_TASKLET_VERIFY)) { + v->use_tasklet = true; + continue; + } else if (verity_is_fec_opt_arg(arg_name)) { r = verity_fec_parse_opt_args(as, v, &argc, arg_name); if (r) return r; continue; + } else if (verity_verify_is_sig_opt_arg(arg_name)) { r = verity_verify_sig_parse_opt_args(as, v, verify_args, @@ -1023,7 +1088,6 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, if (r) return r; continue; - } ti->error = "Unrecognized verity feature request"; @@ -1155,7 +1219,11 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) goto bad; } - v->tfm = crypto_alloc_ahash(v->alg_name, 0, 0); + /* + * FIXME: CRYPTO_ALG_ASYNC should be conditional on v->use_tasklet + * but verity_parse_opt_args() happens below and has data dep on tfm. + */ + v->tfm = crypto_alloc_ahash(v->alg_name, 0, CRYPTO_ALG_ASYNC); if (IS_ERR(v->tfm)) { ti->error = "Cannot initialize hash function"; r = PTR_ERR(v->tfm); @@ -1265,7 +1333,8 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) v->bufio = dm_bufio_client_create(v->hash_dev->bdev, 1 << v->hash_dev_block_bits, 1, sizeof(struct buffer_aux), - dm_bufio_alloc_callback, NULL, 0); + dm_bufio_alloc_callback, NULL, + v->use_tasklet ? DM_BUFIO_CLIENT_NO_SLEEP : 0); if (IS_ERR(v->bufio)) { ti->error = "Cannot initialize dm-bufio"; r = PTR_ERR(v->bufio); @@ -1312,7 +1381,7 @@ bad: static struct target_type verity_target = { .name = "verity", .features = DM_TARGET_IMMUTABLE, - .version = {1, 8, 0}, + .version = {1, 9, 0}, .module = THIS_MODULE, .ctr = verity_ctr, .dtr = verity_dtr, diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h index 4e769d13473a..fb92b22a6404 100644 --- a/drivers/md/dm-verity.h +++ b/drivers/md/dm-verity.h @@ -13,6 +13,7 @@ #include #include +#include #include #define DM_VERITY_MAX_LEVELS 63 @@ -51,9 +52,10 @@ struct dm_verity { unsigned char hash_per_block_bits; /* log2(hashes in hash block) */ unsigned char levels; /* the number of tree levels */ unsigned char version; + bool hash_failed:1; /* set if hash of any block failed */ + bool use_tasklet:1; /* try to verify in tasklet before work-queue */ unsigned digest_size; /* digest size for the current hash algorithm */ unsigned int ahash_reqsize;/* the size of temporary space for crypto */ - int hash_failed; /* set to 1 if hash of any block failed */ enum verity_mode mode; /* mode for handling verification errors */ unsigned corrupted_errs;/* Number of errors for corrupted blocks */ @@ -76,10 +78,12 @@ struct dm_verity_io { sector_t block; unsigned n_blocks; + bool in_tasklet; struct bvec_iter iter; struct work_struct work; + struct tasklet_struct tasklet; /* * Three variably-size fields follow this struct: -- cgit v1.2.3 From df326e7a06990bab011afc8c17de1ab2774e4bb8 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Tue, 26 Jul 2022 11:29:50 -0400 Subject: dm verity: allow optional args to alter primary args handling The previous commit ("dm verity: Add optional "try_verify_in_tasklet" feature") imposed that CRYPTO_ALG_ASYNC mask be used even if the optional "try_verify_in_tasklet" feature was not specified. This was because verity_parse_opt_args() was called after handling the primary args (due to it having data dependencies on having first parsed all primary args). Enhance verity_ctr() so that simple optional args, that don't have a data dependency on primary args parsing, can alter how the primary args are handled. In practice this means verity_parse_opt_args() gets called twice. First with the new 'only_modifier_opts' arg set to true, then again with it set to false _after_ parsing all primary args. This allows the v->use_tasklet flag to be properly set and then used when verity_ctr() parses the primary args and then calls crypto_alloc_ahash() with CRYPTO_ALG_ASYNC conditionally set. Signed-off-by: Mike Snitzer --- drivers/md/dm-verity-target.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index 44af4faa30ab..4895d6b30559 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -1027,7 +1027,8 @@ static int verity_parse_verity_mode(struct dm_verity *v, const char *arg_name) } static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, - struct dm_verity_sig_opts *verify_args) + struct dm_verity_sig_opts *verify_args, + bool only_modifier_opts) { int r; unsigned argc; @@ -1050,6 +1051,8 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, argc--; if (verity_is_verity_mode(arg_name)) { + if (only_modifier_opts) + continue; r = verity_parse_verity_mode(v, arg_name); if (r) { ti->error = "Conflicting error handling parameters"; @@ -1058,6 +1061,8 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, continue; } else if (!strcasecmp(arg_name, DM_VERITY_OPT_IGN_ZEROES)) { + if (only_modifier_opts) + continue; r = verity_alloc_zero_digest(v); if (r) { ti->error = "Cannot allocate zero digest"; @@ -1066,6 +1071,8 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, continue; } else if (!strcasecmp(arg_name, DM_VERITY_OPT_AT_MOST_ONCE)) { + if (only_modifier_opts) + continue; r = verity_alloc_most_once(v); if (r) return r; @@ -1076,12 +1083,16 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, continue; } else if (verity_is_fec_opt_arg(arg_name)) { + if (only_modifier_opts) + continue; r = verity_fec_parse_opt_args(as, v, &argc, arg_name); if (r) return r; continue; } else if (verity_verify_is_sig_opt_arg(arg_name)) { + if (only_modifier_opts) + continue; r = verity_verify_sig_parse_opt_args(as, v, verify_args, &argc, arg_name); @@ -1148,6 +1159,15 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) goto bad; } + /* Parse optional parameters that modify primary args */ + if (argc > 10) { + as.argc = argc - 10; + as.argv = argv + 10; + r = verity_parse_opt_args(&as, v, &verify_args, true); + if (r < 0) + goto bad; + } + if (sscanf(argv[0], "%u%c", &num, &dummy) != 1 || num > 1) { ti->error = "Invalid version"; @@ -1219,11 +1239,8 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) goto bad; } - /* - * FIXME: CRYPTO_ALG_ASYNC should be conditional on v->use_tasklet - * but verity_parse_opt_args() happens below and has data dep on tfm. - */ - v->tfm = crypto_alloc_ahash(v->alg_name, 0, CRYPTO_ALG_ASYNC); + v->tfm = crypto_alloc_ahash(v->alg_name, 0, + v->use_tasklet ? CRYPTO_ALG_ASYNC : 0); if (IS_ERR(v->tfm)) { ti->error = "Cannot initialize hash function"; r = PTR_ERR(v->tfm); @@ -1285,8 +1302,7 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) if (argc) { as.argc = argc; as.argv = argv; - - r = verity_parse_opt_args(&as, v, &verify_args); + r = verity_parse_opt_args(&as, v, &verify_args, false); if (r < 0) goto bad; } -- cgit v1.2.3 From 3c1c875d058669b4dee68f877ec8a65ad4ae8b5b Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Mon, 25 Jul 2022 18:26:48 -0400 Subject: dm bufio: conditionally enable branching for DM_BUFIO_CLIENT_NO_SLEEP Use jump_label to limit the need for branching unless the optional DM_BUFIO_CLIENT_NO_SLEEP is used. Signed-off-by: Mike Snitzer --- drivers/md/dm-bufio.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index 486179d1831c..0a38a7aef49c 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -18,6 +18,7 @@ #include #include #include +#include #define DM_MSG_PREFIX "bufio" @@ -164,13 +165,15 @@ struct dm_buffer { #endif }; +static DEFINE_STATIC_KEY_FALSE(no_sleep_enabled); + /*----------------------------------------------------------------*/ #define dm_bufio_in_request() (!!current->bio_list) static void dm_bufio_lock(struct dm_bufio_client *c) { - if (c->no_sleep) + if (static_branch_unlikely(&no_sleep_enabled) && c->no_sleep) spin_lock_irqsave_nested(&c->spinlock, c->spinlock_flags, dm_bufio_in_request()); else mutex_lock_nested(&c->lock, dm_bufio_in_request()); @@ -178,7 +181,7 @@ static void dm_bufio_lock(struct dm_bufio_client *c) static int dm_bufio_trylock(struct dm_bufio_client *c) { - if (c->no_sleep) + if (static_branch_unlikely(&no_sleep_enabled) && c->no_sleep) return spin_trylock_irqsave(&c->spinlock, c->spinlock_flags); else return mutex_trylock(&c->lock); @@ -186,7 +189,7 @@ static int dm_bufio_trylock(struct dm_bufio_client *c) static void dm_bufio_unlock(struct dm_bufio_client *c) { - if (c->no_sleep) + if (static_branch_unlikely(&no_sleep_enabled) && c->no_sleep) spin_unlock_irqrestore(&c->spinlock, c->spinlock_flags); else mutex_unlock(&c->lock); @@ -1760,8 +1763,10 @@ struct dm_bufio_client *dm_bufio_client_create(struct block_device *bdev, unsign c->alloc_callback = alloc_callback; c->write_callback = write_callback; - if (flags & DM_BUFIO_CLIENT_NO_SLEEP) + if (flags & DM_BUFIO_CLIENT_NO_SLEEP) { c->no_sleep = true; + static_branch_inc(&no_sleep_enabled); + } for (i = 0; i < LIST_SIZE; i++) { INIT_LIST_HEAD(&c->lru[i]); @@ -1895,6 +1900,8 @@ void dm_bufio_client_destroy(struct dm_bufio_client *c) kmem_cache_destroy(c->slab_buffer); dm_io_client_destroy(c->dm_io); mutex_destroy(&c->lock); + if (c->no_sleep) + static_branch_dec(&no_sleep_enabled); kfree(c); } EXPORT_SYMBOL_GPL(dm_bufio_client_destroy); -- cgit v1.2.3 From ba2cce82ba1ba74cd83bb3fd0e47849af4f2a605 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Mon, 25 Jul 2022 16:52:17 -0400 Subject: dm verity: conditionally enable branching for "try_verify_in_tasklet" Use jump_label to limit the need for branching unless the optional "try_verify_in_tasklet" feature is used. Signed-off-by: Mike Snitzer --- drivers/md/dm-verity-target.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index 4895d6b30559..d287d01b7684 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -19,6 +19,7 @@ #include #include #include +#include #define DM_MSG_PREFIX "verity" @@ -43,6 +44,8 @@ static unsigned dm_verity_prefetch_cluster = DM_VERITY_DEFAULT_PREFETCH_SIZE; module_param_named(prefetch_cluster, dm_verity_prefetch_cluster, uint, S_IRUGO | S_IWUSR); +static DEFINE_STATIC_KEY_FALSE(use_tasklet_enabled); + struct dm_verity_prefetch_work { struct work_struct work; struct dm_verity *v; @@ -287,7 +290,7 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io, verity_hash_at_level(v, block, level, &hash_block, &offset); - if (io->in_tasklet) { + if (static_branch_unlikely(&use_tasklet_enabled) && io->in_tasklet) { data = dm_bufio_get(v->bufio, hash_block, &buf); if (data == NULL) { /* @@ -320,7 +323,8 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io, if (likely(memcmp(verity_io_real_digest(v, io), want_digest, v->digest_size) == 0)) aux->hash_verified = 1; - else if (io->in_tasklet) { + else if (static_branch_unlikely(&use_tasklet_enabled) && + io->in_tasklet) { /* * Error handling code (FEC included) cannot be run in a * tasklet since it may sleep, so fallback to work-queue. @@ -553,7 +557,8 @@ static int verity_verify_io(struct dm_verity_io *io) if (v->validated_blocks) set_bit(cur_block, v->validated_blocks); continue; - } else if (io->in_tasklet) { + } else if (static_branch_unlikely(&use_tasklet_enabled) && + io->in_tasklet) { /* * Error handling code (FEC included) cannot be run in a * tasklet since it may sleep, so fallback to work-queue. @@ -598,7 +603,7 @@ static void verity_finish_io(struct dm_verity_io *io, blk_status_t status) bio->bi_end_io = io->orig_bi_end_io; bio->bi_status = status; - if (!io->in_tasklet) + if (!static_branch_unlikely(&use_tasklet_enabled) || !io->in_tasklet) verity_fec_finish_io(io); bio_endio(bio); @@ -641,7 +646,7 @@ static void verity_end_io(struct bio *bio) return; } - if (io->v->use_tasklet) { + if (static_branch_unlikely(&use_tasklet_enabled) && io->v->use_tasklet) { tasklet_init(&io->tasklet, verity_tasklet, (unsigned long)io); tasklet_schedule(&io->tasklet); } else { @@ -949,6 +954,9 @@ static void verity_dtr(struct dm_target *ti) kfree(v->signature_key_desc); + if (v->use_tasklet) + static_branch_dec(&use_tasklet_enabled); + kfree(v); } @@ -1080,6 +1088,7 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, } else if (!strcasecmp(arg_name, DM_VERITY_OPT_TASKLET_VERIFY)) { v->use_tasklet = true; + static_branch_inc(&use_tasklet_enabled); continue; } else if (verity_is_fec_opt_arg(arg_name)) { -- cgit v1.2.3 From 0a36463f4ca287e4d4ac15580c0aae5b23619212 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Wed, 3 Aug 2022 22:43:22 -0400 Subject: dm verity: optimize verity_verify_io if FEC not configured Only declare and copy bvec_iter if CONFIG_DM_VERITY_FEC is defined and FEC enabled for the verity device. Signed-off-by: Mike Snitzer --- drivers/md/dm-verity-target.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index d287d01b7684..5a4ee3292853 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -498,7 +498,9 @@ static int verity_verify_io(struct dm_verity_io *io) { bool is_zero; struct dm_verity *v = io->v; +#if defined(CONFIG_DM_VERITY_FEC) struct bvec_iter start; +#endif /* * Copy the iterator in case we need to restart verification in a * work-queue. @@ -542,7 +544,10 @@ static int verity_verify_io(struct dm_verity_io *io) if (unlikely(r < 0)) return r; - start = iter_copy; +#if defined(CONFIG_DM_VERITY_FEC) + if (verity_fec_is_enabled(v)) + start = iter_copy; +#endif r = verity_for_io_block(v, io, &iter_copy, &wait); if (unlikely(r < 0)) return r; @@ -564,9 +569,11 @@ static int verity_verify_io(struct dm_verity_io *io) * tasklet since it may sleep, so fallback to work-queue. */ return -EAGAIN; +#if defined(CONFIG_DM_VERITY_FEC) } else if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_DATA, cur_block, NULL, &start) == 0) { continue; +#endif } else { if (bio->bi_status) { /* -- cgit v1.2.3 From e9307e3deb52603c4f742624fa4799c7996422d8 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Thu, 4 Aug 2022 13:37:53 -0400 Subject: dm verity: only copy bvec_iter in verity_verify_io if in_tasklet Avoid extra bvec_iter copy unless it is needed to allow retrying verification, that failed from a tasklet, from a workqueue. Signed-off-by: Mike Snitzer --- drivers/md/dm-verity-target.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index 5a4ee3292853..a56e254214fa 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -501,15 +501,22 @@ static int verity_verify_io(struct dm_verity_io *io) #if defined(CONFIG_DM_VERITY_FEC) struct bvec_iter start; #endif - /* - * Copy the iterator in case we need to restart verification in a - * work-queue. - */ - struct bvec_iter iter_copy = io->iter; + struct bvec_iter iter_copy; + struct bvec_iter *iter; struct crypto_wait wait; struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size); unsigned int b; + if (static_branch_unlikely(&use_tasklet_enabled) && io->in_tasklet) { + /* + * Copy the iterator in case we need to restart + * verification in a work-queue. + */ + iter_copy = io->iter; + iter = &iter_copy; + } else + iter = &io->iter; + for (b = 0; b < io->n_blocks; b++) { int r; sector_t cur_block = io->block + b; @@ -517,7 +524,7 @@ static int verity_verify_io(struct dm_verity_io *io) if (v->validated_blocks && likely(test_bit(cur_block, v->validated_blocks))) { - verity_bv_skip_block(v, io, &iter_copy); + verity_bv_skip_block(v, io, iter); continue; } @@ -532,7 +539,7 @@ static int verity_verify_io(struct dm_verity_io *io) * If we expect a zero block, don't validate, just * return zeros. */ - r = verity_for_bv_block(v, io, &iter_copy, + r = verity_for_bv_block(v, io, iter, verity_bv_zero); if (unlikely(r < 0)) return r; @@ -546,9 +553,9 @@ static int verity_verify_io(struct dm_verity_io *io) #if defined(CONFIG_DM_VERITY_FEC) if (verity_fec_is_enabled(v)) - start = iter_copy; + start = *iter; #endif - r = verity_for_io_block(v, io, &iter_copy, &wait); + r = verity_for_io_block(v, io, iter, &wait); if (unlikely(r < 0)) return r; -- cgit v1.2.3 From 43fa47cb116daa48f731fa5c00d781200addb2ae Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Thu, 4 Aug 2022 14:53:07 -0400 Subject: dm verity: remove WQ_CPU_INTENSIVE flag since using WQ_UNBOUND The documentation [1] says that WQ_CPU_INTENSIVE is "meaningless" for unbound wq. So remove WQ_CPU_INTENSIVE from the verify_wq allocation. 1. https://www.kernel.org/doc/html/latest/core-api/workqueue.html#flags Suggested-by: Maksym Planeta Signed-off-by: Mike Snitzer --- drivers/md/dm-verity-target.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index a56e254214fa..9b87db8ef99f 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -1388,7 +1388,7 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) } /* WQ_UNBOUND greatly improves performance when running on ramdisk */ - v->verify_wq = alloc_workqueue("kverityd", WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM | WQ_UNBOUND, num_online_cpus()); + v->verify_wq = alloc_workqueue("kverityd", WQ_MEM_RECLAIM | WQ_UNBOUND, num_online_cpus()); if (!v->verify_wq) { ti->error = "Cannot allocate workqueue"; r = -ENOMEM; -- cgit v1.2.3 From 12907efde6ad984f2d76cc1a7dbaae132384d8a5 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Thu, 4 Aug 2022 15:55:57 -0400 Subject: dm verity: have verify_wq use WQ_HIGHPRI if "try_verify_in_tasklet" Allow verify_wq to preempt softirq since verification in tasklet will fall-back to using it for error handling (or if the bufio cache doesn't have required hashes). Suggested-by: Nathan Huckleberry Signed-off-by: Mike Snitzer --- drivers/md/dm-verity-target.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index 9b87db8ef99f..981821f18a18 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -1151,6 +1151,7 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) struct dm_verity_sig_opts verify_args = {0}; struct dm_arg_set as; unsigned int num; + unsigned int wq_flags; unsigned long long num_ll; int r; int i; @@ -1388,7 +1389,16 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) } /* WQ_UNBOUND greatly improves performance when running on ramdisk */ - v->verify_wq = alloc_workqueue("kverityd", WQ_MEM_RECLAIM | WQ_UNBOUND, num_online_cpus()); + wq_flags = WQ_MEM_RECLAIM | WQ_UNBOUND; + if (v->use_tasklet) { + /* + * Allow verify_wq to preempt softirq since verification in + * tasklet will fall-back to using it for error handling + * (or if the bufio cache doesn't have required hashes). + */ + wq_flags |= WQ_HIGHPRI; + } + v->verify_wq = alloc_workqueue("kverityd", wq_flags, num_online_cpus()); if (!v->verify_wq) { ti->error = "Cannot allocate workqueue"; r = -ENOMEM; -- cgit v1.2.3