Skip to contentSkip to conversation

fs: cleanup single page synchronous read interface

PATCH In review Nate Diller 8 participants 33 messages Apr 12, 2007 – Jul 16, 2007

Subsystems (13)
Memory management VFS Memory technology devices (MTD) Page cache +9 more subsystems
lore
Share & tools
lore

Nate Diller

author
#0 lore raw

cover fs: cleanup single page synchronous read interface

Nick Piggin recently changed the read_cache_page interface to be synchronous, which is pretty much what the file systems want anyway. Turns out that they have more in common than that, though, and some of them want to be able to get an uptodate *locked* page. Many of them want a kmapped page, which is uptodate and unlocked, and they all have their own individual helper functions to achieve this.

Since the helper functions are so similar, this patch just combines them into a small number of simple library functions, which call read_cache_page (renamed to __read_cache_page because it now returns a locked page). The immediate result is a vast reduction in the number of fs-specific helper functions. The secondary goal is to reduce the number of places the page lock is taken, and eliminate a lot of PageUptodate and PageError checks.

The file systems that still use PageChecked now have checker functions that return an error if the page is corrupted or has some other error. This simplifies the logic since the checker function is not part of any helper function anymore.

Compile tested on x86_64.

Signed-off-by Nate Diller <[email protected]>

---

 drivers/mtd/devices/block2mtd.c          |   28 +------
 fs/afs/dir.c                             |   56 +++-----------
 fs/afs/mntpt.c                           |   10 --
 fs/cramfs/inode.c                        |    3 
 fs/ext2/dir.c                            |   82 ++++++++-------------
 fs/freevxfs/vxfs_extern.h                |    1 
 fs/freevxfs/vxfs_inode.c                 |    2 
 fs/freevxfs/vxfs_lookup.c                |    4 -
 fs/freevxfs/vxfs_subr.c                  |   33 --------
 fs/hfs/bnode.c                           |    4 -
 fs/hfsplus/bnode.c                       |    4 -
 fs/jffs2/fs.c                            |   27 -------
 fs/jffs2/gc.c                            |   15 ++-
 fs/jfs/jfs_metapage.c                    |    5 -
 fs/minix/dir.c                           |   59 ++++-----------
 fs/ntfs/aops.h                           |   67 -----------------
 fs/ntfs/bitmap.c                         |    8 +-
 fs/ntfs/dir.c                            |   65 ++++++-----------
 fs/ntfs/index.c                          |   12 +--
 fs/ntfs/lcnalloc.c                       |    6 -
 fs/ntfs/logfile.c                        |   12 +--
 fs/ntfs/mft.c                            |   53 +++++--------
 fs/ntfs/super.c                          |   38 ++++-----
 fs/ntfs/usnjrnl.c                        |    4 -
 fs/partitions/check.c                    |   14 +--
 fs/reiser4/plugin/file/tail_conversion.c |    8 --
 fs/reiser4/plugin/item/extent_file_ops.c |    9 --
 fs/reiserfs/xattr.c                      |   48 ++----------
 fs/sysv/dir.c                            |   19 +---
 fs/ufs/balloc.c                          |    8 +-
 fs/ufs/dir.c                             |   90 +++++++++--------------
 fs/ufs/truncate.c                        |    8 +-
 fs/ufs/util.c                            |   52 -------------
 fs/ufs/util.h                            |   10 --
 include/linux/pagemap.h                  |   53 ++++++++++++-
 mm/filemap.c                             |  118 +++++++------------------------
 36 files changed, 315 insertions(+), 720 deletions(-)
author
#1 lore raw

PATCH 1/17 cramfs: use read_mapping_page

read_mapping_page_async() is going away, so convert its only user to read_mapping_page(). This change has not been benchmarked, however, in order to get real parallelism this wants something completely different, like __do_page_cache_readahead(), which is not currently exported.

Signed-off-by Nate Diller <[email protected]>

---

fs/cramfs/inode.c +1 -2
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/cramfs/inode.c linux-2.6.21-rc6-mm1-test/fs/cramfs/inode.c
@@ -180,8 +180,7 @@ static void *cramfs_read(struct super_bl
 		struct page *page = NULL;
 
 		if (blocknr + i < devsize) {
-			page = read_mapping_page_async(mapping, blocknr + i,
-									NULL);
+			page = read_mapping_page(mapping, blocknr + i, NULL);
 			/* synchronous error? */
 			if (IS_ERR(page))
 				page = NULL;
#2 lore raw

Re: [PATCH 1/17] cramfs: use read_mapping_page

Quoted 4 lines · Nate Diller

read_mapping_page_async() is going away, so convert its only user to read_mapping_page(). This change has not been benchmarked, however, in order to get real parallelism this wants something completely different, like __do_page_cache_readahead(), which is not currently exported.

Why is read_mapping_page_async going away? This probably needs a lot more testing, and I'd be much happier if you split it out of the series and sent it separately at the end.

#3 lore raw

Re: [PATCH 1/17] cramfs: use read_mapping_page

Hi,

Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Nate Diller

read_mapping_page_async() is going away, so convert its only user to read_mapping_page(). This change has not been benchmarked, however, in order to get real parallelism this wants something completely different, like __do_page_cache_readahead(), which is not currently exported.

Why is read_mapping_page_async going away? This probably needs a lot more testing, and I'd be much happier if you split it out of the series and sent it separately at the end.

That function wasn't fully async anyway, as it would often sleep in lock_page(). AFAICT only in the special case of a partial written page would this function return a not yet uptodate page.

bye, Roman

author
#4 lore raw

Re: [PATCH 1/17] cramfs: use read_mapping_page

Quoted 10 lines · Roman Zippel

Hi,

Quoted 6 lines · Christoph Hellwig
Quoted 4 lines · Nate Diller

read_mapping_page_async() is going away, so convert its only user to read_mapping_page(). This change has not been benchmarked, however, in order to get real parallelism this wants something completely different, like __do_page_cache_readahead(), which is not currently exported.

Quoted 7 lines
Quoted 3 lines

Why is read_mapping_page_async going away? This probably needs a lot more testing, and I'd be much happier if you split it out of the series and sent it separately at the end.

That function wasn't fully async anyway, as it would often sleep in lock_page(). AFAICT only in the special case of a partial written page would this function return a not yet uptodate page.

yes, exactly, the structure of read_cache_page() and friends is totally not appropriate for doing async I/O to more than one page at a time, and the whole point of the special treatment in cramfs was to read 4 pages at once rather than synchronously reading each of the 4 seperately. read_cache_page_async() is totally wrong for that use, its purpose would be to get a reference to a single page that is likely to be in cache already without having to take the page_lock. Turns out nobody needs to do that, so there's no point in keeping it around.

If the performance gain of reading all 4 pages at once would be worth the effort, this code should be using __do_page_cache_readahead(). That function allocates all the pages first, then reads them in asynchronously as a group. It is currently not exported.

NATE

author
#5 lore raw

PATCH 2/17 fs: introduce new read_cache_page interface

In reply to Nate Diller

Export a single version of read_cache_page, which returns with a locked, Uptodate page or a synchronous error, and use inline helper functions to replicate the old behavior. Also, introduce new helper functions for the most common file system uses, which include kmapping the page, as well as needing to keep the page locked. These changes collectively eliminate a substantial amount of private fs logic in favor of generic code.

It also simplifies filemap.c significantly, by assuming that callers want synchronous behavior, which is true for all callers anyway except one.

Signed-off-by Nate Diller <[email protected]>

---

include/linux/pagemap.h +46 -7
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/include/linux/pagemap.h linux-2.6.21-rc6-mm1-test/include/linux/pagemap.h
@@ -108,21 +108,30 @@ static inline struct page *grab_cache_pa
 
 extern struct page * grab_cache_page_nowait(struct address_space *mapping,
 				unsigned long index);
-extern struct page * read_cache_page_async(struct address_space *mapping,
-				unsigned long index, filler_t *filler,
-				void *data);
-extern struct page * read_cache_page(struct address_space *mapping,
+extern struct page *__read_cache_page(struct address_space *mapping,
 				unsigned long index, filler_t *filler,
 				void *data);
 extern int read_cache_pages(struct address_space *mapping,
 		struct list_head *pages, filler_t *filler, void *data);
 
-static inline struct page *read_mapping_page_async(
-						struct address_space *mapping,
+void fastcall unlock_page(struct page *page);
+static inline struct page *read_cache_page(struct address_space *mapping,
+				unsigned long index, filler_t *filler,
+				void *data)
+{
+	struct page *page;
+
+	page = __read_cache_page(mapping, index, filler, data);
+	if (!IS_ERR(page))
+		unlock_page(page);
+	return page;
+}
+
+static inline struct page *__read_mapping_page(struct address_space *mapping,
 					     unsigned long index, void *data)
 {
 	filler_t *filler = (filler_t *)mapping->a_ops->readpage;
-	return read_cache_page_async(mapping, index, filler, data);
+	return __read_cache_page(mapping, index, filler, data);
 }
 
 static inline struct page *read_mapping_page(struct address_space *mapping,

@@ -132,6 +141,36 @@ static inline struct page *read_mapping_
 	return read_cache_page(mapping, index, filler, data);
 }
 
+static inline struct page *__read_kmap_page(struct address_space *mapping,
+					  unsigned long index)
+{
+	struct page *page = __read_mapping_page(mapping, index, NULL);
+	if (!IS_ERR(page))
+		kmap(page);
+	return page;
+}
+
+static inline struct page *read_kmap_page(struct address_space *mapping,
+					  unsigned long index)
+{
+	struct page *page = read_mapping_page(mapping, index, NULL);
+	if (!IS_ERR(page))
+		kmap(page);
+	return page;
+}
+
+static inline void put_kmapped_page(struct page *page)
+{
+	kunmap(page);
+	page_cache_release(page);
+}
+
+static inline void put_locked_page(struct page *page)
+{
+	unlock_page(page);
+	put_kmapped_page(page);
+}
+
 int add_to_page_cache(struct page *page, struct address_space *mapping,
 				unsigned long index, gfp_t gfp_mask);
 int add_to_page_cache_lru(struct page *page, struct address_space *mapping,
mm/filemap.c +28 -90

143 lines not on this page · raw message

author
#6 lore raw

PATCH 3/17 afs: convert afs_dir_get_page to read_kmap_page

In reply to Nate Diller

Replace afs_dir_get_page() and afs_dir_put_page() using the new read_kmap_page() and put_kmapped_page() calls, and eliminate unnecessary PageError checks. Also, change the afs_dir_check_page() call to return the page's error status, and update the call site accordingly.

Signed-off-by Nate Diller <[email protected]>

---

fs/afs/dir.c +14 -42
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/afs/dir.c linux-2.6.21-rc5-mm4-test/fs/afs/dir.c
@@ -115,12 +115,15 @@ struct afs_dir_lookup_cookie {
 /*
  * check that a directory page is valid
  */
-static inline void afs_dir_check_page(struct inode *dir, struct page *page)
+static inline int afs_dir_check_page(struct inode *dir, struct page *page)
 {
 	struct afs_dir_page *dbuf;
 	loff_t latter;
 	int tmp, qty;
 
+	if (likely(PageChecked(page)))
+		return PageError(page);
+
 #if 0
 	/* check the page count */
 	qty = desc.size / sizeof(dbuf->blocks[0]);

@@ -154,52 +157,16 @@ static inline void afs_dir_check_page(st
 	}
 
 	SetPageChecked(page);
-	return;
+	return 0;
 
  error:
 	SetPageChecked(page);
 	SetPageError(page);
-
+	return 1;
 } /* end afs_dir_check_page() */
 
 /*****************************************************************************/
 /*
- * discard a page cached in the pagecache
- */
-static inline void afs_dir_put_page(struct page *page)
-{
-	kunmap(page);
-	page_cache_release(page);
-
-} /* end afs_dir_put_page() */
-
-/*****************************************************************************/
-/*
- * get a page into the pagecache
- */
-static struct page *afs_dir_get_page(struct inode *dir, unsigned long index)
-{
-	struct page *page;
-
-	_enter("{%lu},%lu", dir->i_ino, index);
-
-	page = read_mapping_page(dir->i_mapping, index, NULL);
-	if (!IS_ERR(page)) {
-		kmap(page);
-		if (!PageChecked(page))
-			afs_dir_check_page(dir, page);
-		if (PageError(page))
-			goto fail;
-	}
-	return page;
-
- fail:
-	afs_dir_put_page(page);
-	return ERR_PTR(-EIO);
-} /* end afs_dir_get_page() */
-
-/*****************************************************************************/
-/*
  * open an AFS directory file
  */
 static int afs_dir_open(struct inode *inode, struct file *file)

@@ -344,11 +311,16 @@ static int afs_dir_iterate(struct inode 
 		blkoff = *fpos & ~(sizeof(union afs_dir_block) - 1);
 
 		/* fetch the appropriate page from the directory */
-		page = afs_dir_get_page(dir, blkoff / PAGE_SIZE);
+		page = read_kmap_page(dir->i_mapping, blkoff / PAGE_SIZE);
 		if (IS_ERR(page)) {
 			ret = PTR_ERR(page);
 			break;
 		}
+		if (afs_check_page(dir, page)) {
+			err = -EIO;
+			put_kmapped_page(page);
+			break;
+		}
 
 		limit = blkoff & ~(PAGE_SIZE - 1);
 

@@ -361,7 +333,7 @@ static int afs_dir_iterate(struct inode 
 			ret = afs_dir_iterate_block(fpos, dblock, blkoff,
 						    cookie, filldir);
 			if (ret != 1) {
-				afs_dir_put_page(page);
+				put_kmapped_page(page);
 				goto out;
 			}
 

@@ -369,7 +341,7 @@ static int afs_dir_iterate(struct inode 
 
 		} while (*fpos < dir->i_size && blkoff < limit);
 
-		afs_dir_put_page(page);
+		put_kmapped_page(page);
 		ret = 0;
 	}
fs/afs/mntpt.c +0 -10
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/afs/mntpt.c linux-2.6.21-rc6-mm1-test/fs/afs/mntpt.c
@@ -74,11 +74,6 @@ int afs_mntpt_check_symlink(struct afs_v
 		ret = PTR_ERR(page);
 		goto out;
 	}
-
-	ret = -EIO;
-	if (PageError(page))
-		goto out_free;
-
 	buf = kmap(page);
 
 	/* examine the symlink's contents */

@@ -98,7 +93,6 @@ int afs_mntpt_check_symlink(struct afs_v
 	ret = 0;
 
 	kunmap(page);
- out_free:
 	page_cache_release(page);
  out:
 	_leave(" = %d", ret);

@@ -180,10 +174,6 @@ static struct vfsmount *afs_mntpt_do_aut
 		goto error;
 	}
 
-	ret = -EIO;
-	if (PageError(page))
-		goto error;
-
 	buf = kmap(page);
 	memcpy(devname, buf, size);
 	kunmap(page);
maintainer
#7 lore raw

Re: [PATCH 3/17] afs: convert afs_dir_get_page to read_kmap_page

Quoted 1 line · Nate Diller

-static struct page *afs_dir_get_page(struct inode *dir, unsigned long index)

NAK. This conflicts with my AFS security patches, and eliminates any way of passing the key through to readpage().

David

author
#8 lore raw

Re: [PATCH 3/17] afs: convert afs_dir_get_page to read_kmap_page

Quoted 6 lines · David Howells
Quoted 1 line · Nate Diller

-static struct page *afs_dir_get_page(struct inode *dir, unsigned long index)

NAK. This conflicts with my AFS security patches, and eliminates any way of passing the key through to readpage().

Hmmm you're right. Is your security work going into the next -mm? If so, I'll just re-base this cleanup patch on that ... at the very least I want to get rid of afs_dir_put_page(). Also, did you consider passing the key pointer directly and modifying the readpage actor to simply cast the pointer back, like read_mapping_page(mapping, page, (struct file *)key)? It seems like a waste to allocate a whole file struct on the stack just for the ->private field.

Andrew in the mean time just disregard this patch.

NATE

maintainer
#9 lore raw

Re: [PATCH 3/17] afs: convert afs_dir_get_page to read_kmap_page

Quoted 1 line · Nate Diller

Hmmm you're right. Is your security work going into the next -mm?

I don't know. Andrew hasn't said anything. Andrew? Are you waiting for it to go through DaveM's networking tree?

Quoted 2 lines

If so, I'll just re-base this cleanup patch on that ... at the very least I want to get rid of afs_dir_put_page().

That's reasonable.

Quoted 4 lines

Also, did you consider passing the key pointer directly and modifying the readpage actor to simply cast the pointer back, like read_mapping_page(mapping, page, (struct file *)key)? It seems like a waste to allocate a whole file struct on the stack just for the ->private field.

There's one small problem with that... And that's filemap_nopage() (it passes vma->vm_file to readpage() unconditionally). Unless, of course, your patches fix that too...

David

maintainer
#10 lore raw

Re: [PATCH 3/17] afs: convert afs_dir_get_page to read_kmap_page

Quoted 4 lines · On Thu, 12 Apr 2007 19:57:23 +0100 David Howells <[email protected]> wrote:
Quoted 1 line

Hmmm you're right. Is your security work going into the next -mm?

I don't know. Andrew hasn't said anything. Andrew? Are you waiting for it to go through DaveM's networking tree?

AF_RXRPC is a davem thing and "AFS: Add security support and fix bugs" has dependencise upon it. So we're waiting for you and the networking guys to sort all that out.

Or am I wrong??

maintainer
#11 lore raw

Re: [PATCH 3/17] afs: convert afs_dir_get_page to read_kmap_page

Quoted 8 lines · Andrew Morton
Quoted 4 lines
Quoted 1 line

Hmmm you're right. Is your security work going into the next -mm?

I don't know. Andrew hasn't said anything. Andrew? Are you waiting for it to go through DaveM's networking tree?

AF_RXRPC is a davem thing and "AFS: Add security support and fix bugs" has dependencise upon it. So we're waiting for you and the networking guys to sort all that out.

I thought this was probably the case, but no-one's actually said what the assimilation procedure is.

Quoted 1 line

Or am I wrong??

Doubtful.

David

#12 lore raw

Drop patch update-isdn-tree-to-use-pci_get_device.patch from -mm tree

In reply to Andrew Morton

Hi Andrew,
I would request you to drop the patch
update-isdn-tree-to-use-pci_get_device.patch
from the -mm tree since a new bug is been identified in that by jeff.

thanks.
surya.

author
#13 lore raw

Re: [PATCH 3/17] afs: convert afs_dir_get_page to read_kmap_page

In reply to David Howells
Quoted 12 lines · David Howells
Quoted 1 line · Nate Diller

Hmmm you're right. Is your security work going into the next -mm?

I don't know. Andrew hasn't said anything. Andrew? Are you waiting for it to go through DaveM's networking tree?

Quoted 2 lines

If so, I'll just re-base this cleanup patch on that ... at the very least I want to get rid of afs_dir_put_page().

That's reasonable.

Quoted 8 lines
Quoted 4 lines

Also, did you consider passing the key pointer directly and modifying the readpage actor to simply cast the pointer back, like read_mapping_page(mapping, page, (struct file *)key)? It seems like a waste to allocate a whole file struct on the stack just for the ->private field.

There's one small problem with that... And that's filemap_nopage() (it passes vma->vm_file to readpage() unconditionally). Unless, of course, your patches fix that too...

But you can't mmap() a directory anyway so ... oh. Interesting. afs_file_readpage() does directories too. The only thing I can think of then is

struct address_space_operations afs_file_aops = {
.readpage = afs_file_readpage,
}
struct address_space_operations afs_dir_aops = {
.readpage = afs_key_readpage,
}

int afs_file_readpage(file, page){
return afs_key_readpage(file->private, page)
}

but that's a lot of code to avoid a single stack allocation. The whole fake file pointer thing still strikes me as a little ugly, and you're definitely not the first one who needed this sort of hackery. ugh

NATE

maintainer
#14 lore raw

Re: [PATCH 3/17] afs: convert afs_dir_get_page to read_kmap_page

Quoted 4 lines · Nate Diller

but that's a lot of code to avoid a single stack allocation. The whole fake file pointer thing still strikes me as a little ugly, and you're definitely not the first one who needed this sort of hackery. ugh

A better way might be to stick a void * in struct file and pass that through to readpage() and readpages() instead of the struct file *. That way, anyone who wants the traditional arrangement can just point that extra void * at the struct file.

Of course, I'm in favour of making it a struct key * like this:

	struct address_space_operations {
	...
		int (*readpage)(struct key *, struct page *);
	...
		int (*readpages)(struct key *, struct address_space *,
				struct list_head *, unsigned);
	...
	};

	struct file {
	...
		struct key *f_key;
	};

	struct page *filemap_nopage(struct vm_area_struct *area, ...)
	{
	...
		struct file *file = area->vm_file;
	...
		error = mapping->a_ops->readpage(file->f_key, page);
	...
	}

But I'm not sure the NFS crew, for instance, would be happy with that. Maybe passing file->private_data through would do. That's basically what NFS and FUSE, for instance, want, and it would also do for AFS.

David

author
#15 lore raw

PATCH 4/17 ext2: convert ext2_get_page to read_kmap_page

In reply to Nate Diller

Replace ext2_get_page() and ext2_put_page() using the new read_kmap_page() and put_kmapped_page() calls. Also, change the ext2_check_page() call to return the page's error status, and update the call sites accordingly.

Signed-off-by Nate Diller <[email protected]>

---

fs/ext2/dir.c +34 -48
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ext2/dir.c linux-2.6.21-rc5-mm4-test/fs/ext2/dir.c
@@ -35,12 +35,6 @@ static inline unsigned ext2_chunk_size(s
 	return inode->i_sb->s_blocksize;
 }
 
-static inline void ext2_put_page(struct page *page)
-{
-	kunmap(page);
-	page_cache_release(page);
-}
-
 static inline unsigned long dir_pages(struct inode *inode)
 {
 	return (inode->i_size+PAGE_CACHE_SIZE-1)>>PAGE_CACHE_SHIFT;

@@ -74,7 +68,7 @@ static int ext2_commit_chunk(struct page
 	return err;
 }
 
-static void ext2_check_page(struct page *page)
+static int ext2_check_page(struct page *page)
 {
 	struct inode *dir = page->mapping->host;
 	struct super_block *sb = dir->i_sb;

@@ -86,6 +80,14 @@ static void ext2_check_page(struct page 
 	ext2_dirent *p;
 	char *error;
 
+	if (likely(PageChecked(page))) {
+		if (likely(!PageError(page)))
+			return 0;
+
+		put_kmapped_page(page);
+		return -EIO;
+	}
+
 	if ((dir->i_size >> PAGE_CACHE_SHIFT) == page->index) {
 		limit = dir->i_size & ~PAGE_CACHE_MASK;
 		if (limit & (chunk_size - 1))

@@ -112,7 +114,7 @@ static void ext2_check_page(struct page 
 		goto Eend;
 out:
 	SetPageChecked(page);
-	return;
+	return 0;
 
 	/* Too bad, we had an error */
 

@@ -153,24 +155,8 @@ Eend:
 fail:
 	SetPageChecked(page);
 	SetPageError(page);
-}
-
-static struct page * ext2_get_page(struct inode *dir, unsigned long n)
-{
-	struct address_space *mapping = dir->i_mapping;
-	struct page *page = read_mapping_page(mapping, n, NULL);
-	if (!IS_ERR(page)) {
-		kmap(page);
-		if (!PageChecked(page))
-			ext2_check_page(page);
-		if (PageError(page))
-			goto fail;
-	}
-	return page;
-
-fail:
-	ext2_put_page(page);
-	return ERR_PTR(-EIO);
+	put_kmapped_page(page);
+	return -EIO;
 }
 
 /*

@@ -262,9 +248,9 @@ ext2_readdir (struct file * filp, void *
 	for ( ; n < npages; n++, offset = 0) {
 		char *kaddr, *limit;
 		ext2_dirent *de;
-		struct page *page = ext2_get_page(inode, n);
+		struct page *page = read_kmap_page(inode->i_mapping, n);
 
-		if (IS_ERR(page)) {
+		if (IS_ERR(page) || ext2_check_page(page)) {
 			ext2_error(sb, __FUNCTION__,
 				   "bad page in #%lu",
 				   inode->i_ino);

@@ -286,7 +272,7 @@ ext2_readdir (struct file * filp, void *
 			if (de->rec_len == 0) {
 				ext2_error(sb, __FUNCTION__,
 					"zero-length directory entry");
-				ext2_put_page(page);
+				put_kmapped_page(page);
 				return -EIO;
 			}
 			if (de->inode) {

@@ -301,13 +287,13 @@ ext2_readdir (struct file * filp, void *
 						(n<<PAGE_CACHE_SHIFT) | offset,
 						le32_to_cpu(de->inode), d_type);
 				if (over) {
-					ext2_put_page(page);
+					put_kmapped_page(page);
 					return 0;
 				}
 			}
 			filp->f_pos += le16_to_cpu(de->rec_len);
 		}
-		ext2_put_page(page);
+		put_kmapped_page(page);
 	}
 	return 0;
 }

@@ -344,8 +330,8 @@ struct ext2_dir_entry_2 * ext2_find_entr
 	n = start;
 	do {
 		char *kaddr;
-		page = ext2_get_page(dir, n);
-		if (!IS_ERR(page)) {
+		page = read_kmap_page(dir->i_mapping, n);
+		if (!IS_ERR(page) && !ext2_check_page(page)) {
 			kaddr = page_address(page);
 			de = (ext2_dirent *) kaddr;
 			kaddr += ext2_last_byte(dir, n) - reclen;

@@ -353,14 +339,14 @@ struct ext2_dir_entry_2 * ext2_find_entr
 				if (de->rec_len == 0) {
 					ext2_error(dir->i_sb, __FUNCTION__,
 						"zero-length directory entry");
-					ext2_put_page(page);
+					put_kmapped_page(page);
 					goto out;
 				}
 				if (ext2_match (namelen, name, de))
 					goto found;
 				de = ext2_next_entry(de);
 			}
-			ext2_put_page(page);
+			put_kmapped_page(page);
 		}
 		if (++n >= npages)
 			n = 0;

@@ -384,10 +370,10 @@ found:
 
 struct ext2_dir_entry_2 * ext2_dotdot (struct inode *dir, struct page **p)
 {
-	struct page *page = ext2_get_page(dir, 0);
+	struct page *page = read_kmap_page(dir->i_mapping, 0);
 	ext2_dirent *de = NULL;
 
-	if (!IS_ERR(page)) {
+	if (!IS_ERR(page) && !ext2_check_page(page)) {
 		de = ext2_next_entry((ext2_dirent *) page_address(page));
 		*p = page;
 	}

@@ -403,7 +389,7 @@ ino_t ext2_inode_by_name(struct inode * 
 	de = ext2_find_entry (dir, dentry, &page);
 	if (de) {
 		res = le32_to_cpu(de->inode);
-		ext2_put_page(page);
+		put_kmapped_page(page);
 	}
 	return res;
 }

@@ -422,7 +408,7 @@ void ext2_set_link(struct inode *dir, st
 	de->inode = cpu_to_le32(inode->i_ino);
 	ext2_set_de_type (de, inode);
 	err = ext2_commit_chunk(page, from, to);
-	ext2_put_page(page);
+	put_kmapped_page(page);
 	dir->i_mtime = dir->i_ctime = CURRENT_TIME_SEC;
 	EXT2_I(dir)->i_flags &= ~EXT2_BTREE_FL;
 	mark_inode_dirty(dir);

@@ -455,9 +441,9 @@ int ext2_add_link (struct dentry *dentry
 	for (n = 0; n <= npages; n++) {
 		char *dir_end;
 
-		page = ext2_get_page(dir, n);
+		page = read_kmap_page(dir->i_mapping, n);
 		err = PTR_ERR(page);
-		if (IS_ERR(page))
+		if (IS_ERR(page) || (err = ext2_check_page(page)))
 			goto out;
 		lock_page(page);
 		kaddr = page_address(page);

@@ -491,7 +477,7 @@ int ext2_add_link (struct dentry *dentry
 			de = (ext2_dirent *) ((char *) de + rec_len);
 		}
 		unlock_page(page);
-		ext2_put_page(page);
+		put_kmapped_page(page);
 	}
 	BUG();
 	return -EINVAL;

@@ -518,7 +504,7 @@ got_it:
 	mark_inode_dirty(dir);
 	/* OFFSET_CACHE */
 out_put:
-	ext2_put_page(page);
+	put_kmapped_page(page);
 out:
 	return err;
 out_unlock:

@@ -564,7 +550,7 @@ int ext2_delete_entry (struct ext2_dir_e
 	EXT2_I(inode)->i_flags &= ~EXT2_BTREE_FL;
 	mark_inode_dirty(inode);
 out:
-	ext2_put_page(page);
+	put_kmapped_page(page);
 	return err;
 }
 

@@ -620,9 +606,9 @@ int ext2_empty_dir (struct inode * inode
 	for (i = 0; i < npages; i++) {
 		char *kaddr;
 		ext2_dirent * de;
-		page = ext2_get_page(inode, i);
+		page = read_kmap_page(inode->i_mapping, i);
 
-		if (IS_ERR(page))
+		if (IS_ERR(page) || ext2_check_page(page))
 			continue;
 
 		kaddr = page_address(page);

@@ -651,12 +637,12 @@ int ext2_empty_dir (struct inode * inode
 			}
 			de = ext2_next_entry(de);
 		}
-		ext2_put_page(page);
+		put_kmapped_page(page);
 	}
 	return 1;
 
 not_empty:
-	ext2_put_page(page);
+	put_kmapped_page(page);
 	return 0;
 }
author
#16 lore raw

PATCH 5/17 hfsplus: remove redundant read_mapping_page error check

In reply to Nate Diller

Now that read_mapping_page() does error checking internally, there is no need to check PageError here.

Signed-off-by Nate Diller <[email protected]>

---

fs/hfsplus/bnode.c +0 -4
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/hfsplus/bnode.c linux-2.6.21-rc6-mm1-test/fs/hfsplus/bnode.c
@@ -442,10 +442,6 @@ static struct hfs_bnode *__hfs_bnode_cre
 		page = read_mapping_page(mapping, block, NULL);
 		if (IS_ERR(page))
 			goto fail;
-		if (PageError(page)) {
-			page_cache_release(page);
-			goto fail;
-		}
 		page_cache_release(page);
 		node->page[i] = page;
 	}
author
#17 lore raw

PATCH 6/17 hfs: remove redundant read_mapping_page error check

In reply to Nate Diller

Now that read_mapping_page() does error checking internally, there is no need to check PageError here.

Signed-off-by Nate Diller <[email protected]>

---

fs/hfs/bnode.c +0 -4
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/hfs/bnode.c linux-2.6.21-rc6-mm1-test/fs/hfs/bnode.c
@@ -282,10 +282,6 @@ static struct hfs_bnode *__hfs_bnode_cre
 		page = read_mapping_page(mapping, block++, NULL);
 		if (IS_ERR(page))
 			goto fail;
-		if (PageError(page)) {
-			page_cache_release(page);
-			goto fail;
-		}
 		page_cache_release(page);
 		node->page[i] = page;
 	}
author
#18 lore raw

PATCH 7/17 jffs2: convert jffs2_gc_fetch_page to read_cache_page

In reply to Nate Diller

Replace jffs2_gc_fetch_page() and jffs2_gc_release_page() using the read_cache_page() and put_kmapped_page() calls, and update the call site accordingly. Explicit calls to kmap()/kunmap() make the code more clear.

Signed-off-by Nate Diller <[email protected]>

---

fs/jffs2/fs.c +0 -27
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/jffs2/fs.c linux-2.6.21-rc5-mm4-test/fs/jffs2/fs.c
@@ -621,33 +621,6 @@ struct jffs2_inode_info *jffs2_gc_fetch_
 	return JFFS2_INODE_INFO(inode);
 }
 
-unsigned char *jffs2_gc_fetch_page(struct jffs2_sb_info *c,
-				   struct jffs2_inode_info *f,
-				   unsigned long offset,
-				   unsigned long *priv)
-{
-	struct inode *inode = OFNI_EDONI_2SFFJ(f);
-	struct page *pg;
-
-	pg = read_cache_page(inode->i_mapping, offset >> PAGE_CACHE_SHIFT,
-			     (void *)jffs2_do_readpage_unlock, inode);
-	if (IS_ERR(pg))
-		return (void *)pg;
-
-	*priv = (unsigned long)pg;
-	return kmap(pg);
-}
-
-void jffs2_gc_release_page(struct jffs2_sb_info *c,
-			   unsigned char *ptr,
-			   unsigned long *priv)
-{
-	struct page *pg = (void *)*priv;
-
-	kunmap(pg);
-	page_cache_release(pg);
-}
-
 static int jffs2_flash_setup(struct jffs2_sb_info *c) {
 	int ret = 0;
fs/jffs2/gc.c +10 -5
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/jffs2/gc.c linux-2.6.21-rc5-mm4-test/fs/jffs2/gc.c
@@ -1078,7 +1078,7 @@ static int jffs2_garbage_collect_dnode(s
 	uint32_t alloclen, offset, orig_end, orig_start;
 	int ret = 0;
 	unsigned char *comprbuf = NULL, *writebuf;
-	unsigned long pg;
+	struct page *page;
 	unsigned char *pg_ptr;
 
 	memset(&ri, 0, sizeof(ri));

@@ -1219,12 +1219,16 @@ static int jffs2_garbage_collect_dnode(s
 	 *    page OK. We'll actually write it out again in commit_write, which is a little
 	 *    suboptimal, but at least we're correct.
 	 */
-	pg_ptr = jffs2_gc_fetch_page(c, f, start, &pg);
+	page = read_cache_page(OFNI_EDONI_2SFFJ(f)->i_mapping,
+				start >> PAGE_CACHE_SHIFT,
+				(void *)jffs2_do_readpage_unlock,
+				OFNI_EDONI_2SFFJ(f));
 
-	if (IS_ERR(pg_ptr)) {
+	if (IS_ERR(page)) {
 		printk(KERN_WARNING "read_cache_page() returned error: %ld\n", PTR_ERR(pg_ptr));
-		return PTR_ERR(pg_ptr);
+		return PTR_ERR(page);
 	}
+	pg_ptr = kmap(page);
 
 	offset = start;
 	while(offset < orig_end) {

@@ -1287,6 +1291,7 @@ static int jffs2_garbage_collect_dnode(s
 		}
 	}
 
-	jffs2_gc_release_page(c, pg_ptr, &pg);
+	kunmap(page);
+	page_cache_release(page);
 	return ret;
 }
#19 lore raw

Re: [PATCH 7/17] jffs2: convert jffs2_gc_fetch_page to read_cache_page

fs/jffs2/gc.c static int jffs2_garbage_collect_dnode(s · line 1228

Quoted 8 lines · Nate Diller
fs/jffs2/gc.c +5 -1
+	page = read_cache_page(OFNI_EDONI_2SFFJ(f)->i_mapping,
+				start >> PAGE_CACHE_SHIFT,
+				(void *)jffs2_do_readpage_unlock,
+				OFNI_EDONI_2SFFJ(f));
 
-	if (IS_ERR(pg_ptr)) {
+	if (IS_ERR(page)) {
 		printk(KERN_WARNING "read_cache_page() returned error: %ld\n", PTR_ERR(pg_ptr));

should be

  		printk(KERN_WARNING "read_cache_page() returned error: %ld\n", PTR_ERR(page));
Quoted 2 lines
fs/jffs2/gc.c +1 -1
-		return PTR_ERR(pg_ptr);
+		return PTR_ERR(page);
author
#20 lore raw

Re: [PATCH 7/17] jffs2: convert jffs2_gc_fetch_page to read_cache_page

Quoted 7 lines · Phillip Lougher
Quoted 5 lines · Nate Diller
fs/jffs2/gc.c +4 -0
+     page = read_cache_page(OFNI_EDONI_2SFFJ(f)->i_mapping,
+                             start >> PAGE_CACHE_SHIFT,
+                             (void *)jffs2_do_readpage_unlock,
+                             OFNI_EDONI_2SFFJ(f));
Quoted 11 lines

fs/jffs2/gc.c static int jffs2_garbage_collect_dnode(s · line 1228

Quoted 3 lines
fs/jffs2/gc.c +1 -1
-     if (IS_ERR(pg_ptr)) {
+     if (IS_ERR(page)) {
              printk(KERN_WARNING "read_cache_page() returned error: %ld\n", PTR_ERR(pg_ptr));

should be

                printk(KERN_WARNING "read_cache_page() returned error: %ld\n", PTR_ERR(page));
Quoted 2 lines
fs/jffs2/gc.c +1 -1
-             return PTR_ERR(pg_ptr);
+             return PTR_ERR(page);

wow, you're right. I was sure I compile-tested this ... oh, "depends on MTD". oops.

thanks for reviewing. does it look OK to you otherwise?

NATE

#21 lore raw

Re: [PATCH 7/17] jffs2: convert jffs2_gc_fetch_page to read_cache_page

Quoted 6 lines · Nate Diller

wow, you're right. I was sure I compile-tested this ... oh, "depends on MTD". oops.

thanks for reviewing. does it look OK to you otherwise?

Yes..

Quoted 2 lines

NATE

maintainer
#22 lore raw

Re: [PATCH 7/17] jffs2: convert jffs2_gc_fetch_page to read_cache_page

In reply to Nate Diller
Quoted 5 lines · Nate Diller

Replace jffs2_gc_fetch_page() and jffs2_gc_release_page() using the read_cache_page() and put_kmapped_page() calls, and update the call site accordingly. Explicit calls to kmap()/kunmap() make the code more clear.

Signed-off-by Nate Diller <[email protected]>

Please don't remove the jffs2_gc_{fetch,release}_page functions. The reason they're in a separate file is because that file (fs.c) is built on Linux only, while the file you're moving the code into (gc.c) is OS-agnostic; it's used on other sytems (eCos) too.

Rather than forcing eCos and any other systems which have a JFFS2 port to implement a crappy Linux 'emulation' layer, I try to keep the Linuxisms localised to certain files which can be completely reimplemented for a non-Linux port.

Signature · 1 line
dwmw2
author
#23 lore raw

PATCH 8/17 jfs: use locking read_mapping_page

In reply to Nate Diller

Use the new locking variant of read_mapping_page to avoid doing extra work.

Signed-off-by Nate Diller <[email protected]>

---

fs/jfs/jfs_metapage.c +2 -3
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/jfs/jfs_metapage.c linux-2.6.21-rc6-mm1-test/fs/jfs/jfs_metapage.c
@@ -632,12 +632,11 @@ struct metapage *__get_metapage(struct i
 		}
 		SetPageUptodate(page);
 	} else {
-		page = read_mapping_page(mapping, page_index, NULL);
-		if (IS_ERR(page) || !PageUptodate(page)) {
+		page = __read_mapping_page(mapping, page_index, NULL);
+		if (IS_ERR(page)) {
 			jfs_err("read_mapping_page failed!");
 			return NULL;
 		}
-		lock_page(page);
 	}
 
 	mp = page_to_mp(page, page_offset);
author
#24 lore raw

PATCH 9/17 minix: convert dir_get_page to read_kmap_page

In reply to Nate Diller

Replace minix dir_get_page() and dir_put_page() using the new read_kmap_page() and put_kmapped_page()/put_locked_page() calls. Also, use __read_kmap_page() instead of re-taking the page_lock.

Signed-off-by Nate Diller <[email protected]>

---

fs/minix/dir.c +17 -42
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/minix/dir.c linux-2.6.21-rc5-mm4-test/fs/minix/dir.c
@@ -23,12 +23,6 @@ const struct file_operations minix_dir_o
 	.fsync		= minix_sync_file,
 };
 
-static inline void dir_put_page(struct page *page)
-{
-	kunmap(page);
-	page_cache_release(page);
-}
-
 /*
  * Return the offset into page `page_nr' of the last valid
  * byte in that page, plus one.

@@ -60,22 +54,6 @@ static int dir_commit_chunk(struct page 
 	return err;
 }
 
-static struct page * dir_get_page(struct inode *dir, unsigned long n)
-{
-	struct address_space *mapping = dir->i_mapping;
-	struct page *page = read_mapping_page(mapping, n, NULL);
-	if (!IS_ERR(page)) {
-		kmap(page);
-		if (!PageUptodate(page))
-			goto fail;
-	}
-	return page;
-
-fail:
-	dir_put_page(page);
-	return ERR_PTR(-EIO);
-}
-
 static inline void *minix_next_entry(void *de, struct minix_sb_info *sbi)
 {
 	return (void*)((char*)de + sbi->s_dirsize);

@@ -102,7 +80,7 @@ static int minix_readdir(struct file * f
 
 	for ( ; n < npages; n++, offset = 0) {
 		char *p, *kaddr, *limit;
-		struct page *page = dir_get_page(inode, n);
+		struct page *page = read_kmap_page(inode->i_mapping, n);
 
 		if (IS_ERR(page))
 			continue;

@@ -128,12 +106,12 @@ static int minix_readdir(struct file * f
 					(n << PAGE_CACHE_SHIFT) | offset,
 					inumber, DT_UNKNOWN);
 				if (over) {
-					dir_put_page(page);
+					put_kmapped_page(page);
 					goto done;
 				}
 			}
 		}
-		dir_put_page(page);
+		put_kmapped_page(page);
 	}
 
 done:

@@ -177,7 +155,7 @@ minix_dirent *minix_find_entry(struct de
 	for (n = 0; n < npages; n++) {
 		char *kaddr, *limit;
 
-		page = dir_get_page(dir, n);
+		page = read_kmap_page(dir->i_mapping, n);
 		if (IS_ERR(page))
 			continue;
 

@@ -198,7 +176,7 @@ minix_dirent *minix_find_entry(struct de
 			if (namecompare(namelen, sbi->s_namelen, name, namx))
 				goto found;
 		}
-		dir_put_page(page);
+		put_kmapped_page(page);
 	}
 	return NULL;
 

@@ -233,11 +211,10 @@ int minix_add_link(struct dentry *dentry
 	for (n = 0; n <= npages; n++) {
 		char *limit, *dir_end;
 
-		page = dir_get_page(dir, n);
+		page = __read_kmap_page(dir->i_mapping, n);
 		err = PTR_ERR(page);
 		if (IS_ERR(page))
 			goto out;
-		lock_page(page);
 		kaddr = (char*)page_address(page);
 		dir_end = kaddr + minix_last_byte(dir, n);
 		limit = kaddr + PAGE_CACHE_SIZE - sbi->s_dirsize;

@@ -265,8 +242,7 @@ int minix_add_link(struct dentry *dentry
 			if (namecompare(namelen, sbi->s_namelen, name, namx))
 				goto out_unlock;
 		}
-		unlock_page(page);
-		dir_put_page(page);
+		put_locked_page(page);
 	}
 	BUG();
 	return -EINVAL;

@@ -288,13 +264,12 @@ got_it:
 	err = dir_commit_chunk(page, from, to);
 	dir->i_mtime = dir->i_ctime = CURRENT_TIME_SEC;
 	mark_inode_dirty(dir);
-out_put:
-	dir_put_page(page);
+	put_kmapped_page(page);
 out:
 	return err;
 out_unlock:
-	unlock_page(page);
-	goto out_put;
+	put_locked_page(page);
+	return err;
 }
 
 int minix_delete_entry(struct minix_dir_entry *de, struct page *page)

@@ -314,7 +289,7 @@ int minix_delete_entry(struct minix_dir_
 	} else {
 		unlock_page(page);
 	}
-	dir_put_page(page);
+	put_kmapped_page(page);
 	inode->i_ctime = inode->i_mtime = CURRENT_TIME_SEC;
 	mark_inode_dirty(inode);
 	return err;

@@ -378,7 +353,7 @@ int minix_empty_dir(struct inode * inode
 	for (i = 0; i < npages; i++) {
 		char *p, *kaddr, *limit;
 
-		page = dir_get_page(inode, i);
+		page = read_kmap_page(inode->i_mapping, i);
 		if (IS_ERR(page))
 			continue;
 

@@ -408,12 +383,12 @@ int minix_empty_dir(struct inode * inode
 					goto not_empty;
 			}
 		}
-		dir_put_page(page);
+		put_kmapped_page(page);
 	}
 	return 1;
 
 not_empty:
-	dir_put_page(page);
+	put_kmapped_page(page);
 	return 0;
 }
 

@@ -435,14 +410,14 @@ void minix_set_link(struct minix_dir_ent
 	} else {
 		unlock_page(page);
 	}
-	dir_put_page(page);
+	put_kmapped_page(page);
 	dir->i_mtime = dir->i_ctime = CURRENT_TIME_SEC;
 	mark_inode_dirty(dir);
 }
 
 struct minix_dir_entry * minix_dotdot (struct inode *dir, struct page **p)
 {
-	struct page *page = dir_get_page(dir, 0);
+	struct page *page = read_kmap_page(dir->i_mapping, 0);
 	struct minix_sb_info *sbi = minix_sb(dir->i_sb);
 	struct minix_dir_entry *de = NULL;
 

@@ -461,7 +436,7 @@ ino_t minix_inode_by_name(struct dentry 
 
 	if (de) {
 		res = de->inode;
-		dir_put_page(page);
+		put_kmapped_page(page);
 	}
 	return res;
 }
author
#25 lore raw

PATCH 10/17 mtd: convert page_read to read_kmap_page

In reply to Nate Diller

Replace page_read() with read_kmap_page()/__read_kmap_page(). This probably fixes behaviour on highmem systems, since page_address() was being used without kmap(). Also eliminate the need to re-take the page lock during writes to the page.

Signed-off-by Nate Diller <[email protected]>

---

drivers/mtd/devices/block2mtd.c +6 -22
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/drivers/mtd/devices/block2mtd.c linux-2.6.21-rc5-mm4-test/drivers/mtd/devices/block2mtd.c
@@ -39,12 +39,6 @@ struct block2mtd_dev {
 /* Static info about the MTD, used in cleanup_module */
 static LIST_HEAD(blkmtd_device_list);
 
-
-static struct page *page_read(struct address_space *mapping, int index)
-{
-	return read_mapping_page(mapping, index, NULL);
-}
-
 /* erase a specified part of the device */
 static int _block2mtd_erase(struct block2mtd_dev *dev, loff_t to, size_t len)
 {

@@ -56,23 +50,19 @@ static int _block2mtd_erase(struct block
 	u_long *max;
 
 	while (pages) {
-		page = page_read(mapping, index);
-		if (!page)
-			return -ENOMEM;
+		page = __read_kmap_page(mapping, index);
 		if (IS_ERR(page))
 			return PTR_ERR(page);
 
 		max = page_address(page) + PAGE_SIZE;
 		for (p=page_address(page); p<max; p++)
 			if (*p != -1UL) {
-				lock_page(page);
 				memset(page_address(page), 0xff, PAGE_SIZE);
 				set_page_dirty(page);
-				unlock_page(page);
 				break;
 			}
 
-		page_cache_release(page);
+		put_locked_page(page);
 		pages--;
 		index++;
 	}

@@ -125,14 +115,12 @@ static int block2mtd_read(struct mtd_inf
 			cpylen = len;	// this page
 		len = len - cpylen;
 
-		page = page_read(dev->blkdev->bd_inode->i_mapping, index);
-		if (!page)
-			return -ENOMEM;
+		page = read_kmap_page(dev->blkdev->bd_inode->i_mapping, index);
 		if (IS_ERR(page))
 			return PTR_ERR(page);
 
 		memcpy(buf, page_address(page) + offset, cpylen);
-		page_cache_release(page);
+		put_kmapped_page(page);
 
 		if (retlen)
 			*retlen += cpylen;

@@ -163,19 +151,15 @@ static int _block2mtd_write(struct block
 			cpylen = len;			// this page
 		len = len - cpylen;
 
-		page = page_read(mapping, index);
-		if (!page)
-			return -ENOMEM;
+		page = __read_kmap_page(mapping, index);
 		if (IS_ERR(page))
 			return PTR_ERR(page);
 
 		if (memcmp(page_address(page)+offset, buf, cpylen)) {
-			lock_page(page);
 			memcpy(page_address(page) + offset, buf, cpylen);
 			set_page_dirty(page);
-			unlock_page(page);
 		}
-		page_cache_release(page);
+		put_locked_page(page);
 
 		if (retlen)
 			*retlen += cpylen;
author
#26 lore raw

PATCH 11/17 ntfs: convert ntfs_map_page to read_kmap_page

In reply to Nate Diller

Replace ntfs_map_page() and ntfs_unmap_page() using the new read_kmap_page() and put_kmapped_page() calls, and their locking variants, and remove unneeded PageError checking.

Signed-off-by Nate Diller <[email protected]>

---

fs/ntfs/aops.h +0 -67
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ntfs/aops.h linux-2.6.21-rc5-mm4-test/fs/ntfs/aops.h
@@ -31,73 +31,6 @@
 
 #include "inode.h"
 
-/**
- * ntfs_unmap_page - release a page that was mapped using ntfs_map_page()
- * @page:	the page to release
- *
- * Unpin, unmap and release a page that was obtained from ntfs_map_page().
- */
-static inline void ntfs_unmap_page(struct page *page)
-{
-	kunmap(page);
-	page_cache_release(page);
-}
-
-/**
- * ntfs_map_page - map a page into accessible memory, reading it if necessary
- * @mapping:	address space for which to obtain the page
- * @index:	index into the page cache for @mapping of the page to map
- *
- * Read a page from the page cache of the address space @mapping at position
- * @index, where @index is in units of PAGE_CACHE_SIZE, and not in bytes.
- *
- * If the page is not in memory it is loaded from disk first using the readpage
- * method defined in the address space operations of @mapping and the page is
- * added to the page cache of @mapping in the process.
- *
- * If the page belongs to an mst protected attribute and it is marked as such
- * in its ntfs inode (NInoMstProtected()) the mst fixups are applied but no
- * error checking is performed.  This means the caller has to verify whether
- * the ntfs record(s) contained in the page are valid or not using one of the
- * ntfs_is_XXXX_record{,p}() macros, where XXXX is the record type you are
- * expecting to see.  (For details of the macros, see fs/ntfs/layout.h.)
- *
- * If the page is in high memory it is mapped into memory directly addressible
- * by the kernel.
- *
- * Finally the page count is incremented, thus pinning the page into place.
- *
- * The above means that page_address(page) can be used on all pages obtained
- * with ntfs_map_page() to get the kernel virtual address of the page.
- *
- * When finished with the page, the caller has to call ntfs_unmap_page() to
- * unpin, unmap and release the page.
- *
- * Note this does not grant exclusive access. If such is desired, the caller
- * must provide it independently of the ntfs_{un}map_page() calls by using
- * a {rw_}semaphore or other means of serialization. A spin lock cannot be
- * used as ntfs_map_page() can block.
- *
- * The unlocked and uptodate page is returned on success or an encoded error
- * on failure. Caller has to test for error using the IS_ERR() macro on the
- * return value. If that evaluates to 'true', the negative error code can be
- * obtained using PTR_ERR() on the return value of ntfs_map_page().
- */
-static inline struct page *ntfs_map_page(struct address_space *mapping,
-		unsigned long index)
-{
-	struct page *page = read_mapping_page(mapping, index, NULL);
-
-	if (!IS_ERR(page)) {
-		kmap(page);
-		if (!PageError(page))
-			return page;
-		ntfs_unmap_page(page);
-		return ERR_PTR(-EIO);
-	}
-	return page;
-}
-
 #ifdef NTFS_RW
 
 extern void mark_ntfs_record_dirty(struct page *page, const unsigned int ofs);
fs/ntfs/bitmap.c +4 -4
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ntfs/bitmap.c linux-2.6.21-rc5-mm4-test/fs/ntfs/bitmap.c
@@ -72,7 +72,7 @@ int __ntfs_bitmap_set_bits_in_run(struct
 
 	/* Get the page containing the first bit (@start_bit). */
 	mapping = vi->i_mapping;
-	page = ntfs_map_page(mapping, index);
+	page = read_kmap_page(mapping, index);
 	if (IS_ERR(page)) {
 		if (!is_rollback)
 			ntfs_error(vi->i_sb, "Failed to map first page (error "

@@ -123,8 +123,8 @@ int __ntfs_bitmap_set_bits_in_run(struct
 		/* Update @index and get the next page. */
 		flush_dcache_page(page);
 		set_page_dirty(page);
-		ntfs_unmap_page(page);
-		page = ntfs_map_page(mapping, ++index);
+		put_kmapped_page(page);
+		page = read_kmap_page(mapping, ++index);
 		if (IS_ERR(page))
 			goto rollback;
 		kaddr = page_address(page);

@@ -159,7 +159,7 @@ done:
 	/* We are done.  Unmap the page and return success. */
 	flush_dcache_page(page);
 	set_page_dirty(page);
-	ntfs_unmap_page(page);
+	put_kmapped_page(page);
 	ntfs_debug("Done.");
 	return 0;
 rollback:
fs/ntfs/dir.c +25 -40

185 lines not on this page · raw message

fs/ntfs/index.c +4 -8
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ntfs/index.c linux-2.6.21-rc5-mm4-test/fs/ntfs/index.c
@@ -64,8 +64,7 @@ void ntfs_index_ctx_put(ntfs_index_conte
 			struct page *page = ictx->page;
 			if (page) {
 				BUG_ON(!PageLocked(page));
-				unlock_page(page);
-				ntfs_unmap_page(page);
+				put_locked_page(page);
 			}
 		}
 	}

@@ -273,7 +272,7 @@ descend_into_child_node:
 	 * of PAGE_CACHE_SIZE and map the page cache page, reading it from
 	 * disk if necessary.
 	 */
-	page = ntfs_map_page(ia_mapping, vcn <<
+	page = __read_kmap_page(ia_mapping, vcn <<
 			idx_ni->itype.index.vcn_size_bits >> PAGE_CACHE_SHIFT);
 	if (IS_ERR(page)) {
 		ntfs_error(sb, "Failed to map index page, error %ld.",

@@ -281,7 +280,6 @@ descend_into_child_node:
 		err = PTR_ERR(page);
 		goto err_out;
 	}
-	lock_page(page);
 	kaddr = (u8*)page_address(page);
 fast_descend_into_child_node:
 	/* Get to the index allocation block. */

@@ -429,15 +427,13 @@ ia_done:
 				vol->cluster_size_bits >>
 				PAGE_CACHE_SHIFT)
 			goto fast_descend_into_child_node;
-		unlock_page(page);
-		ntfs_unmap_page(page);
+		put_locked_page(page);
 		goto descend_into_child_node;
 	}
 	ntfs_error(sb, "Negative child node vcn in inode 0x%lx.",
 			idx_ni->mft_no);
 unm_err_out:
-	unlock_page(page);
-	ntfs_unmap_page(page);
+	put_locked_page(page);
 err_out:
 	if (!err)
 		err = -EIO;
fs/ntfs/lcnalloc.c +3 -3
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ntfs/lcnalloc.c linux-2.6.21-rc5-mm4-test/fs/ntfs/lcnalloc.c
@@ -280,9 +280,9 @@ runlist_element *ntfs_cluster_alloc(ntfs
 				set_page_dirty(page);
 				need_writeback = 0;
 			}
-			ntfs_unmap_page(page);
+			put_kmapped_page(page);
 		}
-		page = ntfs_map_page(mapping, last_read_pos >>
+		page = read_kmap_page(mapping, last_read_pos >>
 				PAGE_CACHE_SHIFT);
 		if (IS_ERR(page)) {
 			err = PTR_ERR(page);

@@ -748,7 +748,7 @@ out:
 			set_page_dirty(page);
 			need_writeback = 0;
 		}
-		ntfs_unmap_page(page);
+		put_kmapped_page(page);
 	}
 	if (likely(!err)) {
 		up_write(&vol->lcnbmp_lock);
fs/ntfs/logfile.c +6 -6
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ntfs/logfile.c linux-2.6.21-rc5-mm4-test/fs/ntfs/logfile.c
@@ -396,7 +396,7 @@ static int ntfs_check_and_load_restart_p
 		idx = (pos + size) >> PAGE_CACHE_SHIFT;
 		BUG_ON((pos + size) & ~PAGE_CACHE_MASK);
 		do {
-			page = ntfs_map_page(vi->i_mapping, idx);
+			page = read_kmap_page(vi->i_mapping, idx);
 			if (IS_ERR(page)) {
 				ntfs_error(vi->i_sb, "Error mapping $LogFile "
 						"page (index %lu).", idx);

@@ -407,7 +407,7 @@ static int ntfs_check_and_load_restart_p
 			}
 			size = min_t(int, to_read, PAGE_CACHE_SIZE);
 			memcpy((u8*)trp + have_read, page_address(page), size);
-			ntfs_unmap_page(page);
+			put_kmapped_page(page);
 			have_read += size;
 			to_read -= size;
 			idx++;

@@ -541,8 +541,8 @@ bool ntfs_check_logfile(struct inode *lo
 		pgoff_t idx = pos >> PAGE_CACHE_SHIFT;
 		if (!page || page->index != idx) {
 			if (page)
-				ntfs_unmap_page(page);
-			page = ntfs_map_page(mapping, idx);
+				put_kmapped_page(page);
+			page = read_kmap_page(mapping, idx);
 			if (IS_ERR(page)) {
 				ntfs_error(vol->sb, "Error mapping $LogFile "
 						"page (index %lu).", idx);

@@ -602,7 +602,7 @@ bool ntfs_check_logfile(struct inode *lo
 		 * find a valid one further in the file.
 		 */
 		if (err != -EINVAL) {
-			ntfs_unmap_page(page);
+			put_kmapped_page(page);
 			goto err_out;
 		}
 		/* Continue looking. */

@@ -610,7 +610,7 @@ bool ntfs_check_logfile(struct inode *lo
 			pos = NTFS_BLOCK_SIZE >> 1;
 	}
 	if (page)
-		ntfs_unmap_page(page);
+		put_kmapped_page(page);
 	if (logfile_is_empty) {
 		NVolSetLogFileEmpty(vol);
 is_empty:
fs/ntfs/mft.c +21 -32

203 lines not on this page · raw message

fs/ntfs/super.c +19 -19
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ntfs/super.c linux-2.6.21-rc5-mm4-test/fs/ntfs/super.c
@@ -1078,11 +1078,11 @@ static bool check_mft_mirror(ntfs_volume
 		/* Switch pages if necessary. */
 		if (!(i % mrecs_per_page)) {
 			if (index) {
-				ntfs_unmap_page(mft_page);
-				ntfs_unmap_page(mirr_page);
+				put_kmapped_page(mft_page);
+				put_kmapped_page(mirr_page);
 			}
 			/* Get the $MFT page. */
-			mft_page = ntfs_map_page(vol->mft_ino->i_mapping,
+			mft_page = read_kmap_page(vol->mft_ino->i_mapping,
 					index);
 			if (IS_ERR(mft_page)) {
 				ntfs_error(sb, "Failed to read $MFT.");

@@ -1090,7 +1090,7 @@ static bool check_mft_mirror(ntfs_volume
 			}
 			kmft = page_address(mft_page);
 			/* Get the $MFTMirr page. */
-			mirr_page = ntfs_map_page(vol->mftmirr_ino->i_mapping,
+			mirr_page = read_kmap_page(vol->mftmirr_ino->i_mapping,
 					index);
 			if (IS_ERR(mirr_page)) {
 				ntfs_error(sb, "Failed to read $MFTMirr.");

@@ -1107,9 +1107,9 @@ static bool check_mft_mirror(ntfs_volume
 						"transfer detected in mft "
 						"record %i.", i);
 mm_unmap_out:
-				ntfs_unmap_page(mirr_page);
+				put_kmapped_page(mirr_page);
 mft_unmap_out:
-				ntfs_unmap_page(mft_page);
+				put_kmapped_page(mft_page);
 				return false;
 			}
 		}

@@ -1143,8 +1143,8 @@ mft_unmap_out:
 		kmirr += vol->mft_record_size;
 	} while (++i < vol->mftmirr_size);
 	/* Release the last pages. */
-	ntfs_unmap_page(mft_page);
-	ntfs_unmap_page(mirr_page);
+	put_kmapped_page(mft_page);
+	put_kmapped_page(mirr_page);
 
 	/* Construct the mft mirror runlist by hand. */
 	rl2[0].vcn = 0;

@@ -1289,7 +1289,7 @@ static int check_windows_hibernation_sta
 		goto iput_out;
 	}
 	ni = NTFS_I(vi);
-	page = ntfs_map_page(vi->i_mapping, 0);
+	page = read_kmap_page(vi->i_mapping, 0);
 	if (IS_ERR(page)) {
 		ntfs_error(vol->sb, "Failed to read from hiberfil.sys.");
 		ret = PTR_ERR(page);

@@ -1319,7 +1319,7 @@ static int check_windows_hibernation_sta
 			"volume.");
 	ret = 0;
 unm_iput_out:
-	ntfs_unmap_page(page);
+	put_kmapped_page(page);
 iput_out:
 	iput(vi);
 	return ret;

@@ -1509,7 +1509,7 @@ not_enabled:
 		return false;
 	}
 	/* Read the USN_HEADER from $DATA/$Max. */
-	page = ntfs_map_page(vol->usnjrnl_max_ino->i_mapping, 0);
+	page = read_kmap_page(vol->usnjrnl_max_ino->i_mapping, 0);
 	if (IS_ERR(page)) {
 		ntfs_error(vol->sb, "Failed to read from $UsnJrnl/$DATA/$Max "
 				"attribute.");

@@ -1523,7 +1523,7 @@ not_enabled:
 				"maximum size (0x%llx).  $UsnJrnl is corrupt.",
 				(long long)sle64_to_cpu(uh->allocation_delta),
 				(long long)sle64_to_cpu(uh->maximum_size));
-		ntfs_unmap_page(page);
+		put_kmapped_page(page);
 		return false;
 	}
 	/*

@@ -1534,7 +1534,7 @@ not_enabled:
 			i_size_read(vol->usnjrnl_j_ino))) {
 		if (likely(sle64_to_cpu(uh->lowest_valid_usn) ==
 				i_size_read(vol->usnjrnl_j_ino))) {
-			ntfs_unmap_page(page);
+			put_kmapped_page(page);
 			ntfs_debug("$UsnJrnl is enabled but nothing has been "
 					"logged since it was last stamped.  "
 					"Treating this as if the volume does "

@@ -1547,10 +1547,10 @@ not_enabled:
 				"is corrupt.",
 				(long long)sle64_to_cpu(uh->lowest_valid_usn),
 				i_size_read(vol->usnjrnl_j_ino));
-		ntfs_unmap_page(page);
+		put_kmapped_page(page);
 		return false;
 	}
-	ntfs_unmap_page(page);
+	put_kmapped_page(page);
 	ntfs_debug("Done.");
 	return true;
 }

@@ -1592,12 +1592,12 @@ static bool load_and_init_attrdef(ntfs_v
 	while (index < max_index) {
 		/* Read the attrdef table and copy it into the linear buffer. */
 read_partial_attrdef_page:
-		page = ntfs_map_page(ino->i_mapping, index);
+		page = read_kmap_page(ino->i_mapping, index);
 		if (IS_ERR(page))
 			goto free_iput_failed;
 		memcpy((u8*)vol->attrdef + (index++ << PAGE_CACHE_SHIFT),
 				page_address(page), size);
-		ntfs_unmap_page(page);
+		put_kmapped_page(page);
 	};
 	if (size == PAGE_CACHE_SIZE) {
 		size = i_size & ~PAGE_CACHE_MASK;

@@ -1661,12 +1661,12 @@ static bool load_and_init_upcase(ntfs_vo
 	while (index < max_index) {
 		/* Read the upcase table and copy it into the linear buffer. */
 read_partial_upcase_page:
-		page = ntfs_map_page(ino->i_mapping, index);
+		page = read_kmap_page(ino->i_mapping, index);
 		if (IS_ERR(page))
 			goto iput_upcase_failed;
 		memcpy((char*)vol->upcase + (index++ << PAGE_CACHE_SHIFT),
 				page_address(page), size);
-		ntfs_unmap_page(page);
+		put_kmapped_page(page);
 	};
 	if (size == PAGE_CACHE_SIZE) {
 		size = i_size & ~PAGE_CACHE_MASK;
fs/ntfs/usnjrnl.c +2 -2
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ntfs/usnjrnl.c linux-2.6.21-rc5-mm4-test/fs/ntfs/usnjrnl.c
@@ -52,7 +52,7 @@ bool ntfs_stamp_usnjrnl(ntfs_volume *vol
 		struct page *page;
 		USN_HEADER *uh;
 
-		page = ntfs_map_page(vol->usnjrnl_max_ino->i_mapping, 0);
+		page = read_kmap_page(vol->usnjrnl_max_ino->i_mapping, 0);
 		if (IS_ERR(page)) {
 			ntfs_error(vol->sb, "Failed to read from "
 					"$UsnJrnl/$DATA/$Max attribute.");

@@ -73,7 +73,7 @@ bool ntfs_stamp_usnjrnl(ntfs_volume *vol
 		uh->journal_id = stamp;
 		flush_dcache_page(page);
 		set_page_dirty(page);
-		ntfs_unmap_page(page);
+		put_kmapped_page(page);
 		/* Set the flag so we do not have to do it again on remount. */
 		NVolSetUsnJrnlStamped(vol);
 	}
author
#27 lore raw

PATCH 12/17 partition: remove redundant read_mapping_page error checks

In reply to Nate Diller

Remove unneeded PageError checking in read_dev_sector(), and clean up the code a bit.

Can anyone point out why it's OK to use page_address() here on a page which has not been kmapped? If it's not OK, then a good number of callers need to be fixed.

Signed-off-by Nate Diller <[email protected]>

---

fs/partitions/check.c +5 -9
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/partitions/check.c linux-2.6.21-rc6-mm1-test/fs/partitions/check.c
@@ -568,16 +568,12 @@ unsigned char *read_dev_sector(struct bl
 
 	page = read_mapping_page(mapping, (pgoff_t)(n >> (PAGE_CACHE_SHIFT-9)),
 				 NULL);
-	if (!IS_ERR(page)) {
-		if (PageError(page))
-			goto fail;
-		p->v = page;
-		return (unsigned char *)page_address(page) +  ((n & ((1 << (PAGE_CACHE_SHIFT - 9)) - 1)) << 9);
-fail:
-		page_cache_release(page);
+	if (IS_ERR(page)) {
+		p->v = NULL;
+		return NULL;
 	}
-	p->v = NULL;
-	return NULL;
+	p->v = page;
+	return (unsigned char *)page_address(page) +  ((n & ((1 << (PAGE_CACHE_SHIFT - 9)) - 1)) << 9);
 }
 
 EXPORT_SYMBOL(read_dev_sector);
author
#28 lore raw

PATCH 13/17 reiser4: remove redundant read_mapping_page error checks

In reply to Nate Diller

read_mapping_page() is now fully synchronous, so there's no need wait for the page lock or check for I/O errors.

Signed-off-by Nate Diller <[email protected]>

---

fs/reiser4/plugin/file/tail_conversion.c +0 -8
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/reiser4/plugin/file/tail_conversion.c linux-2.6.21-rc6-mm1-test/fs/reiser4/plugin/file/tail_conversion.c
@@ -608,14 +608,6 @@ int extent2tail(unix_file_info_t *uf_inf
 			break;
 		}
 
-		wait_on_page_locked(page);
-
-		if (!PageUptodate(page)) {
-			page_cache_release(page);
-			result = RETERR(-EIO);
-			break;
-		}
-
 		/* cut part of file we have read */
 		start_byte = (__u64) (i << PAGE_CACHE_SHIFT);
 		set_key_offset(&from, start_byte);
fs/reiser4/plugin/item/extent_file_ops.c +1 -8
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/reiser4/plugin/item/extent_file_ops.c linux-2.6.21-rc6-mm1-test/fs/reiser4/plugin/item/extent_file_ops.c
@@ -1220,15 +1220,8 @@ int reiser4_read_extent(struct file *fil
 		page = read_mapping_page(mapping, cur_page, file);
 		if (IS_ERR(page))
 			return PTR_ERR(page);
-		lock_page(page);
-		if (!PageUptodate(page)) {
-			unlock_page(page);
-			page_cache_release(page);
-			warning("jmacd-97178", "extent_read: page is not up to date");
-			return RETERR(-EIO);
-		}
+
 		mark_page_accessed(page);
-		unlock_page(page);
 
 		/* If users can be writing to this page using arbitrary virtual
 		   addresses, take care about potential aliasing before reading
author
#29 lore raw

PATCH 14/17 reiserfs: convert reiserfs_get_page to read_kmap_page

In reply to Nate Diller

Replace reiserfs_get_page() and reiserfs_put_page() using the new read_kmap_page() and put_kmapped_page() calls and their locking variants. Also, propagate the gfp_mask() deadlock comment to callsites.

Signed-off-by Nate Diller <[email protected]>

---

fs/reiserfs/xattr.c +11 -37
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/reiserfs/xattr.c linux-2.6.21-rc5-mm4-test/fs/reiserfs/xattr.c
@@ -438,33 +438,6 @@ int xattr_readdir(struct file *file, fil
 	return res;
 }
 
-/* Internal operations on file data */
-static inline void reiserfs_put_page(struct page *page)
-{
-	kunmap(page);
-	page_cache_release(page);
-}
-
-static struct page *reiserfs_get_page(struct inode *dir, unsigned long n)
-{
-	struct address_space *mapping = dir->i_mapping;
-	struct page *page;
-	/* We can deadlock if we try to free dentries,
-	   and an unlink/rmdir has just occured - GFP_NOFS avoids this */
-	mapping_set_gfp_mask(mapping, GFP_NOFS);
-	page = read_mapping_page(mapping, n, NULL);
-	if (!IS_ERR(page)) {
-		kmap(page);
-		if (PageError(page))
-			goto fail;
-	}
-	return page;
-
-      fail:
-	reiserfs_put_page(page);
-	return ERR_PTR(-EIO);
-}
-
 static inline __u32 xattr_hash(const char *msg, int len)
 {
 	return csum_partial(msg, len, 0);

@@ -537,13 +510,15 @@ reiserfs_xattr_set(struct inode *inode, 
 		else
 			chunk = buffer_size - buffer_pos;
 
-		page = reiserfs_get_page(xinode, file_pos >> PAGE_CACHE_SHIFT);
+		/* We can deadlock if we try to free dentries,
+		   and an unlink/rmdir has just occured - GFP_NOFS avoids this */
+		mapping_set_gfp_mask(mapping, GFP_NOFS);
+		page = __read_kmap_page(mapping, file_pos >> PAGE_CACHE_SHIFT);
 		if (IS_ERR(page)) {
 			err = PTR_ERR(page);
 			goto out_filp;
 		}
 
-		lock_page(page);
 		data = page_address(page);
 
 		if (file_pos == 0) {

@@ -566,8 +541,7 @@ reiserfs_xattr_set(struct inode *inode, 
 							 page_offset + chunk +
 							 skip);
 		}
-		unlock_page(page);
-		reiserfs_put_page(page);
+		put_locked_page(page);
 		buffer_pos += chunk;
 		file_pos += chunk;
 		skip = 0;

@@ -646,13 +620,15 @@ reiserfs_xattr_get(const struct inode *i
 		else
 			chunk = isize - file_pos;
 
-		page = reiserfs_get_page(xinode, file_pos >> PAGE_CACHE_SHIFT);
+		/* We can deadlock if we try to free dentries,
+		   and an unlink/rmdir has just occured - GFP_NOFS avoids this */
+		mapping_set_gfp_mask(xinode->i_mapping, GFP_NOFS);
+		page = __read_kmap_page(xinode->i_mapping, file_pos >> PAGE_CACHE_SHIFT);
 		if (IS_ERR(page)) {
 			err = PTR_ERR(page);
 			goto out_dput;
 		}
 
-		lock_page(page);
 		data = page_address(page);
 		if (file_pos == 0) {
 			struct reiserfs_xattr_header *rxh =

@@ -661,8 +637,7 @@ reiserfs_xattr_get(const struct inode *i
 			chunk -= skip;
 			/* Magic doesn't match up.. */
 			if (rxh->h_magic != cpu_to_le32(REISERFS_XATTR_MAGIC)) {
-				unlock_page(page);
-				reiserfs_put_page(page);
+				put_locked_page(page);
 				reiserfs_warning(inode->i_sb,
 						 "Invalid magic for xattr (%s) "
 						 "associated with %k", name,

@@ -673,8 +648,7 @@ reiserfs_xattr_get(const struct inode *i
 			hash = le32_to_cpu(rxh->h_hash);
 		}
 		memcpy(buffer + buffer_pos, data + skip, chunk);
-		unlock_page(page);
-		reiserfs_put_page(page);
+		put_locked_page(page);
 		file_pos += chunk;
 		buffer_pos += chunk;
 		skip = 0;
author
#30 lore raw

PATCH 15/17 sysv: convert dir_get_page to read_kmap_page

In reply to Nate Diller

Replace sysv dir_get_page() with the new read_kmap_page().

Signed-off-by Nate Diller <[email protected]>

---

fs/sysv/dir.c +5 -14
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/sysv/dir.c linux-2.6.21-rc5-mm4-test/fs/sysv/dir.c
@@ -50,15 +50,6 @@ static int dir_commit_chunk(struct page 
 	return err;
 }
 
-static struct page * dir_get_page(struct inode *dir, unsigned long n)
-{
-	struct address_space *mapping = dir->i_mapping;
-	struct page *page = read_mapping_page(mapping, n, NULL);
-	if (!IS_ERR(page))
-		kmap(page);
-	return page;
-}
-
 static int sysv_readdir(struct file * filp, void * dirent, filldir_t filldir)
 {
 	unsigned long pos = filp->f_pos;

@@ -77,7 +68,7 @@ static int sysv_readdir(struct file * fi
 	for ( ; n < npages; n++, offset = 0) {
 		char *kaddr, *limit;
 		struct sysv_dir_entry *de;
-		struct page *page = dir_get_page(inode, n);
+		struct page *page = read_kmap_page(inode->i_mapping, n);
 
 		if (IS_ERR(page))
 			continue;

@@ -149,7 +140,7 @@ struct sysv_dir_entry *sysv_find_entry(s
 
 	do {
 		char *kaddr;
-		page = dir_get_page(dir, n);
+		page = read_kmap_page(dir->i_mapping, n);
 		if (!IS_ERR(page)) {
 			kaddr = (char*)page_address(page);
 			de = (struct sysv_dir_entry *) kaddr;

@@ -191,7 +182,7 @@ int sysv_add_link(struct dentry *dentry,
 
 	/* We take care of directory expansion in the same loop */
 	for (n = 0; n <= npages; n++) {
-		page = dir_get_page(dir, n);
+		page = read_kmap_page(dir->i_mapping, n);
 		err = PTR_ERR(page);
 		if (IS_ERR(page))
 			goto out;

@@ -299,7 +290,7 @@ int sysv_empty_dir(struct inode * inode)
 	for (i = 0; i < npages; i++) {
 		char *kaddr;
 		struct sysv_dir_entry * de;
-		page = dir_get_page(inode, i);
+		page = read_kmap_page(inode->i_mapping, i);
 
 		if (IS_ERR(page))
 			continue;

@@ -353,7 +344,7 @@ void sysv_set_link(struct sysv_dir_entry
 
 struct sysv_dir_entry * sysv_dotdot (struct inode *dir, struct page **p)
 {
-	struct page *page = dir_get_page(dir, 0);
+	struct page *page = read_kmap_page(dir->i_mapping, 0);
 	struct sysv_dir_entry *de = NULL;
 
 	if (!IS_ERR(page)) {
author
#31 lore raw

PATCH 16/17 ufs: convert ufs_get_page to read_kmap_page

In reply to Nate Diller

Replace ufs_get_page()/ufs_get_locked_page() and ufs_put_page()/ufs_put_locked_page() using the new read_kmap_page() and put_kmapped_page() calls and their locking variants. Also, change the ufs_check_page() call to return the page's error status, and update the call sites accordingly.

Signed-off-by Nate Diller <[email protected]>

---

fs/ufs/balloc.c +5 -3
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ufs/balloc.c linux-2.6.21-rc5-mm4-test/fs/ufs/balloc.c
@@ -272,7 +272,7 @@ static void ufs_change_blocknr(struct in
 		index = i >> (PAGE_CACHE_SHIFT - inode->i_blkbits);
 
 		if (likely(cur_index != index)) {
-			page = ufs_get_locked_page(mapping, index);
+			page = __read_mapping_page(mapping, index, NULL);
 			if (!page)/* it was truncated */
 				continue;
 			if (IS_ERR(page)) {/* or EIO */

@@ -325,8 +325,10 @@ static void ufs_change_blocknr(struct in
 			bh = bh->b_this_page;
 		} while (bh != head);
 
-		if (likely(cur_index != index))
-			ufs_put_locked_page(page);
+		if (likely(cur_index != index)) {
+			unlock_page(page);
+			page_cache_release(page);
+		}
  	}
 	UFSD("EXIT\n");
 }
fs/ufs/truncate.c +5 -3
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ufs/truncate.c linux-2.6.21-rc5-mm4-test/fs/ufs/truncate.c
@@ -395,8 +395,9 @@ static int ufs_alloc_lastblock(struct in
 
 	lastfrag--;
 
-	lastpage = ufs_get_locked_page(mapping, lastfrag >>
-				       (PAGE_CACHE_SHIFT - inode->i_blkbits));
+	lastpage = __read_mapping_page(mapping, lastfrag >>
+				       (PAGE_CACHE_SHIFT - inode->i_blkbits),
+				       NULL);
        if (IS_ERR(lastpage)) {
                err = -EIO;
                goto out;

@@ -441,7 +442,8 @@ static int ufs_alloc_lastblock(struct in
 	       }
        }
 out_unlock:
-       ufs_put_locked_page(lastpage);
+       unlock_page(lastpage);
+       page_cache_release(lastpage);
 out:
        return err;
 }
fs/ufs/util.c +0 -52

55 lines not on this page · raw message

fs/ufs/util.h +0 -10
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/ufs/util.h linux-2.6.21-rc5-mm4-test/fs/ufs/util.h
@@ -251,16 +251,6 @@ extern void _ubh_ubhcpymem_(struct ufs_s
 #define ubh_memcpyubh(ubh,mem,size) _ubh_memcpyubh_(uspi,ubh,mem,size)
 extern void _ubh_memcpyubh_(struct ufs_sb_private_info *, struct ufs_buffer_head *, unsigned char *, unsigned);
 
-/* This functions works with cache pages*/
-extern struct page *ufs_get_locked_page(struct address_space *mapping,
-					pgoff_t index);
-static inline void ufs_put_locked_page(struct page *page)
-{
-       unlock_page(page);
-       page_cache_release(page);
-}
-
-
 /*
  * macros and inline function to get important structures from ufs_sb_private_info
  */
fs/ufs/dir.c +38 -52

228 lines not on this page · raw message

author
#32 lore raw

PATCH 17/17 vxfs: convert vxfs_get_page to read_kmap_page

In reply to Nate Diller

Replace vxfs_get_page() with the new read_kmap_page().

Signed-off-by Nate Diller <[email protected]>

---

fs/freevxfs/vxfs_extern.h +0 -1
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/freevxfs/vxfs_extern.h linux-2.6.21-rc5-mm4-test/fs/freevxfs/vxfs_extern.h
@@ -69,7 +69,6 @@ extern const struct file_operations	vxfs
 extern int			vxfs_read_olt(struct super_block *, u_long);
 
 /* vxfs_subr.c */
-extern struct page *		vxfs_get_page(struct address_space *, u_long);
 extern void			vxfs_put_page(struct page *);
 extern struct buffer_head *	vxfs_bread(struct inode *, int);
fs/freevxfs/vxfs_inode.c +1 -1
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/freevxfs/vxfs_inode.c linux-2.6.21-rc5-mm4-test/fs/freevxfs/vxfs_inode.c
@@ -138,7 +138,7 @@ __vxfs_iget(ino_t ino, struct inode *ili
 	u_long				offset;
 
 	offset = (ino % (PAGE_SIZE / VXFS_ISIZE)) * VXFS_ISIZE;
-	pp = vxfs_get_page(ilistp->i_mapping, ino * VXFS_ISIZE / PAGE_SIZE);
+	pp = read_kmap_page(ilistp->i_mapping, ino * VXFS_ISIZE / PAGE_SIZE);
 
 	if (!IS_ERR(pp)) {
 		struct vxfs_inode_info	*vip;
fs/freevxfs/vxfs_lookup.c +2 -2
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/freevxfs/vxfs_lookup.c linux-2.6.21-rc5-mm4-test/fs/freevxfs/vxfs_lookup.c
@@ -125,7 +125,7 @@ vxfs_find_entry(struct inode *ip, struct
 		caddr_t			kaddr;
 		struct page		*pp;
 
-		pp = vxfs_get_page(ip->i_mapping, page);
+		pp = read_kmap_page(ip->i_mapping, page);
 		if (IS_ERR(pp))
 			continue;
 		kaddr = (caddr_t)page_address(pp);

@@ -280,7 +280,7 @@ vxfs_readdir(struct file *fp, void *retp
 		caddr_t			kaddr;
 		struct page		*pp;
 
-		pp = vxfs_get_page(ip->i_mapping, page);
+		pp = read_kmap_page(ip->i_mapping, page);
 		if (IS_ERR(pp))
 			continue;
 		kaddr = (caddr_t)page_address(pp);
fs/freevxfs/vxfs_subr.c +0 -33
diff -urpN -X dontdiff linux-2.6.21-rc5-mm4/fs/freevxfs/vxfs_subr.c linux-2.6.21-rc5-mm4-test/fs/freevxfs/vxfs_subr.c
@@ -56,39 +56,6 @@ vxfs_put_page(struct page *pp)
 }
 
 /**
- * vxfs_get_page - read a page into memory.
- * @ip:		inode to read from
- * @n:		page number
- *
- * Description:
- *   vxfs_get_page reads the @n th page of @ip into the pagecache.
- *
- * Returns:
- *   The wanted page on success, else a NULL pointer.
- */
-struct page *
-vxfs_get_page(struct address_space *mapping, u_long n)
-{
-	struct page *			pp;
-
-	pp = read_mapping_page(mapping, n, NULL);
-
-	if (!IS_ERR(pp)) {
-		kmap(pp);
-		/** if (!PageChecked(pp)) **/
-			/** vxfs_check_page(pp); **/
-		if (PageError(pp))
-			goto fail;
-	}
-	
-	return (pp);
-		 
-fail:
-	vxfs_put_page(pp);
-	return ERR_PTR(-EIO);
-}
-
-/**
  * vxfs_bread - read buffer for a give inode,block tuple
  * @ip:		inode
  * @block:	logical block