Re: [PATCH] dm-crypt: add the "high_priority" flag

yangerkun <yangerkun@xxxxxxxxxxxxxxx> · Tue, 7 Mar 2023 09:30:20 +0800

在 2023/3/7 3:01, Mikulas Patocka 写道:


On Mon, 6 Mar 2023, Mikulas Patocka wrote:

Hi,

Thanks a lot for your review!

It's ok to fix the softlockup, but for async write encrypt,
kcryptd_crypt_write_io_submit will add bio to write_tree, and once we
call cond_resched before every kcryptd_io_write, the write performance
may be poor while we meet a high cpu usage scene.

Hi

To fix this problem, find the PID of the process "dmcrypt_write" and
change its priority to -20, for example "renice -n -20 -p 34748".

This is the proper way how to fix it; locking up the process for one
second is not.

We used to have high-priority workqueues by default, but it caused audio
playback skipping, so we had to revert it - see
f612b2132db529feac4f965f28a1b9258ea7c22b.

Perhaps we should add an option to have high-priority kernel threads?

Mikulas

Here I'm sending a patch that makes it optional to raise the priority
of dm-crypt workqueues and the write thread. Test it, if it helps for you.

Thanks, will test it!

Mikulas



From: Mikulas Patocka <mpatocka@xxxxxxxxxx>

It was reported that dm-crypt performs badly when the system is loaded[1].
So I'm adding an option "high_priority" that sets the workqueues and the
write_thread to nice level -20. This used to be the default in the past,
but it caused audio skipping, so it had to be reverted - see the commit
f612b2132db529feac4f965f28a1b9258ea7c22b. This commit makes it optional,
so that normal users won't be harmed by it.

[1] https://listman.redhat.com/archives/dm-devel/2023-February/053410.html

Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>

---
  Documentation/admin-guide/device-mapper/dm-crypt.rst |    5 +++
  drivers/md/dm-crypt.c                                |   28 +++++++++++++------
  2 files changed, 25 insertions(+), 8 deletions(-)

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
@@ -133,9 +133,9 @@ struct iv_elephant_private {
   * and encrypts / decrypts at the same time.
   */
  enum flags { DM_CRYPT_SUSPENDED, DM_CRYPT_KEY_VALID,
-	     DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD,
-	     DM_CRYPT_NO_READ_WORKQUEUE, DM_CRYPT_NO_WRITE_WORKQUEUE,
-	     DM_CRYPT_WRITE_INLINE };
+	     DM_CRYPT_SAME_CPU, DM_CRYPT_HIGH_PRIORITY,
+	     DM_CRYPT_NO_OFFLOAD, DM_CRYPT_NO_READ_WORKQUEUE,
+	     DM_CRYPT_NO_WRITE_WORKQUEUE, DM_CRYPT_WRITE_INLINE };
  
  enum cipher_flags {
  	CRYPT_MODE_INTEGRITY_AEAD,	/* Use authenticated mode for cipher */
@@ -3087,7 +3087,7 @@ static int crypt_ctr_optional(struct dm_
  	struct crypt_config *cc = ti->private;
  	struct dm_arg_set as;
  	static const struct dm_arg _args[] = {
-		{0, 8, "Invalid number of feature args"},
+		{0, 9, "Invalid number of feature args"},
  	};
  	unsigned int opt_params, val;
  	const char *opt_string, *sval;
@@ -3114,6 +3114,8 @@ static int crypt_ctr_optional(struct dm_
  
  		else if (!strcasecmp(opt_string, "same_cpu_crypt"))
  			set_bit(DM_CRYPT_SAME_CPU, &cc->flags);
+		else if (!strcasecmp(opt_string, "high_priority"))
+			set_bit(DM_CRYPT_HIGH_PRIORITY, &cc->flags);
  
  		else if (!strcasecmp(opt_string, "submit_from_crypt_cpus"))
  			set_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
@@ -3352,18 +3354,22 @@ static int crypt_ctr(struct dm_target *t
  	}
  
  	ret = -ENOMEM;
-	cc->io_queue = alloc_workqueue("kcryptd_io/%s", WQ_MEM_RECLAIM, 1, devname);
+	cc->io_queue = alloc_workqueue("kcryptd_io/%s", WQ_MEM_RECLAIM |
+				       test_bit(DM_CRYPT_HIGH_PRIORITY, &cc->flags) * WQ_HIGHPRI,
+				       1, devname);
  	if (!cc->io_queue) {
  		ti->error = "Couldn't create kcryptd io queue";
  		goto bad;
  	}
  
  	if (test_bit(DM_CRYPT_SAME_CPU, &cc->flags))
-		cc->crypt_queue = alloc_workqueue("kcryptd/%s", WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM,
+		cc->crypt_queue = alloc_workqueue("kcryptd/%s", WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM |
+						  test_bit(DM_CRYPT_HIGH_PRIORITY, &cc->flags) * WQ_HIGHPRI,
  						  1, devname);
  	else
  		cc->crypt_queue = alloc_workqueue("kcryptd/%s",
-						  WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM | WQ_UNBOUND,
+						  WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM | WQ_UNBOUND |
+						  test_bit(DM_CRYPT_HIGH_PRIORITY, &cc->flags) * WQ_HIGHPRI,
  						  num_online_cpus(), devname);
  	if (!cc->crypt_queue) {
  		ti->error = "Couldn't create kcryptd queue";
@@ -3380,6 +3386,8 @@ static int crypt_ctr(struct dm_target *t
  		ti->error = "Couldn't spawn write thread";
  		goto bad;
  	}
+	if (test_bit(DM_CRYPT_HIGH_PRIORITY, &cc->flags))
+		set_user_nice(cc->write_thread, MIN_NICE);
  
  	ti->num_flush_bios = 1;
  	ti->limit_swap_bios = true;
@@ -3500,6 +3508,7 @@ static void crypt_status(struct dm_targe
  
  		num_feature_args += !!ti->num_discard_bios;
  		num_feature_args += test_bit(DM_CRYPT_SAME_CPU, &cc->flags);
+		num_feature_args += test_bit(DM_CRYPT_HIGH_PRIORITY, &cc->flags);
  		num_feature_args += test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
  		num_feature_args += test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags);
  		num_feature_args += test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags);
@@ -3513,6 +3522,8 @@ static void crypt_status(struct dm_targe
  				DMEMIT(" allow_discards");
  			if (test_bit(DM_CRYPT_SAME_CPU, &cc->flags))
  				DMEMIT(" same_cpu_crypt");
+			if (test_bit(DM_CRYPT_HIGH_PRIORITY, &cc->flags))
+				DMEMIT(" high_priority");
  			if (test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags))
  				DMEMIT(" submit_from_crypt_cpus");
  			if (test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags))
@@ -3532,6 +3543,7 @@ static void crypt_status(struct dm_targe
  		DMEMIT_TARGET_NAME_VERSION(ti->type);
  		DMEMIT(",allow_discards=%c", ti->num_discard_bios ? 'y' : 'n');
  		DMEMIT(",same_cpu_crypt=%c", test_bit(DM_CRYPT_SAME_CPU, &cc->flags) ? 'y' : 'n');
+		DMEMIT(",high_priority=%c", test_bit(DM_CRYPT_HIGH_PRIORITY, &cc->flags) ? 'y' : 'n');
  		DMEMIT(",submit_from_crypt_cpus=%c", test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags) ?
  		       'y' : 'n');
  		DMEMIT(",no_read_workqueue=%c", test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags) ?
@@ -3659,7 +3671,7 @@ static void crypt_io_hints(struct dm_tar
  
  static struct target_type crypt_target = {
  	.name   = "crypt",
-	.version = {1, 24, 0},
+	.version = {1, 25, 0},
  	.module = THIS_MODULE,
  	.ctr    = crypt_ctr,
  	.dtr    = crypt_dtr,
Index: linux-2.6/Documentation/admin-guide/device-mapper/dm-crypt.rst
===================================================================
--- linux-2.6.orig/Documentation/admin-guide/device-mapper/dm-crypt.rst
+++ linux-2.6/Documentation/admin-guide/device-mapper/dm-crypt.rst
@@ -113,6 +113,11 @@ same_cpu_crypt
      The default is to use an unbound workqueue so that encryption work
      is automatically balanced between available CPUs.
  
+high_priority
+    Set dm-crypt workqueues and the writer thread to high priority. This
+    improves throughput and latency of dm-crypt while degrading general
+    responsiveness of the system.
+
  submit_from_crypt_cpus
      Disable offloading writes to a separate thread after encryption.
      There are some situations where offloading write bios from the


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







