dm-verity: make dm_verity_fec_io::bufs variable-length

When correcting a data block, the FEC code performs optimally when it
has enough buffers to hold all the needed RS blocks.  That number of
buffers is '1 << (v->data_dev_block_bits - DM_VERITY_FEC_BUF_RS_BITS)'.

However, since v->data_dev_block_bits isn't a compile-time constant, the
code actually used PAGE_SHIFT instead.

With the traditional PAGE_SIZE == data_block_size == 4096, this was
fine.  However, when PAGE_SIZE > data_block_size, this wastes space.
E.g., with data_block_size == 4096 && PAGE_SIZE == 16384, struct
dm_verity_fec_io is 9240 bytes, when in fact only 3096 bytes are needed.

Fix this by making dm_verity_fec_io::bufs a variable-length array.

This makes the macros DM_VERITY_FEC_BUF_MAX and
fec_for_each_extra_buffer() no longer apply, so remove them.  For
consistency, and because DM_VERITY_FEC_BUF_PREALLOC is fixed at 1 and
was already assumed to be 1 (considering that mempool_alloc() shouldn't
be called in a loop), also remove the related macros
DM_VERITY_FEC_BUF_PREALLOC and fec_for_each_prealloc_buffer().

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
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 2c1544556a1c9497fc1bb147b85c44e62e73acdc..6d0b5b4b2699e8585ce1d1e5a2dfb77aff3e7494 100644 (file)
--- a/drivers/md/dm-verity-fec.c
+++ b/drivers/md/dm-verity-fec.c
@@ -10,6 +10,18 @@
 
 #define DM_MSG_PREFIX  "verity-fec"
 
+/*
+ * When correcting a data block, the FEC code performs optimally when it can
+ * collect all the associated RS blocks at the same time.  As each byte is part
+ * of a different RS block, there are '1 << data_dev_block_bits' RS blocks.
+ * There are '1 << DM_VERITY_FEC_BUF_RS_BITS' RS blocks per buffer, so that
+ * gives '1 << (data_dev_block_bits - DM_VERITY_FEC_BUF_RS_BITS)' buffers.
+ */
+static inline unsigned int fec_max_nbufs(struct dm_verity *v)
+{
+       return 1 << (v->data_dev_block_bits - DM_VERITY_FEC_BUF_RS_BITS);
+}
+
 /*
  * If error correction has been configured, returns true.
  */
@@ -59,14 +71,6 @@ static u8 *fec_read_parity(struct dm_verity *v, u64 rsb, int index,
        return res;
 }
 
-/* Loop over each preallocated buffer slot. */
-#define fec_for_each_prealloc_buffer(__i) \
-       for (__i = 0; __i < DM_VERITY_FEC_BUF_PREALLOC; __i++)
-
-/* Loop over each extra buffer slot. */
-#define fec_for_each_extra_buffer(io, __i) \
-       for (__i = DM_VERITY_FEC_BUF_PREALLOC; __i < DM_VERITY_FEC_BUF_MAX; __i++)
-
 /* Loop over each allocated buffer. */
 #define fec_for_each_buffer(io, __i) \
        for (__i = 0; __i < (io)->nbufs; __i++)
@@ -307,6 +311,7 @@ done:
  */
 static struct dm_verity_fec_io *fec_alloc_and_init_io(struct dm_verity *v)
 {
+       const unsigned int max_nbufs = fec_max_nbufs(v);
        struct dm_verity_fec *f = v->fec;
        struct dm_verity_fec_io *fio;
        unsigned int n;
@@ -314,13 +319,10 @@ static struct dm_verity_fec_io *fec_alloc_and_init_io(struct dm_verity *v)
        fio = mempool_alloc(&f->fio_pool, GFP_NOIO);
        fio->rs = mempool_alloc(&f->rs_pool, GFP_NOIO);
 
-       memset(fio->bufs, 0, sizeof(fio->bufs));
-
-       fec_for_each_prealloc_buffer(n)
-               fio->bufs[n] = mempool_alloc(&f->prealloc_pool, GFP_NOIO);
+       fio->bufs[0] = mempool_alloc(&f->prealloc_pool, GFP_NOIO);
 
        /* try to allocate the maximum number of buffers */
-       fec_for_each_extra_buffer(fio, n) {
+       for (n = 1; n < max_nbufs; n++) {
                fio->bufs[n] = kmem_cache_alloc(f->cache, GFP_NOWAIT);
                /* we can manage with even one buffer if necessary */
                if (unlikely(!fio->bufs[n]))
@@ -462,12 +464,10 @@ void __verity_fec_finish_io(struct dm_verity_io *io)
 
        mempool_free(fio->rs, &f->rs_pool);
 
-       fec_for_each_prealloc_buffer(n)
-               mempool_free(fio->bufs[n], &f->prealloc_pool);
+       mempool_free(fio->bufs[0], &f->prealloc_pool);
 
-       fec_for_each_extra_buffer(fio, n)
-               if (fio->bufs[n])
-                       kmem_cache_free(f->cache, fio->bufs[n]);
+       for (n = 1; n < fio->nbufs; n++)
+               kmem_cache_free(f->cache, fio->bufs[n]);
 
        mempool_free(fio->output, &f->output_pool);
 
@@ -735,7 +735,8 @@ int verity_fec_ctr(struct dm_verity *v)
 
        /* Preallocate some dm_verity_fec_io structures */
        ret = mempool_init_kmalloc_pool(&f->fio_pool, num_online_cpus(),
-                                       sizeof(struct dm_verity_fec_io));
+                                       struct_size((struct dm_verity_fec_io *)0,
+                                                   bufs, fec_max_nbufs(v)));
        if (ret) {
                ti->error = "Cannot allocate FEC IO pool";
                return ret;
@@ -757,9 +758,8 @@ int verity_fec_ctr(struct dm_verity *v)
                return -ENOMEM;
        }
 
-       /* Preallocate DM_VERITY_FEC_BUF_PREALLOC buffers for each thread */
-       ret = mempool_init_slab_pool(&f->prealloc_pool, num_online_cpus() *
-                                    DM_VERITY_FEC_BUF_PREALLOC,
+       /* Preallocate one buffer for each thread */
+       ret = mempool_init_slab_pool(&f->prealloc_pool, num_online_cpus(),
                                     f->cache);
        if (ret) {
                ti->error = "Cannot allocate FEC buffer prealloc pool";

diff --git a/drivers/md/dm-verity-fec.h b/drivers/md/dm-verity-fec.h
index b9488d1ddf1410ba5421c3e2878b9da7a8a3f803..57109743831116acaebb6757d2a2b7a9a41ff4c0 100644 (file)
--- a/drivers/md/dm-verity-fec.h
+++ b/drivers/md/dm-verity-fec.h
@@ -17,11 +17,7 @@
 #define DM_VERITY_FEC_MIN_RSN          231     /* ~10% space overhead */
 
 /* buffers for deinterleaving and decoding */
-#define DM_VERITY_FEC_BUF_PREALLOC     1       /* buffers to preallocate */
 #define DM_VERITY_FEC_BUF_RS_BITS      4       /* 1 << RS blocks per buffer */
-/* we need buffers for at most 1 << block size RS blocks */
-#define DM_VERITY_FEC_BUF_MAX \
-       (1 << (PAGE_SHIFT - DM_VERITY_FEC_BUF_RS_BITS))
 
 #define DM_VERITY_OPT_FEC_DEV          "use_fec_from_device"
 #define DM_VERITY_OPT_FEC_BLOCKS       "fec_blocks"
@@ -52,10 +48,17 @@ struct dm_verity_fec {
 struct dm_verity_fec_io {
        struct rs_control *rs;  /* Reed-Solomon state */
        int erasures[DM_VERITY_FEC_MAX_RSN];    /* erasures for decode_rs8 */
-       u8 *bufs[DM_VERITY_FEC_BUF_MAX];        /* bufs for deinterleaving */
-       unsigned int nbufs;             /* number of buffers allocated */
        u8 *output;             /* buffer for corrected output */
        unsigned int level;             /* recursion level */
+       unsigned int nbufs;             /* number of buffers allocated */
+       /*
+        * Buffers for deinterleaving RS blocks.  Each buffer has space for
+        * the data bytes of (1 << DM_VERITY_FEC_BUF_RS_BITS) RS blocks.  The
+        * array length is fec_max_nbufs(v), and we try to allocate that many
+        * buffers.  However, in low-memory situations we may be unable to
+        * allocate all buffers.  'nbufs' holds the number actually allocated.
+        */
+       u8 *bufs[];
 };
 
 #ifdef CONFIG_DM_VERITY_FEC