[dm-devel] [PATCH 2/2] dm integrity: use init_completion instead of COMPLETION_INITIALIZER_ONSTACK

2017-08-15 Thread Arnd Bergmann 
The new lockdep support for completions causeed the stack usage
in dm-integrity to explode, in case of write_journal from 504 bytes
to 1120 (using arm gcc-7.1.1):

drivers/md/dm-integrity.c: In function 'write_journal':
drivers/md/dm-integrity.c:827:1: error: the frame size of 1120 bytes is larger 
than 1024 bytes [-Werror=frame-larger-than=]

The problem is that not only the size of 'struct completion' grows
significantly, but we end up having multiple copies of it on the stack
when we assign it from a local variable after the initial declaration.

COMPLETION_INITIALIZER_ONSTACK() is the right thing to use when we
want to declare and initialize a completion on the stack. However,
this driver doesn't do that and instead initializes the completion
just before it is used.

In this case, init_completion() does the same thing more efficiently,
and drops the stack usage for the function above down to 496 bytes.
While the other functions in this file are not bad enough to cause
a warning, they benefit equally from the change, so I do the change
across the entire file. In the one place where we reuse a completion,
I picked the cheaper reinit_completion() over init_completion().

Fixes: cd8084f91c02 ("locking/lockdep: Apply crossrelease to completions")
Signed-off-by: Arnd Bergmann 
---
The patch causing this is currently part of linux-next, scheduled for
4.14, so it would be good to have this in the same release.
---
 drivers/md/dm-integrity.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 293a19652d55..b16010bcbd17 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -773,13 +773,13 @@ static void write_journal(struct dm_integrity_c *ic, 
unsigned commit_start, unsi
unsigned i;
 
io_comp.ic = ic;
-   io_comp.comp = COMPLETION_INITIALIZER_ONSTACK(io_comp.comp);
+   init_completion(_comp.comp);
 
if (commit_start + commit_sections <= ic->journal_sections) {
io_comp.in_flight = (atomic_t)ATOMIC_INIT(1);
if (ic->journal_io) {
crypt_comp_1.ic = ic;
-   crypt_comp_1.comp = 
COMPLETION_INITIALIZER_ONSTACK(crypt_comp_1.comp);
+   init_completion(_comp_1.comp);
crypt_comp_1.in_flight = (atomic_t)ATOMIC_INIT(0);
encrypt_journal(ic, true, commit_start, 
commit_sections, _comp_1);
wait_for_completion_io(_comp_1.comp);
@@ -795,18 +795,18 @@ static void write_journal(struct dm_integrity_c *ic, 
unsigned commit_start, unsi
to_end = ic->journal_sections - commit_start;
if (ic->journal_io) {
crypt_comp_1.ic = ic;
-   crypt_comp_1.comp = 
COMPLETION_INITIALIZER_ONSTACK(crypt_comp_1.comp);
+   init_completion(_comp_1.comp);
crypt_comp_1.in_flight = (atomic_t)ATOMIC_INIT(0);
encrypt_journal(ic, true, commit_start, to_end, 
_comp_1);
if (try_wait_for_completion(_comp_1.comp)) {
rw_journal(ic, REQ_OP_WRITE, REQ_FUA, 
commit_start, to_end, _comp);
-   crypt_comp_1.comp = 
COMPLETION_INITIALIZER_ONSTACK(crypt_comp_1.comp);
+   reinit_completion(_comp_1.comp);
crypt_comp_1.in_flight = 
(atomic_t)ATOMIC_INIT(0);
encrypt_journal(ic, true, 0, commit_sections - 
to_end, _comp_1);
wait_for_completion_io(_comp_1.comp);
} else {
crypt_comp_2.ic = ic;
-   crypt_comp_2.comp = 
COMPLETION_INITIALIZER_ONSTACK(crypt_comp_2.comp);
+   init_completion(_comp_2.comp);
crypt_comp_2.in_flight = 
(atomic_t)ATOMIC_INIT(0);
encrypt_journal(ic, true, 0, commit_sections - 
to_end, _comp_2);
wait_for_completion_io(_comp_1.comp);
@@ -1679,7 +1679,7 @@ static void dm_integrity_map_continue(struct 
dm_integrity_io *dio, bool from_map
dio->in_flight = (atomic_t)ATOMIC_INIT(2);
 
if (need_sync_io) {
-   read_comp = COMPLETION_INITIALIZER_ONSTACK(read_comp);
+   init_completion(_comp);
dio->completion = _comp;
} else
dio->completion = NULL;
@@ -1836,7 +1836,7 @@ static void do_journal_write(struct dm_integrity_c *ic, 
unsigned write_start,
 
comp.ic = ic;
comp.in_flight = (atomic_t)ATOMIC_INIT(1);
-   comp.comp = COMPLETION_INITIALIZER_ONSTACK(comp.comp);
+   init_completion();
 
i = write_start;
for (n = 0; n < write_sections; n++, i++, wraparound_section(ic, )) {

Re: [dm-devel] [PATCH v5 02/19] crypto: ccp: use -EAGAIN for transient busy indication

2017-08-15 Thread Gary R Hook 

On 08/14/2017 10:21 AM, Gilad Ben-Yossef wrote:

Replace -EBUSY with -EAGAIN when reporting transient busy
indication in the absence of backlog.

Signed-off-by: Gilad Ben-Yossef 


Reviewed-by: Gary R Hook 


---
 drivers/crypto/ccp/ccp-crypto-main.c | 8 +++-
 drivers/crypto/ccp/ccp-dev.c | 7 +--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-main.c 
b/drivers/crypto/ccp/ccp-crypto-main.c
index 35a9de7..403ff0a 100644
--- a/drivers/crypto/ccp/ccp-crypto-main.c
+++ b/drivers/crypto/ccp/ccp-crypto-main.c
@@ -222,9 +222,10 @@ static int ccp_crypto_enqueue_cmd(struct ccp_crypto_cmd 
*crypto_cmd)

/* Check if the cmd can/should be queued */
if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
-   ret = -EBUSY;
-   if (!(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG))
+   if (!(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG)) {
+   ret = -EAGAIN;
goto e_lock;
+   }
}

/* Look for an entry with the same tfm.  If there is a cmd
@@ -243,9 +244,6 @@ static int ccp_crypto_enqueue_cmd(struct ccp_crypto_cmd 
*crypto_cmd)
ret = ccp_enqueue_cmd(crypto_cmd->cmd);
if (!ccp_crypto_success(ret))
goto e_lock;/* Error, don't queue it */
-   if ((ret == -EBUSY) &&
-   !(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG))
-   goto e_lock;/* Not backlogging, don't queue it */
}

if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c
index 4e029b1..3d637e3 100644
--- a/drivers/crypto/ccp/ccp-dev.c
+++ b/drivers/crypto/ccp/ccp-dev.c
@@ -292,9 +292,12 @@ int ccp_enqueue_cmd(struct ccp_cmd *cmd)
i = ccp->cmd_q_count;

if (ccp->cmd_count >= MAX_CMD_QLEN) {
-   ret = -EBUSY;
-   if (cmd->flags & CCP_CMD_MAY_BACKLOG)
+   if (cmd->flags & CCP_CMD_MAY_BACKLOG) {
+   ret = -EBUSY;
list_add_tail(>entry, >backlog);
+   } else {
+   ret = -EAGAIN;
+   }
} else {
ret = -EINPROGRESS;
ccp->cmd_count++;



--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v5 04/19] crypto: marvell/cesa: remove redundant backlog checks on EBUSY

2017-08-15 Thread Boris Brezillon 
Le Mon, 14 Aug 2017 18:21:14 +0300,
Gilad Ben-Yossef  a écrit :

> Now that -EBUSY return code only indicates backlog queueing
> we can safely remove the now redundant check for the
> CRYPTO_TFM_REQ_MAY_BACKLOG flag when -EBUSY is returned.
> 
> Signed-off-by: Gilad Ben-Yossef 

Acked-by: Boris Brezillon 

> ---
>  drivers/crypto/marvell/cesa.c | 3 +--
>  drivers/crypto/marvell/cesa.h | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/crypto/marvell/cesa.c b/drivers/crypto/marvell/cesa.c
> index 6e7a5c7..269737f 100644
> --- a/drivers/crypto/marvell/cesa.c
> +++ b/drivers/crypto/marvell/cesa.c
> @@ -183,8 +183,7 @@ int mv_cesa_queue_req(struct crypto_async_request *req,
>   spin_lock_bh(>lock);
>   ret = crypto_enqueue_request(>queue, req);
>   if ((mv_cesa_req_get_type(creq) == CESA_DMA_REQ) &&
> - (ret == -EINPROGRESS ||
> - (ret == -EBUSY && req->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
> + (ret == -EINPROGRESS || ret == -EBUSY)
>   mv_cesa_tdma_chain(engine, creq);
>   spin_unlock_bh(>lock);
>  
> diff --git a/drivers/crypto/marvell/cesa.h b/drivers/crypto/marvell/cesa.h
> index b7872f6..63c8457 100644
> --- a/drivers/crypto/marvell/cesa.h
> +++ b/drivers/crypto/marvell/cesa.h
> @@ -763,7 +763,7 @@ static inline int mv_cesa_req_needs_cleanup(struct 
> crypto_async_request *req,
>* the backlog and will be processed later. There's no need to
>* clean it up.
>*/
> - if (ret == -EBUSY && req->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)
> + if (ret == -EBUSY)
>   return false;
>  
>   /* Request wasn't queued, we need to clean it up */


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v5 19/19] crypto: adapt api sample to use async. op wait

2017-08-15 Thread Gilad Ben-Yossef 
The code sample is waiting for an async. crypto op completion.
Adapt sample to use the new generic infrastructure to do the same.

This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.

Signed-off-by: Gilad Ben-Yossef 
---
 Documentation/crypto/api-samples.rst | 52 +++-
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/Documentation/crypto/api-samples.rst 
b/Documentation/crypto/api-samples.rst
index 2531948..006827e 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -7,59 +7,27 @@ Code Example For Symmetric Key Cipher Operation
 ::
 
 
-struct tcrypt_result {
-struct completion completion;
-int err;
-};
-
 /* tie all data structures together */
 struct skcipher_def {
 struct scatterlist sg;
 struct crypto_skcipher *tfm;
 struct skcipher_request *req;
-struct tcrypt_result result;
+struct crypto_wait wait;
 };
 
-/* Callback function */
-static void test_skcipher_cb(struct crypto_async_request *req, int error)
-{
-struct tcrypt_result *result = req->data;
-
-if (error == -EINPROGRESS)
-return;
-result->err = error;
-complete(>completion);
-pr_info("Encryption finished successfully\n");
-}
-
 /* Perform cipher operation */
 static unsigned int test_skcipher_encdec(struct skcipher_def *sk,
  int enc)
 {
-int rc = 0;
+int rc;
 
 if (enc)
-rc = crypto_skcipher_encrypt(sk->req);
+rc = crypto_wait_req(crypto_skcipher_encrypt(sk->req), >wait);
 else
-rc = crypto_skcipher_decrypt(sk->req);
-
-switch (rc) {
-case 0:
-break;
-case -EINPROGRESS:
-case -EBUSY:
-rc = wait_for_completion_interruptible(
->result.completion);
-if (!rc && !sk->result.err) {
-reinit_completion(>result.completion);
-break;
-}
-default:
-pr_info("skcipher encrypt returned with %d result %d\n",
-rc, sk->result.err);
-break;
-}
-init_completion(>result.completion);
+rc = crypto_wait_req(crypto_skcipher_decrypt(sk->req), >wait);
+
+   if (rc)
+   pr_info("skcipher encrypt returned with result %d\n", rc);
 
 return rc;
 }
@@ -89,8 +57,8 @@ Code Example For Symmetric Key Cipher Operation
 }
 
 skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-  test_skcipher_cb,
-  );
+  crypto_req_done,
+  );
 
 /* AES 256 with random key */
 get_random_bytes(, 32);
@@ -122,7 +90,7 @@ Code Example For Symmetric Key Cipher Operation
 /* We encrypt one block */
 sg_init_one(, scratchpad, 16);
 skcipher_request_set_crypt(req, , , 16, ivdata);
-init_completion();
+crypto_init_wait();
 
 /* encrypt data */
 ret = test_skcipher_encdec(, 1);
-- 
2.1.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v5 05/19] crypto: introduce crypto wait for async op

2017-08-15 Thread Jonathan Cameron 
On Mon, 14 Aug 2017 18:21:15 +0300
Gilad Ben-Yossef  wrote:

> Invoking a possibly async. crypto op and waiting for completion
> while correctly handling backlog processing is a common task
> in the crypto API implementation and outside users of it.
> 
> This patch adds a generic implementation for doing so in
> preparation for using it across the board instead of hand
> rolled versions.
> 
> Signed-off-by: Gilad Ben-Yossef 
> CC: Eric Biggers 

One really trivial point inline I happened to notice.

> ---
>  crypto/api.c   | 13 +
>  include/linux/crypto.h | 41 +
>  2 files changed, 54 insertions(+)
> 
> diff --git a/crypto/api.c b/crypto/api.c
> index 941cd4c..2a2479d 100644
> --- a/crypto/api.c
> +++ b/crypto/api.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "internal.h"
>  
>  LIST_HEAD(crypto_alg_list);
> @@ -595,5 +596,17 @@ int crypto_has_alg(const char *name, u32 type, u32 mask)
>  }
>  EXPORT_SYMBOL_GPL(crypto_has_alg);
>  
> +void crypto_req_done(struct crypto_async_request *req, int err)
> +{
> + struct crypto_wait *wait = req->data;
> +
> + if (err == -EINPROGRESS)
> + return;
> +
> + wait->err = err;
> + complete(>completion);
> +}
> +EXPORT_SYMBOL_GPL(crypto_req_done);
> +
>  MODULE_DESCRIPTION("Cryptographic core API");
>  MODULE_LICENSE("GPL");
> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> index 84da997..bb00186 100644
> --- a/include/linux/crypto.h
> +++ b/include/linux/crypto.h
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /*
>   * Autoloaded crypto modules should only use a prefixed name to avoid 
> allowing
> @@ -468,6 +469,45 @@ struct crypto_alg {
>  } CRYPTO_MINALIGN_ATTR;
>  
>  /*
> + * A helper struct for waiting for completion of async crypto ops
> + */
> +struct crypto_wait {
> + struct completion completion;
> + int err;
> +};
> +
> +/*
> + * Macro for declaring a crypto op async wait object on stack
> + */
> +#define DECLARE_CRYPTO_WAIT(_wait) \
> + struct crypto_wait _wait = { \
> + COMPLETION_INITIALIZER_ONSTACK((_wait).completion), 0 }
> +
> +/*
> + * Async ops completion helper functioons
> + */
> +void crypto_req_done(struct crypto_async_request *req, int err);
> +
> +static inline int crypto_wait_req(int err, struct crypto_wait *wait)
> +{
> + switch (err) {
> + case -EINPROGRESS:
> + case -EBUSY:
> + wait_for_completion(>completion);
> + reinit_completion(>completion);
> + err = wait->err;
> + break;
> + };
> +
> + return err;
> +}
> +
> +static inline void crypto_init_wait(struct crypto_wait *wait)
> +{
> + init_completion(>completion);
> +}
> +
> +/*
>   * Algorithm registration interface.
>   */
>  int crypto_register_alg(struct crypto_alg *alg);
> @@ -1604,5 +1644,6 @@ static inline int crypto_comp_decompress(struct 
> crypto_comp *tfm,
>   src, slen, dst, dlen);
>  }
>  
> +

Trivial observation.  Shouldn't have this bonus blank line inserted here.

>  #endif   /* _LINUX_CRYPTO_H */
>  

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v5 18/19] crypto: mediatek: move to generic async completion

2017-08-15 Thread Ryder Lee 
On Mon, 2017-08-14 at 18:21 +0300, Gilad Ben-Yossef wrote:
> The mediatek driver starts several async crypto ops and waits for their
> completions. Move it over to generic code doing the same.
> 
> Signed-off-by: Gilad Ben-Yossef 
> ---

Acked-by: Ryder Lee 

>  drivers/crypto/mediatek/mtk-aes.c | 31 +--
>  1 file changed, 5 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/crypto/mediatek/mtk-aes.c 
> b/drivers/crypto/mediatek/mtk-aes.c
> index 9e845e8..e2c7c95 100644
> --- a/drivers/crypto/mediatek/mtk-aes.c
> +++ b/drivers/crypto/mediatek/mtk-aes.c
> @@ -137,11 +137,6 @@ struct mtk_aes_gcm_ctx {
>   struct crypto_skcipher *ctr;
>  };
>  
> -struct mtk_aes_gcm_setkey_result {
> - int err;
> - struct completion completion;
> -};
> -
>  struct mtk_aes_drv {
>   struct list_head dev_list;
>   /* Device list lock */
> @@ -936,17 +931,6 @@ static int mtk_aes_gcm_crypt(struct aead_request *req, 
> u64 mode)
>   >base);
>  }
>  
> -static void mtk_gcm_setkey_done(struct crypto_async_request *req, int err)
> -{
> - struct mtk_aes_gcm_setkey_result *result = req->data;
> -
> - if (err == -EINPROGRESS)
> - return;
> -
> - result->err = err;
> - complete(>completion);
> -}
> -
>  /*
>   * Because of the hardware limitation, we need to pre-calculate key(H)
>   * for the GHASH operation. The result of the encryption operation
> @@ -962,7 +946,7 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, 
> const u8 *key,
>   u32 hash[4];
>   u8 iv[8];
>  
> - struct mtk_aes_gcm_setkey_result result;
> + struct crypto_wait wait;
>  
>   struct scatterlist sg[1];
>   struct skcipher_request req;
> @@ -1002,22 +986,17 @@ static int mtk_aes_gcm_setkey(struct crypto_aead 
> *aead, const u8 *key,
>   if (!data)
>   return -ENOMEM;
>  
> - init_completion(>result.completion);
> + crypto_init_wait(>wait);
>   sg_init_one(data->sg, >hash, AES_BLOCK_SIZE);
>   skcipher_request_set_tfm(>req, ctr);
>   skcipher_request_set_callback(>req, CRYPTO_TFM_REQ_MAY_SLEEP |
> CRYPTO_TFM_REQ_MAY_BACKLOG,
> -   mtk_gcm_setkey_done, >result);
> +   crypto_req_done, >wait);
>   skcipher_request_set_crypt(>req, data->sg, data->sg,
>  AES_BLOCK_SIZE, data->iv);
>  
> - err = crypto_skcipher_encrypt(>req);
> - if (err == -EINPROGRESS || err == -EBUSY) {
> - err = wait_for_completion_interruptible(
> - >result.completion);
> - if (!err)
> - err = data->result.err;
> - }
> + err = crypto_wait_req(crypto_skcipher_encrypt(>req),
> +   >wait);
>   if (err)
>   goto out;
>  


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm-crypt: limit the number of allocated pages

2017-08-15 Thread Tom Yan 
Just tested the patch with kernel 4.12.6. Well it sort of worked. No
more OOM or kernel panic. Memory takeup is around ~250M on a machine
with 8G RAM. However I keep getting this:

Aug 15 04:04:10 archlinux kernel: INFO: task blkdiscard:538 blocked
for more than 120 seconds.
Aug 15 04:04:10 archlinux kernel:   Tainted: P C O
4.12.6-1-ARCH #1
Aug 15 04:04:10 archlinux kernel: "echo 0 >
/proc/sys/kernel/hung_task_timeout_secs" disables this message.
Aug 15 04:04:10 archlinux kernel: blkdiscard  D0   538537 0x
Aug 15 04:04:10 archlinux kernel: Call Trace:
Aug 15 04:04:10 archlinux kernel:  __schedule+0x236/0x870
Aug 15 04:04:10 archlinux kernel:  schedule+0x3d/0x90
Aug 15 04:04:10 archlinux kernel:  schedule_timeout+0x21f/0x330
Aug 15 04:04:10 archlinux kernel:  io_schedule_timeout+0x1e/0x50
Aug 15 04:04:10 archlinux kernel:  ? io_schedule_timeout+0x1e/0x50
Aug 15 04:04:10 archlinux kernel:  wait_for_completion_io+0xa5/0x120
Aug 15 04:04:10 archlinux kernel:  ? wake_up_q+0x80/0x80
Aug 15 04:04:10 archlinux kernel:  submit_bio_wait+0x68/0x90
Aug 15 04:04:10 archlinux kernel:  blkdev_issue_zeroout+0x80/0xc0
Aug 15 04:04:10 archlinux kernel:  blkdev_ioctl+0x707/0x940
Aug 15 04:04:10 archlinux kernel:  ? blkdev_ioctl+0x707/0x940
Aug 15 04:04:10 archlinux kernel:  block_ioctl+0x3d/0x50
Aug 15 04:04:10 archlinux kernel:  do_vfs_ioctl+0xa5/0x600
Aug 15 04:04:10 archlinux kernel:  ? SYSC_newfstat+0x44/0x70
Aug 15 04:04:10 archlinux kernel:  ? getrawmonotonic64+0x36/0xc0
Aug 15 04:04:10 archlinux kernel:  SyS_ioctl+0x79/0x90
Aug 15 04:04:10 archlinux kernel:  entry_SYSCALL_64_fastpath+0x1a/0xa5
Aug 15 04:04:10 archlinux kernel: RIP: 0033:0x7f2b463378b7
Aug 15 04:04:10 archlinux kernel: RSP: 002b:7fffb2dad8b8 EFLAGS:
0246 ORIG_RAX: 0010
Aug 15 04:04:10 archlinux kernel: RAX: ffda RBX:
00568a3922c8 RCX: 7f2b463378b7
Aug 15 04:04:10 archlinux kernel: RDX: 7fffb2dad910 RSI:
127f RDI: 0003
Aug 15 04:04:10 archlinux kernel: RBP:  R08:
0200 R09: 
Aug 15 04:04:10 archlinux kernel: R10: 7fffb2dad870 R11:
0246 R12: 
Aug 15 04:04:10 archlinux kernel: R13: 0003 R14:
7fffb2dadae8 R15: 

which I do not get if I do `blkdiscard -z` on the underlying device
(which does not support SCSI WRITE SAME) instead of the dm-crypt
container.

In the first trial I got some more lower-level errors. blkdiscard
could not exit after the job was seemingly finished (no more write
according to iostat). The container could not be closed either so I
had to just disconnect the drive. I could not reproduce it in second
trial though, so I am not sure if it was just some coincidental
hardware hiccup. My systemd journal happened to be broken as well so
the log of that trial was lost.

I also have doubt in the approach. Can't we split the bio chain as per
how it was chained and allocate memory bio per bio, and if it's not
enough, also limit the memory allocation with a maximum number
(arbitrary or not) of bios?

On 14 August 2017 at 10:45, Mikulas Patocka  wrote:
> dm-crypt consumes excessive amount memory when the user attempts to zero
> a dm-crypt device with "blkdiscard -z". The command "blkdiscard -z" calls
> the BLKZEROOUT ioctl, it goes to the function __blkdev_issue_zeroout,
> __blkdev_issue_zeroout sends large amount of write bios that contain the
> zero page as their payload.
>
> For each incoming page, dm-crypt allocates another page that holds the
> encrypted data, so when processing "blkdiscard -z", dm-crypt tries to
> allocate the amount of memory that is equal to the size of the device.
> This can trigger OOM killer or cause system crash.
>
> This patch fixes the bug by limiting the amount of memory that dm-crypt
> allocates to 2% of total system memory.
>
> Signed-off-by: Mikulas Patocka 
> Cc: sta...@vger.kernel.org
>
> ---
>  drivers/md/dm-crypt.c |   60 
> +-
>  1 file changed, 59 insertions(+), 1 deletion(-)
>
> Index: linux-2.6/drivers/md/dm-crypt.c
> ===
> --- linux-2.6.orig/drivers/md/dm-crypt.c
> +++ linux-2.6/drivers/md/dm-crypt.c
> @@ -148,6 +148,8 @@ struct crypt_config {
> mempool_t *tag_pool;
> unsigned tag_pool_max_sectors;
>
> +   struct percpu_counter n_allocated_pages;
> +
> struct bio_set *bs;
> struct mutex bio_alloc_lock;
>
> @@ -219,6 +221,12 @@ struct crypt_config {
>  #define MAX_TAG_SIZE   480
>  #define POOL_ENTRY_SIZE512
>
> +static DEFINE_SPINLOCK(dm_crypt_clients_lock);
> +static unsigned dm_crypt_clients_n = 0;
> +static volatile unsigned long dm_crypt_pages_per_client;
> +#define DM_CRYPT_MEMORY_PERCENT2
> +#define DM_CRYPT_MIN_PAGES_PER_CLIENT  (BIO_MAX_PAGES * 16)
> +
>  static void