Re: [PATCH v2] eCryptfs: write optimization

['Tyler Hicks']

On 2012-01-26 13:04:34, Li Wang wrote:
> Hi,
>   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,
> therefore, give iov_iter_fault_in_readable() a chance to handle the page fault 
> for the current iovec, then restart the copy operation.

Hi Li - This approach looks like it will work. Thanks for reworking the
patch! I've been testing it and it seems to be holding up.

I'd like you to respin the patch one last time to fix up the commit
message. You've gotten several eCryptfs patches in now, and I expect
more from you in the future, so I'd like to be able to simply apply
your patches rather than having to fix up the commit message myself.

Keep in mind that the commit message should make sense to someone
reading the "git log" years from now. This particular message only makes
sense if someone looks at my review of v1 of this patch. You would want
to put this type of text underneath the --- separator below because that
text automatically gets stripped out when I used "git am" to apply your
patch.

Check out Documentation/SubmittingPatches for suggestions on writing
good commit messages.

> 
> Cheers,
> Li Wang
> 
> Signed-off-by: Li Wang <liwang@xxxxxxxxxxx>
>                Yunchuan Wen <wenyunchuan@xxxxxxxxxxxxxx>

You always seem to list Yunchuan. You'll need another Signed-off-by: tag
on that line. Does Yunchuan help in the development of the patches or
just perform review? See Documentation/development-process/5.Posting to
identify the correct tag to use for either situation. Also, you may want
to cc Yunchuan on these patches.

Sorry to hold up this patch with "process" stuff, but getting the right
commit message is important. Thanks!

Tyler

> 
> ---
>  
> 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 6a44148..b3fa499 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.
> @@ -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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Attachment: signature.asc
Description: Digital signature