Re: [PATCH] dm-verity: all verity devices share the same dm_io_client and recheck_pool.

[LongPing Wei]

Hi, Mikulas

Thanks for your suggestion. I will test it.

LongPing

On 2025/4/24 2:46, Mikulas Patocka wrote:

Hi

I nack this series of patches, because sharing mempools (or dm_io_client)
is prone to deadlocks.

If you have two dm-verity devices stacked on each other, the upper device
may deplete the mempool and the lower device has no way to do forward
progress, resulting in a deadlock.

For example, suppose that you have dm-snapshot on the top of dm-verity on
the top of another dm-verity device. If the kernel needs to write back
dirty memory, it calls the dm-snapshot target, the dm-snapshot reads data
from the upper dm-verity device, the upper dm-verity device depletes the
shared mempool and the lower dm-verity device has no way to make forward
progress - deadlock.

If you want to reduce memory consumption, set reserved_bio_based_ios to 1,
that should be safe. (setting it to 0 is prone to deadlock).

I remember that the one of the first things that I did at Red Hat 17 years
ago was removing shared resources from device mapper and making them
per-device :-)

Mikulas


On Wed, 23 Apr 2025, LongPing Wei wrote:

The size of biovec's memory of one dm_io_client will be
reserved_bio_based_ios(default value is 16) * 4KiB memory.
When the number of dm-verity devices is dozens,
the total memory size will be several MiBs.

The number of dm-verity devices is 59 on my test device,
and the size of biovec's memory here will be 3.87MiB.

This commit will reduce the memory usage of dm-verity
by making all verity devices share the same dm_io_client and recheck_pool.

Fixes: 9177f3c0dea61 ("dm-verity: recheck the hash after a failure")

Signed-off-by: LongPing Wei <weilongp...@oppo.com>
---
  drivers/md/dm-verity-target.c | 71 ++++++++++++++++++++++++-----------
  drivers/md/dm-verity.h        |  3 --
  2 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 3c427f18a04b..986c3386aa59 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -72,6 +72,9 @@ struct dm_verity_prefetch_work {
        unsigned int n_blocks;
  };

+static struct dm_io_client *verity_io_client;

+static mempool_t verity_recheck_pool;
+
  /*
   * Auxiliary structure appended to each dm-bufio buffer. If the value
   * hash_verified is nonzero, hash of the block has been verified.
@@ -449,14 +452,14 @@ static noinline int verity_recheck(struct dm_verity *v, 
struct dm_verity_io *io,
        struct dm_io_request io_req;
        struct dm_io_region io_loc;

- page = mempool_alloc(&v->recheck_pool, GFP_NOIO);

+       page = mempool_alloc(&verity_recheck_pool, GFP_NOIO);
        buffer = page_to_virt(page);

io_req.bi_opf = REQ_OP_READ;

        io_req.mem.type = DM_IO_KMEM;
        io_req.mem.ptr.addr = buffer;
        io_req.notify.fn = NULL;
-       io_req.client = v->io;
+       io_req.client = verity_io_client;
        io_loc.bdev = v->data_dev->bdev;
        io_loc.sector = cur_block << (v->data_dev_block_bits - SECTOR_SHIFT);
        io_loc.count = 1 << (v->data_dev_block_bits - SECTOR_SHIFT);
@@ -478,7 +481,7 @@ static noinline int verity_recheck(struct dm_verity *v, 
struct dm_verity_io *io,
        memcpy(dest, buffer, 1 << v->data_dev_block_bits);
        r = 0;
  free_ret:
-       mempool_free(page, &v->recheck_pool);
+       mempool_free(page, &verity_recheck_pool);

return r;

  }
@@ -1075,10 +1078,6 @@ static void verity_dtr(struct dm_target *ti)
        if (v->verify_wq)
                destroy_workqueue(v->verify_wq);

- mempool_exit(&v->recheck_pool);

-       if (v->io)
-               dm_io_client_destroy(v->io);
-
        if (v->bufio)
                dm_bufio_client_destroy(v->bufio);

@@ -1625,20 +1624,6 @@ static int verity_ctr(struct dm_target *ti, unsigned int argc, char **argv)

        }
        v->hash_blocks = hash_position;

- r = mempool_init_page_pool(&v->recheck_pool, 1, 0);

-       if (unlikely(r)) {
-               ti->error = "Cannot allocate mempool";
-               goto bad;
-       }
-
-       v->io = dm_io_client_create();
-       if (IS_ERR(v->io)) {
-               r = PTR_ERR(v->io);
-               v->io = NULL;
-               ti->error = "Cannot allocate dm io";
-               goto bad;
-       }
-
        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,
@@ -1822,7 +1807,49 @@ static struct target_type verity_target = {
        .preresume      = verity_preresume,
  #endif /* CONFIG_SECURITY */
  };
-module_dm(verity);
+
+static int __init dm_verity_init(void)
+{
+       int r;
+
+       r = mempool_init_page_pool(&verity_recheck_pool, 1, 0);
+       if (unlikely(r)) {
+               DMERR("Cannot allocate mempool");
+               goto mempool_init_page_pool_failed;
+       }
+
+       verity_io_client = dm_io_client_create();
+       if (IS_ERR(verity_io_client)) {
+               r = PTR_ERR(verity_io_client);
+               verity_io_client = NULL;
+               DMERR("Cannot allocate dm io");
+               goto dm_io_client_create_failed;
+       }
+
+       r = dm_register_target(&(verity_target));
+       if (unlikely(r))
+               goto dm_register_target_failed;
+       return r;
+
+dm_register_target_failed:
+       dm_io_client_destroy(verity_io_client);
+       verity_io_client = NULL;
+dm_io_client_create_failed:
+       mempool_exit(&verity_recheck_pool);
+mempool_init_page_pool_failed:
+       return r;
+}
+module_init(dm_verity_init);
+
+static void __exit dm_verity_exit(void)
+{
+       dm_unregister_target(&(verity_target));
+       mempool_exit(&verity_recheck_pool);
+       if (verity_io_client)
+               dm_io_client_destroy(verity_io_client);
+       verity_io_client = NULL;
+}
+module_exit(dm_verity_exit)

/*

   * Check whether a DM target is a verity target.
diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
index 8cbb57862ae1..f28454f81249 100644
--- a/drivers/md/dm-verity.h
+++ b/drivers/md/dm-verity.h
@@ -75,9 +75,6 @@ struct dm_verity {
        unsigned long *validated_blocks; /* bitset blocks validated */

char *signature_key_desc; /* signature keyring reference */

-
-       struct dm_io_client *io;
-       mempool_t recheck_pool;
  };

struct dm_verity_io {

--
2.34.1