On Wed, Jun 13, 2012 at 11:53 AM, Thieu Le <thieule@xxxxxxxxxx> wrote:
>
> Hi Tyler, I believe the performance improvement from the async
> interface comes from the ability to fully utilize the crypto
> hardware.
>
> Firstly, being able to submit multiple outstanding requests fills
> the crypto engine pipeline which allows it to run more efficiently
> (ie. minimal cycles are wasted waiting for the next crypto request).
> This perf improvement is similar to network transfer efficiency.
> Sending a 1GB file via 4K packets synchronously is not going to
> fully saturate a gigabit link but queuing a bunch of 4K packets to
> send will.
Ok, it is clicking for me now. Additionally, I imagine that the async
interface helps in the multicore/multiprocessor case.
> Secondly, if you have crypto hardware that has multiple crypto
> engines, then the multiple outstanding requests allow the crypto
> hardware to put all of those engines to work.
>
> To answer your question about page_crypt_req, it is used to track
> all of the extent_crypt_reqs for a particular page. When we write a
> page, we break the page up into extents and encrypt each extent.
> For each extent, we submit the encrypt request using
> extent_crypt_req. To determine when the entire page has been
> encrypted, we create one page_crypt_req and associates the
> extent_crypt_req to the page by incrementing
> page_crypt_req::num_refs. As the extent encrypt request completes,
> we decrement num_refs. The entire page is encrypted when num_refs
> goes to zero, at which point, we end the page writeback.
Alright, that is what I had understood from reviewing the code. No
surprises there.
What I'm suggesting is to do away with the page_crypt_req and simply have
ecryptfs_encrypt_page_async() keep track of the extent_crypt_reqs for
the page it is encrypting. Its prototype would look like this:
int ecryptfs_encrypt_page_async(struct page *page);
An example of how it would be called would be something like this:
static int ecryptfs_writepage(struct page *page, struct writeback_control *wbc)
{
int rc = 0;
/*
* Refuse to write the page out if we are called from reclaim context
* since our writepage() path may potentially allocate memory when
* calling into the lower fs vfs_write() which may in turn invoke
* us again.
*/
if (current->flags & PF_MEMALLOC) {
redirty_page_for_writepage(wbc, page);
goto out;
}
set_page_writeback(page);
rc = ecryptfs_encrypt_page_async(page);
if (unlikely(rc)) {
ecryptfs_printk(KERN_WARNING, "Error encrypting "
"page (upper index [0x%.16lx])\n", page->index);
ClearPageUptodate(page);
SetPageError(page);
} else {
SetPageUptodate(page);
}
end_page_writeback(page);
out:
unlock_page(page);
return rc;
}
> We can get rid of page_crypt_req if we can guarantee that the extent
> size and page size are the same.
We can't guarantee that but that doesn't matter because
ecryptfs_encrypt_page_async() already handles that problem. Its caller doesn't
care if the extent size is less than the page size.
Tyler
Attachment:
signature.asc
Description: Digital signature
