[PATCH] Convert ecryptfs to use new aops

[PATCH] Convert ecryptfs to use new aops

[Badari Pulavarty]

Hi Andrew,

As part of moving all the filesystems to make use of new aops to
eventually remove prepare/commit_write, here is the patch to convert
ecryptfs to make use of new aops. 

Originally written by Nick, I did few minor fixes and clean ups.
Michael Halcrow tested and confirmed that it works.

Thanks,
Badari


Convert ecryptfs to use new aops (write_begin/write_end)

Signed-off-by: Nick Piggin <npiggin@suse.de>
Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
Acked-by: Michael Halcrow <mhalcrow@us.ibm.com>

 fs/ecryptfs/mmap.c |   81 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 50 insertions(+), 31 deletions(-)

Index: linux-2.6.27-rc7/fs/ecryptfs/mmap.c
===================================================================
--- linux-2.6.27-rc7.orig/fs/ecryptfs/mmap.c	2008-09-21 15:29:55.000000000 -0700
+++ linux-2.6.27-rc7/fs/ecryptfs/mmap.c	2008-09-24 17:21:58.000000000 -0700
@@ -265,22 +265,34 @@ out:
 }
 
 /**
- * ecryptfs_prepare_write
+ * ecryptfs_write_begin
  * @file: The eCryptfs file
- * @page: The eCryptfs page
- * @from: The start byte from which we will write
- * @to: The end byte to which we will write
+ * @mapping: The eCryptfs object
+ * @pos: The file offset at which to start writing
+ * @len: Length of the write
+ * @flags: Various flags
+ * @pagep: Pointer to return the page
+ * @fsdata: Pointer to return fs data (unused)
  *
  * This function must zero any hole we create
  *
  * Returns zero on success; non-zero otherwise
  */
-static int ecryptfs_prepare_write(struct file *file, struct page *page,
-				  unsigned from, unsigned to)
+static int ecryptfs_write_begin(struct file *file,
+			struct address_space *mapping,
+			loff_t pos, unsigned len, unsigned flags,
+			struct page **pagep, void **fsdata)
 {
+	pgoff_t index = pos >> PAGE_CACHE_SHIFT;
+	struct page *page;
 	loff_t prev_page_end_size;
 	int rc = 0;
 
+	page = __grab_cache_page(mapping, index);
+	if (!page)
+		return -ENOMEM;
+	*pagep = page;
+
 	if (!PageUptodate(page)) {
 		struct ecryptfs_crypt_stat *crypt_stat =
 			&ecryptfs_inode_to_private(
@@ -289,8 +301,7 @@ static int ecryptfs_prepare_write(struct
 		if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)
 		    || (crypt_stat->flags & ECRYPTFS_NEW_FILE)) {
 			rc = ecryptfs_read_lower_page_segment(
-				page, page->index, 0, PAGE_CACHE_SIZE,
-				page->mapping->host);
+				page, index, 0, PAGE_CACHE_SIZE, mapping->host);
 			if (rc) {
 				printk(KERN_ERR "%s: Error attemping to read "
 				       "lower page segment; rc = [%d]\n",
@@ -316,8 +327,8 @@ static int ecryptfs_prepare_write(struct
 				SetPageUptodate(page);
 			} else {
 				rc = ecryptfs_read_lower_page_segment(
-					page, page->index, 0, PAGE_CACHE_SIZE,
-					page->mapping->host);
+					page, index, 0, PAGE_CACHE_SIZE,
+					mapping->host);
 				if (rc) {
 					printk(KERN_ERR "%s: Error reading "
 					       "page; rc = [%d]\n",
@@ -339,10 +350,10 @@ static int ecryptfs_prepare_write(struct
 			SetPageUptodate(page);
 		}
 	}
-	prev_page_end_size = ((loff_t)page->index << PAGE_CACHE_SHIFT);
+	prev_page_end_size = ((loff_t)index << PAGE_CACHE_SHIFT);
 	/* If creating a page or more of holes, zero them out via truncate.
 	 * Note, this will increase i_size. */
-	if (page->index != 0) {
+	if (index != 0) {
 		if (prev_page_end_size > i_size_read(page->mapping->host)) {
 			rc = ecryptfs_truncate(file->f_path.dentry,
 					       prev_page_end_size);
@@ -357,8 +368,8 @@ static int ecryptfs_prepare_write(struct
 	}
 	/* Writing to a new page, and creating a small hole from start
 	 * of page?  Zero it out. */
-	if ((i_size_read(page->mapping->host) == prev_page_end_size)
-	    && (from != 0))
+	if ((i_size_read(mapping->host) == prev_page_end_size)
+	    && (pos != 0))
 		zero_user(page, 0, PAGE_CACHE_SIZE);
 out:
 	return rc;
@@ -445,21 +456,28 @@ int ecryptfs_write_inode_size_to_metadat
 }
 
 /**
- * ecryptfs_commit_write
+ * ecryptfs_write_end
  * @file: The eCryptfs file object
+ * @mapping: The eCryptfs object
+ * @pos: The file position
+ * @len: The length of the data (unused)
+ * @copied: The amount of data copied
  * @page: The eCryptfs page
- * @from: Ignored (we rotate the page IV on each write)
- * @to: Ignored
+ * @fsdata: The fsdata (unused)
  *
  * This is where we encrypt the data and pass the encrypted data to
  * the lower filesystem.  In OpenPGP-compatible mode, we operate on
  * entire underlying packets.
  */
-static int ecryptfs_commit_write(struct file *file, struct page *page,
-				 unsigned from, unsigned to)
+static int ecryptfs_write_end(struct file *file,
+			struct address_space *mapping,
+			loff_t pos, unsigned len, unsigned copied,
+			struct page *page, void *fsdata)
 {
-	loff_t pos;
-	struct inode *ecryptfs_inode = page->mapping->host;
+	pgoff_t index = pos >> PAGE_CACHE_SHIFT;
+	unsigned from = pos & (PAGE_CACHE_SIZE - 1);
+	unsigned to = from + copied;
+	struct inode *ecryptfs_inode = mapping->host;
 	struct ecryptfs_crypt_stat *crypt_stat =
 		&ecryptfs_inode_to_private(file->f_path.dentry->d_inode)->crypt_stat;
 	int rc;
@@ -471,25 +489,22 @@ static int ecryptfs_commit_write(struct 
 	} else
 		ecryptfs_printk(KERN_DEBUG, "Not a new file\n");
 	ecryptfs_printk(KERN_DEBUG, "Calling fill_zeros_to_end_of_page"
-			"(page w/ index = [0x%.16x], to = [%d])\n", page->index,
-			to);
+			"(page w/ index = [0x%.16x], to = [%d])\n", index, to);
 	/* Fills in zeros if 'to' goes beyond inode size */
 	rc = fill_zeros_to_end_of_page(page, to);
 	if (rc) {
 		ecryptfs_printk(KERN_WARNING, "Error attempting to fill "
-				"zeros in page with index = [0x%.16x]\n",
-				page->index);
+			"zeros in page with index = [0x%.16x]\n", index);
 		goto out;
 	}
 	rc = ecryptfs_encrypt_page(page);
 	if (rc) {
 		ecryptfs_printk(KERN_WARNING, "Error encrypting page (upper "
-				"index [0x%.16x])\n", page->index);
+				"index [0x%.16x])\n", index);
 		goto out;
 	}
-	pos = (((loff_t)page->index) << PAGE_CACHE_SHIFT) + to;
-	if (pos > i_size_read(ecryptfs_inode)) {
-		i_size_write(ecryptfs_inode, pos);
+	if (pos + copied > i_size_read(ecryptfs_inode)) {
+		i_size_write(ecryptfs_inode, pos + copied);
 		ecryptfs_printk(KERN_DEBUG, "Expanded file size to "
 				"[0x%.16x]\n", i_size_read(ecryptfs_inode));
 	}
@@ -497,7 +512,11 @@ static int ecryptfs_commit_write(struct 
 	if (rc)
 		printk(KERN_ERR "Error writing inode size to metadata; "
 		       "rc = [%d]\n", rc);
+
+	rc = copied;
 out:
+	unlock_page(page);
+	page_cache_release(page);
 	return rc;
 }
 
@@ -518,7 +537,7 @@ static sector_t ecryptfs_bmap(struct add
 struct address_space_operations ecryptfs_aops = {
 	.writepage = ecryptfs_writepage,
 	.readpage = ecryptfs_readpage,
-	.prepare_write = ecryptfs_prepare_write,
-	.commit_write = ecryptfs_commit_write,
+	.write_begin = ecryptfs_write_begin,
+	.write_end = ecryptfs_write_end,
 	.bmap = ecryptfs_bmap,
 };

Re: [PATCH] Convert ecryptfs to use new aops

[Nick Piggin]

On Wed, Sep 24, 2008 at 02:57:28PM -0700, Badari Pulavarty wrote:
> Hi Andrew,
> 
> As part of moving all the filesystems to make use of new aops to
> eventually remove prepare/commit_write, here is the patch to convert
> ecryptfs to make use of new aops. 
> 
> Originally written by Nick, I did few minor fixes and clean ups.
> Michael Halcrow tested and confirmed that it works.
> 
> Thanks,
> Badari

Hey, thanks very much for taking the initiative on these, I appreciate
it, and you're getting the most important thing done which is getting
testing and review :)

Re: [PATCH] Convert ecryptfs to use new aops

[Dave Kleikamp]

On Wed, 2008-09-24 at 14:57 -0700, Badari Pulavarty wrote:
> Hi Andrew,
> 
> As part of moving all the filesystems to make use of new aops to
> eventually remove prepare/commit_write, here is the patch to convert
> ecryptfs to make use of new aops. 
> 
> Originally written by Nick, I did few minor fixes and clean ups.
> Michael Halcrow tested and confirmed that it works.
> 
> Thanks,
> Badari
> 
> 
> Convert ecryptfs to use new aops (write_begin/write_end)
> 
> Signed-off-by: Nick Piggin <npiggin@suse.de>
> Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
> Acked-by: Michael Halcrow <mhalcrow@us.ibm.com>


> @@ -497,7 +512,11 @@ static int ecryptfs_commit_write(struct 
>  	if (rc)
>  		printk(KERN_ERR "Error writing inode size to metadata; "
>  		       "rc = [%d]\n", rc);
> +
> +	rc = copied;

I think we need an "else" before "rc = copied;".  If rc is not zero, we
should return the error from ecryptfs_write_inode_size_to_metadata().

>  out:
> +	unlock_page(page);
> +	page_cache_release(page);
>  	return rc;
>  }
> 

Other than that, it looks okay to me.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center