[dm-devel] [PATCH v2] dm-crypt: reexport sysfs of kcryptd workqueue

[yangerkun]

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 at 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 at 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;
>