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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20120205184313.GA4020@boyd>
Date:	Sun, 5 Feb 2012 12:43:14 -0600
From:	Tyler Hicks <tyhicks@...onical.com>
To:	Li Wang <liwang@...t.edu.cn>
Cc:	ecryptfs@...r.kernel.org, wenyunchuan@...inos.com.cn,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2] eCryptfs: Write optimization for non-uptodate page

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@...t.edu.cn>
> Signed-off-by: Yunchuan Wen <wenyunchuan@...inos.com.cn>
> Reviewed-by: Tyler Hicks <tyhicks@...onical.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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)