All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Halcrow <mhalcrow@us.ibm.com>
To: Andrew Morton <akpm@osdl.org>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5 (try 2)] eCryptfs: convert kmap() to kmap_atomic()
Date: Fri, 19 Jan 2007 14:35:05 -0600	[thread overview]
Message-ID: <20070119203505.GC3608@us.ibm.com> (raw)
In-Reply-To: <84144f020701190404u5162db92mcea13a709d487602@mail.gmail.com>

On Fri, Jan 19, 2007 at 02:04:47PM +0200, Pekka Enberg wrote:
> On 1/18/07, Michael Halcrow <mhalcrow@us.ibm.com> wrote:
> >+       page_data = (char *)kmap_atomic(page, KM_USER0);
> >+       lower_page_data = (char *)kmap_atomic(lower_page, KM_USER1);
> 
> Drop 'em redundant casts, please.

Replace kmap() with kmap_atomic(). Reduce the amount of time that
mappings are held.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
Signed-off-by: Trevor Highland <tshighla@us.ibm.com>

---

 fs/ecryptfs/crypto.c          |    8 +--
 fs/ecryptfs/ecryptfs_kernel.h |    4 -
 fs/ecryptfs/mmap.c            |  121 ++++++++++++-----------------------------
 3 files changed, 39 insertions(+), 94 deletions(-)

ff2dcd23fe6bd000049987b2bf98ada70755f114
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index fbb62c8..ac4ea8d 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -429,10 +429,10 @@ static int ecryptfs_read_in_page(struct 
 			goto out;
 		}
 	} else {
-		rc = ecryptfs_grab_and_map_lower_page(lower_page, NULL,
-						      lower_inode,
-						      lower_page_idx);
-		if (rc) {
+		*lower_page = grab_cache_page(lower_inode->i_mapping,
+					      lower_page_idx);
+		if (!(*lower_page)) {
+			rc = -EINVAL;
 			ecryptfs_printk(
 				KERN_ERR, "Error attempting to grab and map "
 				"lower page with index [0x%.16x]; rc = [%d]\n",
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 859d31b..0e38d0d 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -527,10 +527,6 @@ int ecryptfs_copy_page_to_lower(struct p
 				struct file *lower_file);
 int ecryptfs_do_readpage(struct file *file, struct page *page,
 			 pgoff_t lower_page_index);
-int ecryptfs_grab_and_map_lower_page(struct page **lower_page,
-				     char **lower_virt,
-				     struct inode *lower_inode,
-				     unsigned long lower_page_index);
 int ecryptfs_writepage_and_release_lower_page(struct page *lower_page,
 					      struct inode *lower_inode,
 					      struct writeback_control *wbc);
diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index 94bcabf..3dec846 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -234,22 +234,11 @@ int ecryptfs_do_readpage(struct file *fi
 		goto out;
 	}
 	wait_on_page_locked(lower_page);
-	page_data = (char *)kmap(page);
-	if (!page_data) {
-		rc = -ENOMEM;
-		ecryptfs_printk(KERN_ERR, "Error mapping page\n");
-		goto out;
-	}
-	lower_page_data = (char *)kmap(lower_page);
-	if (!lower_page_data) {
-		rc = -ENOMEM;
-		ecryptfs_printk(KERN_ERR, "Error mapping page\n");
-		kunmap(page);
-		goto out;
-	}
+	page_data = kmap_atomic(page, KM_USER0);
+	lower_page_data = kmap_atomic(lower_page, KM_USER1);
 	memcpy(page_data, lower_page_data, PAGE_CACHE_SIZE);
-	kunmap(lower_page);
-	kunmap(page);
+	kunmap_atomic(lower_page_data, KM_USER1);
+	kunmap_atomic(page_data, KM_USER0);
 	rc = 0;
 out:
 	if (likely(lower_page))
@@ -325,19 +314,14 @@ static int ecryptfs_readpage(struct file
 			if (page->index < num_pages_in_header_region) {
 				char *page_virt;
 
-				page_virt = (char *)kmap(page);
-				if (!page_virt) {
-					rc = -ENOMEM;
-					printk(KERN_ERR "Error mapping page\n");
-					goto out;
-				}
+				page_virt = kmap_atomic(page, KM_USER0);
 				memset(page_virt, 0, PAGE_CACHE_SIZE);
 				if (page->index == 0) {
 					rc = ecryptfs_read_xattr_region(
 						page_virt, file->f_path.dentry);
 					set_header_info(page_virt, crypt_stat);
 				}
-				kunmap(page);
+				kunmap_atomic(page_virt, KM_USER0);
 				if (rc) {
 					printk(KERN_ERR "Error reading xattr "
 					       "region\n");
@@ -387,26 +371,19 @@ static int fill_zeros_to_end_of_page(str
 {
 	struct inode *inode = page->mapping->host;
 	int end_byte_in_page;
-	int rc = 0;
 	char *page_virt;
 
-	if ((i_size_read(inode) / PAGE_CACHE_SIZE) == page->index) {
-		end_byte_in_page = i_size_read(inode) % PAGE_CACHE_SIZE;
-		if (to > end_byte_in_page)
-			end_byte_in_page = to;
-		page_virt = kmap(page);
-		if (!page_virt) {
-			rc = -ENOMEM;
-			ecryptfs_printk(KERN_WARNING,
-					"Could not map page\n");
-			goto out;
-		}
-		memset((page_virt + end_byte_in_page), 0,
-		       (PAGE_CACHE_SIZE - end_byte_in_page));
-		kunmap(page);
-	}
+	if ((i_size_read(inode) / PAGE_CACHE_SIZE) != page->index)
+		goto out;
+	end_byte_in_page = i_size_read(inode) % PAGE_CACHE_SIZE;
+	if (to > end_byte_in_page)
+		end_byte_in_page = to;
+	page_virt = kmap_atomic(page, KM_USER0);
+	memset((page_virt + end_byte_in_page), 0,
+	       (PAGE_CACHE_SIZE - end_byte_in_page));
+	kunmap_atomic(page_virt, KM_USER0);
 out:
-	return rc;
+	return 0;
 }
 
 static int ecryptfs_prepare_write(struct file *file, struct page *page,
@@ -414,7 +391,6 @@ static int ecryptfs_prepare_write(struct
 {
 	int rc = 0;
 
-	kmap(page);
 	if (from == 0 && to == PAGE_CACHE_SIZE)
 		goto out;	/* If we are writing a full page, it will be
 				   up to date. */
@@ -424,30 +400,6 @@ out:
 	return rc;
 }
 
-int ecryptfs_grab_and_map_lower_page(struct page **lower_page,
-				     char **lower_virt,
-				     struct inode *lower_inode,
-				     unsigned long lower_page_index)
-{
-	int rc = 0;
-
-	(*lower_page) = grab_cache_page(lower_inode->i_mapping,
-					lower_page_index);
-	if (!(*lower_page)) {
-		ecryptfs_printk(KERN_ERR, "grab_cache_page for "
-				"lower_page_index = [0x%.16x] failed\n",
-				lower_page_index);
-		rc = -EINVAL;
-		goto out;
-	}
-	if (lower_virt)
-		(*lower_virt) = kmap((*lower_page));
-	else
-		kmap((*lower_page));
-out:
-	return rc;
-}
-
 int ecryptfs_writepage_and_release_lower_page(struct page *lower_page,
 					      struct inode *lower_inode,
 					      struct writeback_control *wbc)
@@ -466,11 +418,8 @@ out:
 	return rc;
 }
 
-static void ecryptfs_unmap_and_release_lower_page(struct page *lower_page)
+static void ecryptfs_release_lower_page(struct page *lower_page)
 {
-	kunmap(lower_page);
-	ecryptfs_printk(KERN_DEBUG, "Unlocking lower page with index = "
-			"[0x%.16x]\n", lower_page->index);
 	unlock_page(lower_page);
 	page_cache_release(lower_page);
 }
@@ -492,11 +441,11 @@ static int ecryptfs_write_inode_size_to_
 	const struct address_space_operations *lower_a_ops;
 	u64 file_size;
 
-	rc = ecryptfs_grab_and_map_lower_page(&header_page, &header_virt,
-					      lower_inode, 0);
-	if (rc) {
-		ecryptfs_printk(KERN_ERR, "grab_cache_page for header page "
-				"failed\n");
+	header_page = grab_cache_page(lower_inode->i_mapping, 0);
+	if (!header_page) {
+		ecryptfs_printk(KERN_ERR, "grab_cache_page for "
+				"lower_page_index 0 failed\n");
+		rc = -EINVAL;
 		goto out;
 	}
 	lower_a_ops = lower_inode->i_mapping->a_ops;
@@ -504,12 +453,14 @@ static int ecryptfs_write_inode_size_to_
 	file_size = (u64)i_size_read(inode);
 	ecryptfs_printk(KERN_DEBUG, "Writing size: [0x%.16x]\n", file_size);
 	file_size = cpu_to_be64(file_size);
+	header_virt = kmap_atomic(header_page, KM_USER0);
 	memcpy(header_virt, &file_size, sizeof(u64));
+	kunmap_atomic(header_virt, KM_USER0);
 	rc = lower_a_ops->commit_write(lower_file, header_page, 0, 8);
 	if (rc < 0)
 		ecryptfs_printk(KERN_ERR, "Error commiting header page "
 				"write\n");
-	ecryptfs_unmap_and_release_lower_page(header_page);
+	ecryptfs_release_lower_page(header_page);
 	lower_inode->i_mtime = lower_inode->i_ctime = CURRENT_TIME;
 	mark_inode_dirty_sync(inode);
 out:
@@ -597,10 +548,10 @@ int ecryptfs_get_lower_page(struct page 
 {
 	int rc = 0;
 
-	rc = ecryptfs_grab_and_map_lower_page(lower_page, NULL, lower_inode,
-					      lower_page_index);
-	if (rc) {
-		ecryptfs_printk(KERN_ERR, "Error attempting to grab and map "
+	*lower_page = grab_cache_page(lower_inode->i_mapping, lower_page_index);
+	if (!(*lower_page)) {
+		rc = -EINVAL;
+		ecryptfs_printk(KERN_ERR, "Error attempting to grab "
 				"lower page with index [0x%.16x]\n",
 				lower_page_index);
 		goto out;
@@ -616,7 +567,7 @@ int ecryptfs_get_lower_page(struct page 
 	}
 out:
 	if (rc && (*lower_page)) {
-		ecryptfs_unmap_and_release_lower_page(*lower_page);
+		ecryptfs_release_lower_page(*lower_page);
 		(*lower_page) = NULL;
 	}
 	return rc;
@@ -641,7 +592,7 @@ ecryptfs_commit_lower_page(struct page *
 				"Error committing write; rc = [%d]\n", rc);
 	} else
 		rc = 0;
-	ecryptfs_unmap_and_release_lower_page(lower_page);
+	ecryptfs_release_lower_page(lower_page);
 	return rc;
 }
 
@@ -747,7 +698,6 @@ static int ecryptfs_commit_write(struct 
 	lower_inode->i_mtime = lower_inode->i_ctime = CURRENT_TIME;
 	mark_inode_dirty_sync(inode);
 out:
-	kunmap(page); /* mapped in prior call (prepare_write) */
 	if (rc < 0)
 		ClearPageUptodate(page);
 	else
@@ -772,6 +722,7 @@ int write_zeros(struct file *file, pgoff
 {
 	int rc = 0;
 	struct page *tmp_page;
+	char *tmp_page_virt;
 
 	tmp_page = ecryptfs_get1page(file, index);
 	if (IS_ERR(tmp_page)) {
@@ -780,28 +731,26 @@ int write_zeros(struct file *file, pgoff
 		rc = PTR_ERR(tmp_page);
 		goto out;
 	}
-	kmap(tmp_page);
 	rc = ecryptfs_prepare_write(file, tmp_page, start, start + num_zeros);
 	if (rc) {
 		ecryptfs_printk(KERN_ERR, "Error preparing to write zero's "
 				"to remainder of page at index [0x%.16x]\n",
 				index);
-		kunmap(tmp_page);
 		page_cache_release(tmp_page);
 		goto out;
 	}
-	memset(((char *)page_address(tmp_page) + start), 0, num_zeros);
+	tmp_page_virt = kmap_atomic(tmp_page, KM_USER0);
+	memset(((char *)tmp_page_virt + start), 0, num_zeros);
+	kunmap_atomic(tmp_page_virt, KM_USER0);
 	rc = ecryptfs_commit_write(file, tmp_page, start, start + num_zeros);
 	if (rc < 0) {
 		ecryptfs_printk(KERN_ERR, "Error attempting to write zero's "
 				"to remainder of page at index [0x%.16x]\n",
 				index);
-		kunmap(tmp_page);
 		page_cache_release(tmp_page);
 		goto out;
 	}
 	rc = 0;
-	kunmap(tmp_page);
 	page_cache_release(tmp_page);
 out:
 	return rc;
-- 
1.3.3


  reply	other threads:[~2007-01-19 20:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-18 21:26 [PATCH 1/5] eCryptfs: convert f_op->write() to vfs_write() Michael Halcrow
2007-01-18 21:27 ` [PATCH 2/5] eCryptfs: convert kmap() to kmap_atomic() Michael Halcrow
2007-01-19 12:04   ` Pekka Enberg
2007-01-19 20:35     ` Michael Halcrow [this message]
2007-01-18 21:28 ` [PATCH 3/5] eCryptfs: open-code flag checking and manipulation Michael Halcrow
2007-01-18 21:28 ` [PATCH 4/5] eCryptfs: add flush_dcache_page() calls Michael Halcrow
2007-01-18 21:29 ` [PATCH 5/5] eCryptfs: convert lookup_one_len() to lookup_one_len_nd() Michael Halcrow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070119203505.GC3608@us.ibm.com \
    --to=mhalcrow@us.ibm.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penberg@cs.helsinki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.