Hi!
Thanks a lot for your reminder! There is a weekend and it also take me
some time to understand what happened recently in workqueue[1][2](I have
a limited understanding of workqueue, so the following statement may not
be correct...), sorry for the late reply!
在 2023/7/8 2:37, Mike Snitzer 写道:
[top-posting because of all the previous top-posting]
Hi,
I certainly would like the ability to allow control over the
workqueues using WQ_SYSFS. But with Tejun's latest WQ_UNBOUND changes
(just merged during 6.5 merge window) I think we'd do well to audit
the flags we're using.
Tejun offered this note in his summary patch header for his 6.5 changes:
"Alasdair Kergon, Mike Snitzer, DM folks
---------------------------------------
I ran fio on top of dm-crypt to compare performance of different
configurations. It mostly behaved as expected but please let me know if
anything doens't look right. Also, DM_CRYPT_SAME_CPU can now be implemented
by applying strict "cpu" scope on the unbound workqueue and it would make
sense to add WQ_SYSFS to the kcryptd workqueue so that users can tune the
settings on the fly."
Does Hejun means that DM_CRYPT_SAME_CPU will use WQ_CPU_INTENSIVE on
bounded workqueue, and this can trigger the warning added by
636384500520("workqueue: Report work funcs that trigger automatic
CPU_INTENSIVE mechanism"), so advice from Hejun was that we can
implement DM_CRYPT_SAME_CPU by applying strict "cpu" scope on the
unbound workqueue? But it doesn't seem very easy since what
DM_CRYPT_SAME_CPU want to do was using the exactly cpu what we call
queue_work, applying strict "cpu" can't achieve the same effect...
Anyway, I'd welcome you rebasing your patch ontop of Linus's latest
linux.git. Then we (Mikulas, you, and/or I) can take a closer look at
addressing Tejun's DM_CRYPT_SAME_CPU suggestion.
Fine, I will do it later, and thanks again for your reply!
Thanks,
Yang Erkun.
Thanks,
Mike
On Mon, Jun 26 2023 at 4:43P -0400,
yangerkun <yangerkun@xxxxxxxxxxxxxxx> wrote:
Hi, Mike,
Sorry for the noise. This patch has been proposed for a long time. I wonder
does there any suggestion for the patch. Looking forward to your reply!
Thanks,
Yang Erkun.
在 2023/3/25 9:01, yangerkun 写道:
Ping...
在 2023/3/1 11:29, yangerkun 写道:
From: yangerkun <yangerkun@xxxxxxxxxx>
Once there is a heavy IO load, so many encrypt/decrypt work will occupy
all of the cpu, which may lead the poor performance for other service.
So the idea like 'a2b8b2d97567 ("dm crypt: export sysfs of kcryptd
workqueue")' said seems necessary. We can export "kcryptd" workqueue
sysfs, the entry like cpumask/max_active and so on can help us to limit
the usage for encrypt/decrypt work.
However, that commit does not consider the reload table will call .ctr
before .dtr, so the reload for dm-crypt will fail since the same sysfs
problem, and then we revert that commit('48b0777cd93d ("Revert "dm
crypt: export sysfs of kcryptd workqueue"")').
Actually, what we should do is give a unique name once we try reload
table, we can use ida to fix the problem.
Signed-off-by: yangerkun <yangerkun@xxxxxxxxxx>
---
drivers/md/dm-crypt.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
v1->v2:
rewritten the commit msg
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 40cb1719ae4d..948d1e11d064 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -47,6 +47,8 @@
#define DM_MSG_PREFIX "crypt"
+static DEFINE_IDA(crypt_queue_ida);
+
/*
* context holding the current state of a multi-part conversion
*/
@@ -180,6 +182,7 @@ struct crypt_config {
struct crypto_aead **tfms_aead;
} cipher_tfm;
unsigned int tfms_count;
+ int crypt_queue_id;
unsigned long cipher_flags;
/*
@@ -2704,6 +2707,9 @@ static void crypt_dtr(struct dm_target *ti)
if (cc->crypt_queue)
destroy_workqueue(cc->crypt_queue);
+ if (cc->crypt_queue_id)
+ ida_free(&crypt_queue_ida, cc->crypt_queue_id);
+
crypt_free_tfms(cc);
bioset_exit(&cc->bs);
@@ -3340,12 +3346,24 @@ static int crypt_ctr(struct dm_target *ti,
unsigned int argc, char **argv)
}
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,
1, devname);
- else
- cc->crypt_queue = alloc_workqueue("kcryptd/%s",
- WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM |
WQ_UNBOUND,
- num_online_cpus(), devname);
+ else {
+ int id = ida_alloc_min(&crypt_queue_ida, 1, GFP_KERNEL);
+
+ if (id < 0) {
+ ti->error = "Couldn't get kcryptd queue id";
+ ret = id;
+ goto bad;
+ }
+
+ cc->crypt_queue_id = id;
+ cc->crypt_queue = alloc_workqueue("kcryptd-%s-%d",
+ WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM |
+ WQ_UNBOUND | WQ_SYSFS,
+ num_online_cpus(), devname, id);
+ }
+
if (!cc->crypt_queue) {
ti->error = "Couldn't create kcryptd queue";
goto bad;
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
