dm-verity: move dm_verity_fec_io to mempool

Currently, struct dm_verity_fec_io is allocated in the front padding of
struct bio using dm_target::per_io_data_size.  Unfortunately, struct
dm_verity_fec_io is very large: 3096 bytes when CONFIG_64BIT=y &&
PAGE_SIZE == 4096, or 9240 bytes when CONFIG_64BIT=y && PAGE_SIZE ==
16384.  This makes the bio size very large.

Moreover, most of dm_verity_fec_io gets iterated over up to three times,
even on I/O requests that don't require any error correction:

1. To zero the memory on allocation, if init_on_alloc=1.  (This happens
   when the bio is allocated, not in dm-verity itself.)

2. To zero the buffers array in verity_fec_init_io().

3. To free the buffers in verity_fec_finish_io().

Fix all of these inefficiencies by moving dm_verity_fec_io to a mempool.
Replace the embedded dm_verity_fec_io with a pointer
dm_verity_io::fec_io.  verity_fec_init_io() initializes it to NULL,
verity_fec_decode() allocates it on the first call, and
verity_fec_finish_io() cleans it up.  The normal case is that the
pointer simply stays NULL, so the overhead becomes negligible.

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c
index c79de517afee77b9ca4e325d1d2e13a3ddd4cfec..2c1544556a1c9497fc1bb147b85c44e62e73acdc 100644 (file)
--- a/drivers/md/dm-verity-fec.c
+++ b/drivers/md/dm-verity-fec.c
@@ -18,16 +18,6 @@ bool verity_fec_is_enabled(struct dm_verity *v)
        return v->fec && v->fec->dev;
 }
 
-/*
- * Return a pointer to dm_verity_fec_io after dm_verity_io and its variable
- * length fields.
- */
-static inline struct dm_verity_fec_io *fec_io(struct dm_verity_io *io)
-{
-       return (struct dm_verity_fec_io *)
-               ((char *)io + io->v->ti->per_io_data_size - sizeof(struct dm_verity_fec_io));
-}
-
 /*
  * Return an interleaved offset for a byte in RS block.
  */
@@ -211,7 +201,7 @@ static int fec_read_bufs(struct dm_verity *v, struct dm_verity_io *io,
        int i, j, target_index = -1;
        struct dm_buffer *buf;
        struct dm_bufio_client *bufio;
-       struct dm_verity_fec_io *fio = fec_io(io);
+       struct dm_verity_fec_io *fio = io->fec_io;
        u64 block, ileaved;
        u8 *bbuf, *rs_block;
        u8 want_digest[HASH_MAX_DIGESTSIZE];
@@ -307,39 +297,40 @@ done:
 }
 
 /*
- * Allocate RS control structure and FEC buffers from preallocated mempools,
- * and attempt to allocate as many extra buffers as available.
+ * Allocate and initialize a struct dm_verity_fec_io to use for FEC for a bio.
+ * This runs the first time a block needs to be corrected for a bio.  In the
+ * common case where no block needs to be corrected, this code never runs.
+ *
+ * This always succeeds, as all required allocations are done from mempools.
+ * Additional buffers are also allocated opportunistically to improve error
+ * correction performance, but these aren't required to succeed.
  */
-static int fec_alloc_bufs(struct dm_verity *v, struct dm_verity_fec_io *fio)
+static struct dm_verity_fec_io *fec_alloc_and_init_io(struct dm_verity *v)
 {
+       struct dm_verity_fec *f = v->fec;
+       struct dm_verity_fec_io *fio;
        unsigned int n;
 
-       if (!fio->rs)
-               fio->rs = mempool_alloc(&v->fec->rs_pool, GFP_NOIO);
+       fio = mempool_alloc(&f->fio_pool, GFP_NOIO);
+       fio->rs = mempool_alloc(&f->rs_pool, GFP_NOIO);
 
-       fec_for_each_prealloc_buffer(n) {
-               if (fio->bufs[n])
-                       continue;
+       memset(fio->bufs, 0, sizeof(fio->bufs));
 
-               fio->bufs[n] = mempool_alloc(&v->fec->prealloc_pool, GFP_NOIO);
-       }
+       fec_for_each_prealloc_buffer(n)
+               fio->bufs[n] = mempool_alloc(&f->prealloc_pool, GFP_NOIO);
 
        /* try to allocate the maximum number of buffers */
        fec_for_each_extra_buffer(fio, n) {
-               if (fio->bufs[n])
-                       continue;
-
-               fio->bufs[n] = kmem_cache_alloc(v->fec->cache, GFP_NOWAIT);
+               fio->bufs[n] = kmem_cache_alloc(f->cache, GFP_NOWAIT);
                /* we can manage with even one buffer if necessary */
                if (unlikely(!fio->bufs[n]))
                        break;
        }
        fio->nbufs = n;
 
-       if (!fio->output)
-               fio->output = mempool_alloc(&v->fec->output_pool, GFP_NOIO);
-
-       return 0;
+       fio->output = mempool_alloc(&f->output_pool, GFP_NOIO);
+       fio->level = 0;
+       return fio;
 }
 
 /*
@@ -368,10 +359,6 @@ static int fec_decode_rsb(struct dm_verity *v, struct dm_verity_io *io,
        int r, neras = 0;
        unsigned int pos;
 
-       r = fec_alloc_bufs(v, fio);
-       if (unlikely(r < 0))
-               return r;
-
        for (pos = 0; pos < 1 << v->data_dev_block_bits; ) {
                fec_init_bufs(v, fio);
 
@@ -408,12 +395,16 @@ int verity_fec_decode(struct dm_verity *v, struct dm_verity_io *io,
                      sector_t block, u8 *dest)
 {
        int r;
-       struct dm_verity_fec_io *fio = fec_io(io);
+       struct dm_verity_fec_io *fio;
        u64 offset, res, rsb;
 
        if (!verity_fec_is_enabled(v))
                return -EOPNOTSUPP;
 
+       fio = io->fec_io;
+       if (!fio)
+               fio = io->fec_io = fec_alloc_and_init_io(v);
+
        if (fio->level)
                return -EIO;
 
@@ -463,14 +454,11 @@ done:
 /*
  * Clean up per-bio data.
  */
-void verity_fec_finish_io(struct dm_verity_io *io)
+void __verity_fec_finish_io(struct dm_verity_io *io)
 {
        unsigned int n;
        struct dm_verity_fec *f = io->v->fec;
-       struct dm_verity_fec_io *fio = fec_io(io);
-
-       if (!verity_fec_is_enabled(io->v))
-               return;
+       struct dm_verity_fec_io *fio = io->fec_io;
 
        mempool_free(fio->rs, &f->rs_pool);
 
@@ -482,23 +470,9 @@ void verity_fec_finish_io(struct dm_verity_io *io)
                        kmem_cache_free(f->cache, fio->bufs[n]);
 
        mempool_free(fio->output, &f->output_pool);
-}
-
-/*
- * Initialize per-bio data.
- */
-void verity_fec_init_io(struct dm_verity_io *io)
-{
-       struct dm_verity_fec_io *fio = fec_io(io);
-
-       if (!verity_fec_is_enabled(io->v))
-               return;
 
-       fio->rs = NULL;
-       memset(fio->bufs, 0, sizeof(fio->bufs));
-       fio->nbufs = 0;
-       fio->output = NULL;
-       fio->level = 0;
+       mempool_free(fio, &f->fio_pool);
+       io->fec_io = NULL;
 }
 
 /*
@@ -529,6 +503,7 @@ void verity_fec_dtr(struct dm_verity *v)
        if (!verity_fec_is_enabled(v))
                goto out;
 
+       mempool_exit(&f->fio_pool);
        mempool_exit(&f->rs_pool);
        mempool_exit(&f->prealloc_pool);
        mempool_exit(&f->output_pool);
@@ -758,6 +733,14 @@ int verity_fec_ctr(struct dm_verity *v)
                return -E2BIG;
        }
 
+       /* Preallocate some dm_verity_fec_io structures */
+       ret = mempool_init_kmalloc_pool(&f->fio_pool, num_online_cpus(),
+                                       sizeof(struct dm_verity_fec_io));
+       if (ret) {
+               ti->error = "Cannot allocate FEC IO pool";
+               return ret;
+       }
+
        /* Preallocate an rs_control structure for each worker thread */
        ret = mempool_init(&f->rs_pool, num_online_cpus(), fec_rs_alloc,
                           fec_rs_free, (void *) v);
@@ -791,8 +774,5 @@ int verity_fec_ctr(struct dm_verity *v)
                return ret;
        }
 
-       /* Reserve space for our per-bio data */
-       ti->per_io_data_size += sizeof(struct dm_verity_fec_io);
-
        return 0;
 }

diff --git a/drivers/md/dm-verity-fec.h b/drivers/md/dm-verity-fec.h
index 5fd26787381271eeb0d287170f82919feea24159..b9488d1ddf1410ba5421c3e2878b9da7a8a3f803 100644 (file)
--- a/drivers/md/dm-verity-fec.h
+++ b/drivers/md/dm-verity-fec.h
@@ -40,6 +40,7 @@ struct dm_verity_fec {
        sector_t hash_blocks;   /* blocks covered after v->hash_start */
        unsigned char roots;    /* number of parity bytes, M-N of RS(M, N) */
        unsigned char rsn;      /* N of RS(M, N) */
+       mempool_t fio_pool;     /* mempool for dm_verity_fec_io */
        mempool_t rs_pool;      /* mempool for fio->rs */
        mempool_t prealloc_pool;        /* mempool for preallocated buffers */
        mempool_t output_pool;  /* mempool for output */
@@ -71,8 +72,17 @@ extern int verity_fec_decode(struct dm_verity *v, struct dm_verity_io *io,
 extern unsigned int verity_fec_status_table(struct dm_verity *v, unsigned int sz,
                                        char *result, unsigned int maxlen);
 
-extern void verity_fec_finish_io(struct dm_verity_io *io);
-extern void verity_fec_init_io(struct dm_verity_io *io);
+extern void __verity_fec_finish_io(struct dm_verity_io *io);
+static inline void verity_fec_finish_io(struct dm_verity_io *io)
+{
+       if (unlikely(io->fec_io))
+               __verity_fec_finish_io(io);
+}
+
+static inline void verity_fec_init_io(struct dm_verity_io *io)
+{
+       io->fec_io = NULL;
+}
 
 extern bool verity_is_fec_opt_arg(const char *arg_name);
 extern int verity_fec_parse_opt_args(struct dm_arg_set *as,

diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
index f975a9e5c5d6ba411766e090130aa8c7ff759781..4ad7ce3dae0a569720f141757e450bc3efa4ad27 100644 (file)
--- a/drivers/md/dm-verity.h
+++ b/drivers/md/dm-verity.h
@@ -104,6 +104,10 @@ struct dm_verity_io {
        bool in_bh;
        bool had_mismatch;
 
+#ifdef CONFIG_DM_VERITY_FEC
+       struct dm_verity_fec_io *fec_io;
+#endif
+
        struct work_struct work;
        struct work_struct bh_work;