On 2012-01-25 23:08:58, Li Wang wrote:
> Hi,
> ecryptfs_write_begin() grabs a page from page cache for writing.
> If the page does not contain valid data, or the data are older than
> the counterpart on the disk, eCryptfs will read out the corresponding data
> from the disk into the eCryptfs page cache, decrypt them,
> then perform writing. However, for current page, if the length of
> the data to be written into is equal to page size, that means the whole
> page of data will be overwritten, in which case, it does not matter whatever
> the data were before, it is beneficial to perform writing directly.
>
> This is useful while using eCryptfs in backup situation, user copies file
> out from eCryptfs folder, modifies, and copies the revised file back to
> replace the original one.
>
> With this optimization, according to our test, iozone 'write' operation on an existing
> file with write size being multiple of page size will enjoy a steady 3x speedup.
I've coded this same optimization up once before and did see the nice
performance boost it provides.
What stopped me from committing it is that I noticed a short copy is
possible during the iov_iter_copy_from_user_atomic() call in
generic_perform_write().
I didn't ever follow up to figure out the best way to handle the short
copy in ecryptfs_write_end() and then forgot about it all. :/
How do you propose that case be handled? I suppose we could *detect* it
by not marking the page uptodate in ecryptfs_write_begin() and then if
ecryptfs_write_end() sees a non-uptodate page and len is less than
PAGE_CACHE_SIZE, we would know a short copy happened. But I'm not sure
what the acceptable options are to *handle* that case. Just return an
error? Try to fill the page in ecryptfs_write_end() (ugly, IMO)?
Adding linux-fsdevel for suggestions, since I assume other filesystems
may be doing this.
Tyler
>
> Signed-off-by: Li Wang <liwang@xxxxxxxxxxx>
> Yunchuan Wen <wenyunchuan@xxxxxxxxxxxxxx>
>
> ---
>
> fs/ecryptfs/mmap.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
> index 6a44148..9724ef2 100644
> --- a/fs/ecryptfs/mmap.c
> +++ b/fs/ecryptfs/mmap.c
> @@ -346,7 +346,8 @@ static int ecryptfs_write_begin(struct file *file,
> if (prev_page_end_size
> >= i_size_read(page->mapping->host)) {
> zero_user(page, 0, PAGE_CACHE_SIZE);
> - } else {
> + SetPageUptodate(page);
> + } else if (len < PAGE_CACHE_SIZE) {
> rc = ecryptfs_decrypt_page(page);
> if (rc) {
> printk(KERN_ERR "%s: Error decrypting "
> @@ -356,8 +357,8 @@ static int ecryptfs_write_begin(struct file *file,
> ClearPageUptodate(page);
> goto out;
> }
> + SetPageUptodate(page);
> }
> - SetPageUptodate(page);
> }
> }
> /* If creating a page or more of holes, zero them out via truncate.
>
> --
> To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: Digital signature
