[PATCH] eCryptfs: write optimization

[PATCH] eCryptfs: write optimization

[Li Wang]

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.

Signed-off-by: Li Wang <liwang@nudt.edu.cn>
		Yunchuan Wen <wenyunchuan@kylinos.com.cn>

---

 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.  

[PATCH] eCryptfs: write optimization

[Li Wang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="GBK", Size: 2014 bytes --]

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 writting. 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 writting 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.

Signed-off-by: Li Wang <liwang@nudt.edu.cn>
		Yunchuan Wen <wenyunchuan@kylinos.com.cn>

---

 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.  ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] eCryptfs: write optimization

[Tyler Hicks]

[-- Attachment #1: Type: text/plain, Size: 3179 bytes --]

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@nudt.edu.cn>
> 		Yunchuan Wen <wenyunchuan@kylinos.com.cn>
> 
> ---
> 
>  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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]