[PATCH v2] eCryptfs: Write optimization for non-uptodate page

[PATCH v2] eCryptfs: Write optimization for non-uptodate page

[Li Wang]

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 especially 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 on a machine with IntelR Core
T 2 Duo processor, 
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>
Signed-off-by: Yunchuan Wen <wenyunchuan@kylinos.com.cn>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>

--- 

Changes from v1:

The new version adds the code to handle short copy issue which may occure
in iov_iter_copy_from_user_atomic(). The idea is to let ecryptfs_write_end()
return zero,
in order to give iov_iter_fault_in_readable() a chance to handle the page
fault 
for the current iovec, then restart the copy operation.

 fs/ecryptfs/mmap.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index 10ec695..c6f665e 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -350,7 +350,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 "
@@ -360,8 +361,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.
@@ -512,6 +513,13 @@ static int ecryptfs_write_end(struct file *file,
 		}
 		goto out;
 	}
+	if (!PageUptodate(page)) {
+		if (copied < PAGE_CACHE_SIZE) {
+			rc = 0;
+			goto out;
+		}
+		SetPageUptodate(page);
+	}
 	/* Fills in zeros if 'to' goes beyond inode size */
 	rc = fill_zeros_to_end_of_page(page, to);
 	if (rc) {

[PATCH v2] eCryptfs: Write optimization for non-uptodate page

[Li Wang]

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 especially 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 on a machine with IntelR Core
T 2 Duo processor, 
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>
Signed-off-by: Yunchuan Wen <wenyunchuan@kylinos.com.cn>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>

--- 

Changes from v1:

The new version adds the code to handle short copy issue which may occure
in iov_iter_copy_from_user_atomic(). The idea is to let ecryptfs_write_end()
return zero,
in order to give iov_iter_fault_in_readable() a chance to handle the page
fault 
for the current iovec, then restart the copy operation.

 fs/ecryptfs/mmap.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index 10ec695..c6f665e 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -350,7 +350,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 "
@@ -360,8 +361,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.
@@ -512,6 +513,13 @@ static int ecryptfs_write_end(struct file *file,
 		}
 		goto out;
 	}
+	if (!PageUptodate(page)) {
+		if (copied < PAGE_CACHE_SIZE) {
+			rc = 0;
+			goto out;
+		}
+		SetPageUptodate(page);
+	}
 	/* Fills in zeros if 'to' goes beyond inode size */
 	rc = fill_zeros_to_end_of_page(page, to);
 	if (rc) {

[PATCH v2] eCryptfs: Write optimization for non-uptodate page

[Li Wang]

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 especially 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 on a machine with IntelR Core
T 2 Duo processor, 
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>
Signed-off-by: Yunchuan Wen <wenyunchuan@kylinos.com.cn>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>

--- 

Changes from v1:

The new version adds the code to handle short copy issue which may occure
in iov_iter_copy_from_user_atomic(). The idea is to let ecryptfs_write_end()
return zero,
in order to give iov_iter_fault_in_readable() a chance to handle the page
fault 
for the current iovec, then restart the copy operation.

 fs/ecryptfs/mmap.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index 10ec695..c6f665e 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -350,7 +350,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 "
@@ -360,8 +361,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.
@@ -512,6 +513,13 @@ static int ecryptfs_write_end(struct file *file,
 		}
 		goto out;
 	}
+	if (!PageUptodate(page)) {
+		if (copied < PAGE_CACHE_SIZE) {
+			rc = 0;
+			goto out;
+		}
+		SetPageUptodate(page);
+	}
 	/* Fills in zeros if 'to' goes beyond inode size */
 	rc = fill_zeros_to_end_of_page(page, to);
 	if (rc) {

Re: [PATCH v2] eCryptfs: Write optimization for non-uptodate page

[Tyler Hicks]

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

On 2012-02-05 23:33:51, Li Wang wrote:
> 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 especially 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 on a machine with IntelR Core
> T 2 Duo processor, 
> iozone 'write' operation on an existing file with write size being multiple
> of page size will 
> enjoy a steady 3x speedup.

Looks like your email client is incorrectly wrapping some lines.

> 
> Signed-off-by: Li Wang <liwang@nudt.edu.cn>
> Signed-off-by: Yunchuan Wen <wenyunchuan@kylinos.com.cn>
> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
> 
> --- 
> 
> Changes from v1:
> 
> The new version adds the code to handle short copy issue which may occure
> in iov_iter_copy_from_user_atomic(). The idea is to let ecryptfs_write_end()
> return zero,
> in order to give iov_iter_fault_in_readable() a chance to handle the page
> fault 
> for the current iovec, then restart the copy operation.
> 
>  fs/ecryptfs/mmap.c |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
> index 10ec695..c6f665e 100644
> --- a/fs/ecryptfs/mmap.c
> +++ b/fs/ecryptfs/mmap.c
> @@ -350,7 +350,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 "

The patch didn't even apply because of this bad line wrap.

Looks like you've still got mail client issues, but I went ahead and
fixed everything up on my end. I just ask that you get these issues
straightened out soon. `git send-email` is a great way to send patches.
Give it a try.

I really appreciate you taking the time to fix up the commit message!

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/tyhicks/ecryptfs.git#next

BTW, since this isn't a bug fix, it will live in -next until the 3.4
merge window.

Tyler

> @@ -360,8 +361,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.
> @@ -512,6 +513,13 @@ static int ecryptfs_write_end(struct file *file,
>  		}
>  		goto out;
>  	}
> +	if (!PageUptodate(page)) {
> +		if (copied < PAGE_CACHE_SIZE) {
> +			rc = 0;
> +			goto out;
> +		}
> +		SetPageUptodate(page);
> +	}
>  	/* Fills in zeros if 'to' goes beyond inode size */
>  	rc = fill_zeros_to_end_of_page(page, to);
>  	if (rc) {
> 
> --
> 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 --]