* [dm-crypt] Hang problem with dm-crypt
@ 2016-09-26 6:50 Yu, Wenqian
2016-09-26 10:27 ` Milan Broz
0 siblings, 1 reply; 5+ messages in thread
From: Yu, Wenqian @ 2016-09-26 6:50 UTC (permalink / raw)
To: dm-crypt; +Cc: Yu, Wenqian
[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]
Hi, all,
I tried to use dm-crypt for disk encryption with accelerators and found that it will hang when accelerator returned EBUSY, which means the driver request queue is full.
Per the logic in crypt_convert(), the request will be skipped if the request is not sent to crypto driver when the driver request queue is full. Is this expected behavior?
In crypt_convert_block(), the sector is advanced (bio_advance_iter()) no matter whether crypto_skcipher_encrypt()/crypto_skcipher_decrypt() send the request to accelerator driver or not. When the driver request queue is full, EBUSY will be returned from crypto_skcipher_encrypt()/crypto_skcipher_decrypt(). And in crypt_convert(), the existing implementation is waiting for a completion from a request, which is not queued in the driver when EBUSY is encountered from crypt_convert_block (). In this case, the sector should not be advanced or should be rolled back as the request is not sent to accelerator driver.
Thanks,
- Wenqian
[-- Attachment #2: Type: text/html, Size: 5771 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dm-crypt] Hang problem with dm-crypt
2016-09-26 6:50 [dm-crypt] Hang problem with dm-crypt Yu, Wenqian
@ 2016-09-26 10:27 ` Milan Broz
2016-09-26 13:08 ` Yu, Wenqian
0 siblings, 1 reply; 5+ messages in thread
From: Milan Broz @ 2016-09-26 10:27 UTC (permalink / raw)
To: Yu, Wenqian, dm-crypt
On 09/26/2016 08:50 AM, Yu, Wenqian wrote:
> I tried to use dm-crypt for disk encryption with accelerators and
> found that it will hang when accelerator returned EBUSY, which means
> the driver request queue is full.
That is normal state, when request is processed asynchronously later.
Please read explicit comments in code we added to understand this logic.
added in this commit:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-crypt.c?id=54cea3f6681ad9360814e2926d1f723bbd0f74ed
> Per the logic in crypt_convert(), the request will be skipped if the
> request is not sent to crypto driver when the driver request queue is
> full. Is this expected behavior?
It is not skipped, it is queued (or it waits if queue is full and then
processes as asynchronous branch (EINPROGRESS))
> In crypt_convert_block(), the sector is advanced (bio_advance_iter())
> no matter whether crypto_skcipher_encrypt()/crypto_skcipher_decrypt()
> send the request to accelerator driver or not. When the driver
> request queue is full, EBUSY will be returned from
> crypto_skcipher_encrypt()/crypto_skcipher_decrypt(). And in
> crypt_convert(), the existing implementation is waiting for a
> completion from a request, which is not queued in the driver when
> EBUSY is encountered from crypt_convert_block (). In this case, the
> sector should not be advanced or should be rolled back as the request
> is not sent to accelerator driver.
I think it should be queued (IOW the one that returns BUSY should be queued).
If it is not done, I would say it is bug in acceleration driver.
Note this flag:
/*
* Use REQ_MAY_BACKLOG so a cipher driver internally backlogs
* requests if driver request queue is full.
*/
Anyway, this is more question for crypto API mailing list...
I think that dmcrypt processing is correct here.
Milan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dm-crypt] Hang problem with dm-crypt
2016-09-26 10:27 ` Milan Broz
@ 2016-09-26 13:08 ` Yu, Wenqian
2016-09-27 6:44 ` Milan Broz
0 siblings, 1 reply; 5+ messages in thread
From: Yu, Wenqian @ 2016-09-26 13:08 UTC (permalink / raw)
To: Milan Broz, dm-crypt; +Cc: Yu, Wenqian
Hi, Milan,
Thanks for the detail information. I noticed the comments and the underlying design logic.
In dm-crypt existing design, there is an assumption that the acceleration driver can queue the requests which are not sent to hardware.
I think there are at least two scenarios we should consider to make it more robust.
1. The queue is full even if the driver has the ability to queue a number of the requests.
2. The acceleration hardware/driver doesn't have the ability to queue the requests.
Should we add other error code to handle this?
Thanks,
- Wenqian
-----Original Message-----
From: Milan Broz [mailto:gmazyland@gmail.com]
Sent: Monday, September 26, 2016 6:28 PM
To: Yu, Wenqian; dm-crypt@saout.de
Subject: Re: [dm-crypt] Hang problem with dm-crypt
On 09/26/2016 08:50 AM, Yu, Wenqian wrote:
> I tried to use dm-crypt for disk encryption with accelerators and
> found that it will hang when accelerator returned EBUSY, which means
> the driver request queue is full.
That is normal state, when request is processed asynchronously later.
Please read explicit comments in code we added to understand this logic.
added in this commit:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-crypt.c?id=54cea3f6681ad9360814e2926d1f723bbd0f74ed
> Per the logic in crypt_convert(), the request will be skipped if the
> request is not sent to crypto driver when the driver request queue is
> full. Is this expected behavior?
It is not skipped, it is queued (or it waits if queue is full and then processes as asynchronous branch (EINPROGRESS))
> In crypt_convert_block(), the sector is advanced (bio_advance_iter())
> no matter whether crypto_skcipher_encrypt()/crypto_skcipher_decrypt()
> send the request to accelerator driver or not. When the driver
> request queue is full, EBUSY will be returned from
> crypto_skcipher_encrypt()/crypto_skcipher_decrypt(). And in
> crypt_convert(), the existing implementation is waiting for a
> completion from a request, which is not queued in the driver when
> EBUSY is encountered from crypt_convert_block (). In this case, the
> sector should not be advanced or should be rolled back as the request
> is not sent to accelerator driver.
I think it should be queued (IOW the one that returns BUSY should be queued).
If it is not done, I would say it is bug in acceleration driver.
Note this flag:
/*
* Use REQ_MAY_BACKLOG so a cipher driver internally backlogs
* requests if driver request queue is full.
*/
Anyway, this is more question for crypto API mailing list...
I think that dmcrypt processing is correct here.
Milan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dm-crypt] Hang problem with dm-crypt
@ 2016-09-27 6:44 ` Milan Broz
0 siblings, 0 replies; 5+ messages in thread
From: Milan Broz @ 2016-09-27 6:44 UTC (permalink / raw)
To: Yu, Wenqian, dm-crypt, device-mapper development
On 09/26/2016 03:08 PM, Yu, Wenqian wrote:
> Hi, Milan,
>
> Thanks for the detail information. I noticed the comments and the underlying design logic.
>
> In dm-crypt existing design, there is an assumption that the acceleration driver can queue the requests which are not sent to hardware.
>
> I think there are at least two scenarios we should consider to make it more robust.
> 1. The queue is full even if the driver has the ability to queue a number of the requests.
> 2. The acceleration hardware/driver doesn't have the ability to queue the requests.
>
> Should we add other error code to handle this?
I would prefer to ask on crypto API mailing list how the interface is expected to behave
(if the queue/REQ_MAY_BACKLOG is mandatory etc) before complicating any logic in dmcrypt.
Also please send any possible patches to dm-devel mailing list (I added cc there now).
Thanks,
Milan
>
> Thanks,
> - Wenqian
>
> -----Original Message-----
> From: Milan Broz [mailto:gmazyland@gmail.com]
> Sent: Monday, September 26, 2016 6:28 PM
> To: Yu, Wenqian; dm-crypt@saout.de
> Subject: Re: [dm-crypt] Hang problem with dm-crypt
>
> On 09/26/2016 08:50 AM, Yu, Wenqian wrote:
>> I tried to use dm-crypt for disk encryption with accelerators and
>> found that it will hang when accelerator returned EBUSY, which means
>> the driver request queue is full.
>
> That is normal state, when request is processed asynchronously later.
>
> Please read explicit comments in code we added to understand this logic.
> added in this commit:
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-crypt.c?id=54cea3f6681ad9360814e2926d1f723bbd0f74ed
>
>> Per the logic in crypt_convert(), the request will be skipped if the
>> request is not sent to crypto driver when the driver request queue is
>> full. Is this expected behavior?
>
> It is not skipped, it is queued (or it waits if queue is full and then processes as asynchronous branch (EINPROGRESS))
>
>> In crypt_convert_block(), the sector is advanced (bio_advance_iter())
>> no matter whether crypto_skcipher_encrypt()/crypto_skcipher_decrypt()
>> send the request to accelerator driver or not. When the driver
>> request queue is full, EBUSY will be returned from
>> crypto_skcipher_encrypt()/crypto_skcipher_decrypt(). And in
>> crypt_convert(), the existing implementation is waiting for a
>> completion from a request, which is not queued in the driver when
>> EBUSY is encountered from crypt_convert_block (). In this case, the
>> sector should not be advanced or should be rolled back as the request
>> is not sent to accelerator driver.
>
> I think it should be queued (IOW the one that returns BUSY should be queued).
> If it is not done, I would say it is bug in acceleration driver.
> Note this flag:
> /*
> * Use REQ_MAY_BACKLOG so a cipher driver internally backlogs
> * requests if driver request queue is full.
> */
>
> Anyway, this is more question for crypto API mailing list...
> I think that dmcrypt processing is correct here.
>
> Milan
> _______________________________________________
> dm-crypt mailing list
> dm-crypt@saout.de
> http://www.saout.de/mailman/listinfo/dm-crypt
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Hang problem with dm-crypt
@ 2016-09-27 6:44 ` Milan Broz
0 siblings, 0 replies; 5+ messages in thread
From: Milan Broz @ 2016-09-27 6:44 UTC (permalink / raw)
To: Yu, Wenqian, dm-crypt-4q3lyFh4P1g, device-mapper development
On 09/26/2016 03:08 PM, Yu, Wenqian wrote:
> Hi, Milan,
>
> Thanks for the detail information. I noticed the comments and the underlying design logic.
>
> In dm-crypt existing design, there is an assumption that the acceleration driver can queue the requests which are not sent to hardware.
>
> I think there are at least two scenarios we should consider to make it more robust.
> 1. The queue is full even if the driver has the ability to queue a number of the requests.
> 2. The acceleration hardware/driver doesn't have the ability to queue the requests.
>
> Should we add other error code to handle this?
I would prefer to ask on crypto API mailing list how the interface is expected to behave
(if the queue/REQ_MAY_BACKLOG is mandatory etc) before complicating any logic in dmcrypt.
Also please send any possible patches to dm-devel mailing list (I added cc there now).
Thanks,
Milan
>
> Thanks,
> - Wenqian
>
> -----Original Message-----
> From: Milan Broz [mailto:gmazyland-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> Sent: Monday, September 26, 2016 6:28 PM
> To: Yu, Wenqian; dm-crypt-4q3lyFh4P1g@public.gmane.org
> Subject: Re: [dm-crypt] Hang problem with dm-crypt
>
> On 09/26/2016 08:50 AM, Yu, Wenqian wrote:
>> I tried to use dm-crypt for disk encryption with accelerators and
>> found that it will hang when accelerator returned EBUSY, which means
>> the driver request queue is full.
>
> That is normal state, when request is processed asynchronously later.
>
> Please read explicit comments in code we added to understand this logic.
> added in this commit:
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-crypt.c?id=54cea3f6681ad9360814e2926d1f723bbd0f74ed
>
>> Per the logic in crypt_convert(), the request will be skipped if the
>> request is not sent to crypto driver when the driver request queue is
>> full. Is this expected behavior?
>
> It is not skipped, it is queued (or it waits if queue is full and then processes as asynchronous branch (EINPROGRESS))
>
>> In crypt_convert_block(), the sector is advanced (bio_advance_iter())
>> no matter whether crypto_skcipher_encrypt()/crypto_skcipher_decrypt()
>> send the request to accelerator driver or not. When the driver
>> request queue is full, EBUSY will be returned from
>> crypto_skcipher_encrypt()/crypto_skcipher_decrypt(). And in
>> crypt_convert(), the existing implementation is waiting for a
>> completion from a request, which is not queued in the driver when
>> EBUSY is encountered from crypt_convert_block (). In this case, the
>> sector should not be advanced or should be rolled back as the request
>> is not sent to accelerator driver.
>
> I think it should be queued (IOW the one that returns BUSY should be queued).
> If it is not done, I would say it is bug in acceleration driver.
> Note this flag:
> /*
> * Use REQ_MAY_BACKLOG so a cipher driver internally backlogs
> * requests if driver request queue is full.
> */
>
> Anyway, this is more question for crypto API mailing list...
> I think that dmcrypt processing is correct here.
>
> Milan
> _______________________________________________
> dm-crypt mailing list
> dm-crypt-4q3lyFh4P1g@public.gmane.org
> http://www.saout.de/mailman/listinfo/dm-crypt
>
_______________________________________________
dm-crypt mailing list
dm-crypt-4q3lyFh4P1g@public.gmane.org
http://www.saout.de/mailman/listinfo/dm-crypt
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-27 6:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-26 6:50 [dm-crypt] Hang problem with dm-crypt Yu, Wenqian
2016-09-26 10:27 ` Milan Broz
2016-09-26 13:08 ` Yu, Wenqian
2016-09-27 6:44 ` Milan Broz
2016-09-27 6:44 ` Milan Broz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.