[patch] ntfs: use add_to_page_cache_lru

[patch] ntfs: use add_to_page_cache_lru

[Nick Piggin]

ntfs: use add_to_page_cache_lru

add_to_page_cache_lru is exported, so it should be used. Benefits over
using a private pagevec: neater code, 128 bytes fewer stack used, percpu
lru ordering is preserved, and finally don't need to flush pagevec
before returning so batching may be shared with other LRU insertions.

Signed-off-by: Nick Piggin <npiggin@suse.de>:
---
 fs/ntfs/file.c |   25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

Index: linux-2.6/fs/ntfs/file.c
===================================================================
--- linux-2.6.orig/fs/ntfs/file.c
+++ linux-2.6/fs/ntfs/file.c
@@ -21,7 +21,6 @@
 
 #include <linux/buffer_head.h>
 #include <linux/pagemap.h>
-#include <linux/pagevec.h>
 #include <linux/sched.h>
 #include <linux/swap.h>
 #include <linux/uio.h>
@@ -73,7 +72,6 @@ static int ntfs_file_open(struct inode *
  * @ni:			ntfs inode of the attribute to extend
  * @new_init_size:	requested new initialized size in bytes
  * @cached_page:	store any allocated but unused page here
- * @lru_pvec:		lru-buffering pagevec of the caller
  *
  * Extend the initialized size of an attribute described by the ntfs inode @ni
  * to @new_init_size bytes.  This involves zeroing any non-sparse space between
@@ -97,8 +95,7 @@ static int ntfs_file_open(struct inode *
  * the page at all.  For a more detailed explanation see ntfs_truncate() in
  * fs/ntfs/inode.c.
  *
- * @cached_page and @lru_pvec are just optimizations for dealing with multiple
- * pages.
+ * @cached_page is just an optimization for dealing with multiple pages.
  *
  * Return 0 on success and -errno on error.  In the case that an error is
  * encountered it is possible that the initialized size will already have been
@@ -110,7 +107,7 @@ static int ntfs_file_open(struct inode *
  *	    held by the caller.
  */
 static int ntfs_attr_extend_initialized(ntfs_inode *ni, const s64 new_init_size,
-		struct page **cached_page, struct pagevec *lru_pvec)
+		struct page **cached_page)
 {
 	s64 old_init_size;
 	loff_t old_i_size;
@@ -397,13 +394,11 @@ static inline void ntfs_fault_in_pages_r
  * @nr_pages:	number of page cache pages to obtain
  * @pages:	array of pages in which to return the obtained page cache pages
  * @cached_page: allocated but as yet unused page
- * @lru_pvec:	lru-buffering pagevec of caller
  *
  * Obtain @nr_pages locked page cache pages from the mapping @mapping and
  * starting at index @index.
  *
- * If a page is newly created, increment its refcount and add it to the
- * caller's lru-buffering pagevec @lru_pvec.
+ * If a page is newly created, increment its refcount.
  *
  * This is the same as mm/filemap.c::__grab_cache_page(), except that @nr_pages
  * are obtained at once instead of just one page and that 0 is returned on
@@ -413,7 +408,7 @@ static inline void ntfs_fault_in_pages_r
  */
 static inline int __ntfs_grab_cache_pages(struct address_space *mapping,
 		pgoff_t index, const unsigned nr_pages, struct page **pages,
-		struct page **cached_page, struct pagevec *lru_pvec)
+		struct page **cached_page)
 {
 	int err, nr;
 
@@ -429,7 +424,7 @@ static inline int __ntfs_grab_cache_page
 					goto err_out;
 				}
 			}
-			err = add_to_page_cache(*cached_page, mapping, index,
+			err = add_to_page_cache_lru(*cached_page, mapping, index,
 					GFP_KERNEL);
 			if (unlikely(err)) {
 				if (err == -EEXIST)
@@ -438,8 +433,6 @@ static inline int __ntfs_grab_cache_page
 			}
 			pages[nr] = *cached_page;
 			page_cache_get(*cached_page);
-			if (unlikely(!pagevec_add(lru_pvec, *cached_page)))
-				__pagevec_lru_add_file(lru_pvec);
 			*cached_page = NULL;
 		}
 		index++;
@@ -1799,7 +1792,6 @@ static ssize_t ntfs_file_buffered_write(
 	ssize_t status, written;
 	unsigned nr_pages;
 	int err;
-	struct pagevec lru_pvec;
 
 	ntfs_debug("Entering for i_ino 0x%lx, attribute type 0x%x, "
 			"pos 0x%llx, count 0x%lx.",
@@ -1911,7 +1903,6 @@ static ssize_t ntfs_file_buffered_write(
 			}
 		}
 	}
-	pagevec_init(&lru_pvec, 0);
 	written = 0;
 	/*
 	 * If the write starts beyond the initialized size, extend it up to the
@@ -1924,8 +1915,7 @@ static ssize_t ntfs_file_buffered_write(
 	ll = ni->initialized_size;
 	read_unlock_irqrestore(&ni->size_lock, flags);
 	if (pos > ll) {
-		err = ntfs_attr_extend_initialized(ni, pos, &cached_page,
-				&lru_pvec);
+		err = ntfs_attr_extend_initialized(ni, pos, &cached_page);
 		if (err < 0) {
 			ntfs_error(vol->sb, "Cannot perform write to inode "
 					"0x%lx, attribute type 0x%x, because "
@@ -2011,7 +2001,7 @@ static ssize_t ntfs_file_buffered_write(
 			ntfs_fault_in_pages_readable_iovec(iov, iov_ofs, bytes);
 		/* Get and lock @do_pages starting at index @start_idx. */
 		status = __ntfs_grab_cache_pages(mapping, start_idx, do_pages,
-				pages, &cached_page, &lru_pvec);
+				pages, &cached_page);
 		if (unlikely(status))
 			break;
 		/*
@@ -2076,7 +2066,6 @@ err_out:
 	*ppos = pos;
 	if (cached_page)
 		page_cache_release(cached_page);
-	pagevec_lru_add_file(&lru_pvec);
 	ntfs_debug("Done.  Returning %s (written 0x%lx, status %li).",
 			written ? "written" : "status", (unsigned long)written,
 			(long)status);

Re: [patch] ntfs: use add_to_page_cache_lru

[Anton Altaparmakov]

Hi Nick,

Looks great, thanks.  Feel free to add:

Acked-by: Anton Altaparmakov <aia21@cantab.net>

Could you please send to Andrew and/or Linus for inclusion?  Thanks!

Best regards,

	Anton

On 17 Mar 2010, at 06:27, Nick Piggin wrote:

> ntfs: use add_to_page_cache_lru
> 
> add_to_page_cache_lru is exported, so it should be used. Benefits over
> using a private pagevec: neater code, 128 bytes fewer stack used, percpu
> lru ordering is preserved, and finally don't need to flush pagevec
> before returning so batching may be shared with other LRU insertions.
> 
> Signed-off-by: Nick Piggin <npiggin@suse.de>:
> ---
> fs/ntfs/file.c |   25 +++++++------------------
> 1 file changed, 7 insertions(+), 18 deletions(-)
> 
> Index: linux-2.6/fs/ntfs/file.c
> ===================================================================
> --- linux-2.6.orig/fs/ntfs/file.c
> +++ linux-2.6/fs/ntfs/file.c
> @@ -21,7 +21,6 @@
> 
> #include <linux/buffer_head.h>
> #include <linux/pagemap.h>
> -#include <linux/pagevec.h>
> #include <linux/sched.h>
> #include <linux/swap.h>
> #include <linux/uio.h>
> @@ -73,7 +72,6 @@ static int ntfs_file_open(struct inode *
>  * @ni:			ntfs inode of the attribute to extend
>  * @new_init_size:	requested new initialized size in bytes
>  * @cached_page:	store any allocated but unused page here
> - * @lru_pvec:		lru-buffering pagevec of the caller
>  *
>  * Extend the initialized size of an attribute described by the ntfs inode @ni
>  * to @new_init_size bytes.  This involves zeroing any non-sparse space between
> @@ -97,8 +95,7 @@ static int ntfs_file_open(struct inode *
>  * the page at all.  For a more detailed explanation see ntfs_truncate() in
>  * fs/ntfs/inode.c.
>  *
> - * @cached_page and @lru_pvec are just optimizations for dealing with multiple
> - * pages.
> + * @cached_page is just an optimization for dealing with multiple pages.
>  *
>  * Return 0 on success and -errno on error.  In the case that an error is
>  * encountered it is possible that the initialized size will already have been
> @@ -110,7 +107,7 @@ static int ntfs_file_open(struct inode *
>  *	    held by the caller.
>  */
> static int ntfs_attr_extend_initialized(ntfs_inode *ni, const s64 new_init_size,
> -		struct page **cached_page, struct pagevec *lru_pvec)
> +		struct page **cached_page)
> {
> 	s64 old_init_size;
> 	loff_t old_i_size;
> @@ -397,13 +394,11 @@ static inline void ntfs_fault_in_pages_r
>  * @nr_pages:	number of page cache pages to obtain
>  * @pages:	array of pages in which to return the obtained page cache pages
>  * @cached_page: allocated but as yet unused page
> - * @lru_pvec:	lru-buffering pagevec of caller
>  *
>  * Obtain @nr_pages locked page cache pages from the mapping @mapping and
>  * starting at index @index.
>  *
> - * If a page is newly created, increment its refcount and add it to the
> - * caller's lru-buffering pagevec @lru_pvec.
> + * If a page is newly created, increment its refcount.
>  *
>  * This is the same as mm/filemap.c::__grab_cache_page(), except that @nr_pages
>  * are obtained at once instead of just one page and that 0 is returned on
> @@ -413,7 +408,7 @@ static inline void ntfs_fault_in_pages_r
>  */
> static inline int __ntfs_grab_cache_pages(struct address_space *mapping,
> 		pgoff_t index, const unsigned nr_pages, struct page **pages,
> -		struct page **cached_page, struct pagevec *lru_pvec)
> +		struct page **cached_page)
> {
> 	int err, nr;
> 
> @@ -429,7 +424,7 @@ static inline int __ntfs_grab_cache_page
> 					goto err_out;
> 				}
> 			}
> -			err = add_to_page_cache(*cached_page, mapping, index,
> +			err = add_to_page_cache_lru(*cached_page, mapping, index,
> 					GFP_KERNEL);
> 			if (unlikely(err)) {
> 				if (err == -EEXIST)
> @@ -438,8 +433,6 @@ static inline int __ntfs_grab_cache_page
> 			}
> 			pages[nr] = *cached_page;
> 			page_cache_get(*cached_page);
> -			if (unlikely(!pagevec_add(lru_pvec, *cached_page)))
> -				__pagevec_lru_add_file(lru_pvec);
> 			*cached_page = NULL;
> 		}
> 		index++;
> @@ -1799,7 +1792,6 @@ static ssize_t ntfs_file_buffered_write(
> 	ssize_t status, written;
> 	unsigned nr_pages;
> 	int err;
> -	struct pagevec lru_pvec;
> 
> 	ntfs_debug("Entering for i_ino 0x%lx, attribute type 0x%x, "
> 			"pos 0x%llx, count 0x%lx.",
> @@ -1911,7 +1903,6 @@ static ssize_t ntfs_file_buffered_write(
> 			}
> 		}
> 	}
> -	pagevec_init(&lru_pvec, 0);
> 	written = 0;
> 	/*
> 	 * If the write starts beyond the initialized size, extend it up to the
> @@ -1924,8 +1915,7 @@ static ssize_t ntfs_file_buffered_write(
> 	ll = ni->initialized_size;
> 	read_unlock_irqrestore(&ni->size_lock, flags);
> 	if (pos > ll) {
> -		err = ntfs_attr_extend_initialized(ni, pos, &cached_page,
> -				&lru_pvec);
> +		err = ntfs_attr_extend_initialized(ni, pos, &cached_page);
> 		if (err < 0) {
> 			ntfs_error(vol->sb, "Cannot perform write to inode "
> 					"0x%lx, attribute type 0x%x, because "
> @@ -2011,7 +2001,7 @@ static ssize_t ntfs_file_buffered_write(
> 			ntfs_fault_in_pages_readable_iovec(iov, iov_ofs, bytes);
> 		/* Get and lock @do_pages starting at index @start_idx. */
> 		status = __ntfs_grab_cache_pages(mapping, start_idx, do_pages,
> -				pages, &cached_page, &lru_pvec);
> +				pages, &cached_page);
> 		if (unlikely(status))
> 			break;
> 		/*
> @@ -2076,7 +2066,6 @@ err_out:
> 	*ppos = pos;
> 	if (cached_page)
> 		page_cache_release(cached_page);
> -	pagevec_lru_add_file(&lru_pvec);
> 	ntfs_debug("Done.  Returning %s (written 0x%lx, status %li).",
> 			written ? "written" : "status", (unsigned long)written,
> 			(long)status);

-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/