| Message ID | 20230301032904.3561641-1-yangerkun@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] dm-crypt: reexport sysfs of kcryptd workqueue | expand |
Ping... 在 2023/3/1 11:29, yangerkun 写道: > From: yangerkun <yangerkun@huawei.com> > > 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@huawei.com> > --- > 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@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
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@huawei.com> >> >> 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@huawei.com> >> --- >> 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@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
[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." 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. Thanks, Mike On Mon, Jun 26 2023 at 4:43P -0400, yangerkun <yangerkun@huaweicloud.com> 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@huawei.com> > > > > > > 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@huawei.com> > > > --- > > > 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@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
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@huaweicloud.com> 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@huawei.com> >>>> >>>> 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@huawei.com> >>>> --- >>>> 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@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
在 2023/7/10 20:22, yangerkun 写道: > 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! Forget to add the link... Sorry... [1]. https://lore.kernel.org/lkml/ZJoG-BptcGoHII35@slm.duckdns.org/T/ [2]. https://lore.kernel.org/lkml/20230519001709.2563-8-tj@kernel.org/T/ > > > 在 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@huaweicloud.com> 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@huawei.com> >>>>> >>>>> 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@huawei.com> >>>>> --- >>>>> 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@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
Hello, Mike. On Fri, Jul 07, 2023 at 02:37:38PM -0400, Mike Snitzer wrote: > 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 That part didn't get merged yet. I'm still waiting on some benchmark results to verify that the change won't cause regressions which are difficult to work around. The target is the next merge window. Thanks.
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;