[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130503080043.GC10633@quack.suse.cz>
Date: Fri, 3 May 2013 10:00:43 +0200
From: Jan Kara <jack@...e.cz>
To: Mel Gorman <mgorman@...e.de>
Cc: Alexey Lyahkov <alexey.lyashkov@...il.com>,
Andrew Perepechko <anserper@...ru>,
Robin Dong <sanbai@...bao.com>, Theodore Tso <tytso@....edu>,
Andrew Morton <akpm@...ux-foundation.org>,
Hugh Dickins <hughd@...gle.com>,
Rik van Riel <riel@...hat.com>,
Johannes Weiner <hannes@...xchg.org>,
Bernd Schubert <bernd.schubert@...tmail.fm>,
David Howells <dhowells@...hat.com>,
Trond Myklebust <Trond.Myklebust@...app.com>,
Linux-fsdevel <linux-fsdevel@...r.kernel.org>,
Linux-ext4 <linux-ext4@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Linux-mm <linux-mm@...ck.org>
Subject: Re: [PATCH 3/3] mm: Remove lru parameter from __pagevec_lru_add
and remove parts of pagevec API
On Mon 29-04-13 17:31:59, Mel Gorman wrote:
> Now that the LRU to add a page to is decided at LRU-add time, remove the
> misleading lru parameter from __pagevec_lru_add. A consequence of this is
> that the pagevec_lru_add_file, pagevec_lru_add_anon and similar helpers
> are misleading as the caller no longer has direct control over what LRU
> the page is added to. Unused helpers are removed by this patch and existing
> users of pagevec_lru_add_file() are converted to use lru_cache_add_file()
> directly and use the per-cpu pagevecs instead of creating their own pagevec.
Ah, OK, in this patch you remove the bogus argument and the warning. You
can add:
Reviewed-by: Jan Kara <jack@...e.cz>
for whatever it is worth for mm related patches :)
Honza
>
> Signed-off-by: Mel Gorman <mgorman@...e.de>
> ---
> fs/cachefiles/rdwr.c | 30 +++++++-----------------------
> fs/nfs/dir.c | 7 ++-----
> include/linux/pagevec.h | 34 +---------------------------------
> mm/swap.c | 12 ++++--------
> 4 files changed, 14 insertions(+), 69 deletions(-)
>
> diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c
> index 4809922..d24c13e 100644
> --- a/fs/cachefiles/rdwr.c
> +++ b/fs/cachefiles/rdwr.c
> @@ -12,6 +12,7 @@
> #include <linux/mount.h>
> #include <linux/slab.h>
> #include <linux/file.h>
> +#include <linux/swap.h>
> #include "internal.h"
>
> /*
> @@ -227,8 +228,7 @@ static void cachefiles_read_copier(struct fscache_operation *_op)
> */
> static int cachefiles_read_backing_file_one(struct cachefiles_object *object,
> struct fscache_retrieval *op,
> - struct page *netpage,
> - struct pagevec *pagevec)
> + struct page *netpage)
> {
> struct cachefiles_one_read *monitor;
> struct address_space *bmapping;
> @@ -237,8 +237,6 @@ static int cachefiles_read_backing_file_one(struct cachefiles_object *object,
>
> _enter("");
>
> - pagevec_reinit(pagevec);
> -
> _debug("read back %p{%lu,%d}",
> netpage, netpage->index, page_count(netpage));
>
> @@ -283,9 +281,7 @@ installed_new_backing_page:
> backpage = newpage;
> newpage = NULL;
>
> - page_cache_get(backpage);
> - pagevec_add(pagevec, backpage);
> - __pagevec_lru_add_file(pagevec);
> + lru_cache_add_file(backpage);
>
> read_backing_page:
> ret = bmapping->a_ops->readpage(NULL, backpage);
> @@ -452,8 +448,7 @@ int cachefiles_read_or_alloc_page(struct fscache_retrieval *op,
> if (block) {
> /* submit the apparently valid page to the backing fs to be
> * read from disk */
> - ret = cachefiles_read_backing_file_one(object, op, page,
> - &pagevec);
> + ret = cachefiles_read_backing_file_one(object, op, page);
> } else if (cachefiles_has_space(cache, 0, 1) == 0) {
> /* there's space in the cache we can use */
> fscache_mark_page_cached(op, page);
> @@ -482,14 +477,11 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
> {
> struct cachefiles_one_read *monitor = NULL;
> struct address_space *bmapping = object->backer->d_inode->i_mapping;
> - struct pagevec lru_pvec;
> struct page *newpage = NULL, *netpage, *_n, *backpage = NULL;
> int ret = 0;
>
> _enter("");
>
> - pagevec_init(&lru_pvec, 0);
> -
> list_for_each_entry_safe(netpage, _n, list, lru) {
> list_del(&netpage->lru);
>
> @@ -534,9 +526,7 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
> backpage = newpage;
> newpage = NULL;
>
> - page_cache_get(backpage);
> - if (!pagevec_add(&lru_pvec, backpage))
> - __pagevec_lru_add_file(&lru_pvec);
> + lru_cache_add_file(backpage);
>
> reread_backing_page:
> ret = bmapping->a_ops->readpage(NULL, backpage);
> @@ -559,9 +549,7 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
> goto nomem;
> }
>
> - page_cache_get(netpage);
> - if (!pagevec_add(&lru_pvec, netpage))
> - __pagevec_lru_add_file(&lru_pvec);
> + lru_cache_add_file(netpage);
>
> /* install a monitor */
> page_cache_get(netpage);
> @@ -643,9 +631,7 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
>
> fscache_mark_page_cached(op, netpage);
>
> - page_cache_get(netpage);
> - if (!pagevec_add(&lru_pvec, netpage))
> - __pagevec_lru_add_file(&lru_pvec);
> + lru_cache_add_file(netpage);
>
> /* the netpage is unlocked and marked up to date here */
> fscache_end_io(op, netpage, 0);
> @@ -661,8 +647,6 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
>
> out:
> /* tidy up */
> - pagevec_lru_add_file(&lru_pvec);
> -
> if (newpage)
> page_cache_release(newpage);
> if (netpage)
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index f23f455..787d89c 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -33,6 +33,7 @@
> #include <linux/pagevec.h>
> #include <linux/namei.h>
> #include <linux/mount.h>
> +#include <linux/swap.h>
> #include <linux/sched.h>
> #include <linux/kmemleak.h>
> #include <linux/xattr.h>
> @@ -1757,7 +1758,6 @@ EXPORT_SYMBOL_GPL(nfs_unlink);
> */
> int nfs_symlink(struct inode *dir, struct dentry *dentry, const char *symname)
> {
> - struct pagevec lru_pvec;
> struct page *page;
> char *kaddr;
> struct iattr attr;
> @@ -1797,11 +1797,8 @@ int nfs_symlink(struct inode *dir, struct dentry *dentry, const char *symname)
> * No big deal if we can't add this page to the page cache here.
> * READLINK will get the missing page from the server if needed.
> */
> - pagevec_init(&lru_pvec, 0);
> - if (!add_to_page_cache(page, dentry->d_inode->i_mapping, 0,
> + if (!add_to_page_cache_lru(page, dentry->d_inode->i_mapping, 0,
> GFP_KERNEL)) {
> - pagevec_add(&lru_pvec, page);
> - pagevec_lru_add_file(&lru_pvec);
> SetPageUptodate(page);
> unlock_page(page);
> } else
> diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h
> index 2aa12b8..e4dbfab3 100644
> --- a/include/linux/pagevec.h
> +++ b/include/linux/pagevec.h
> @@ -21,7 +21,7 @@ struct pagevec {
> };
>
> void __pagevec_release(struct pagevec *pvec);
> -void __pagevec_lru_add(struct pagevec *pvec, enum lru_list lru);
> +void __pagevec_lru_add(struct pagevec *pvec);
> unsigned pagevec_lookup(struct pagevec *pvec, struct address_space *mapping,
> pgoff_t start, unsigned nr_pages);
> unsigned pagevec_lookup_tag(struct pagevec *pvec,
> @@ -64,36 +64,4 @@ static inline void pagevec_release(struct pagevec *pvec)
> __pagevec_release(pvec);
> }
>
> -static inline void __pagevec_lru_add_anon(struct pagevec *pvec)
> -{
> - __pagevec_lru_add(pvec, LRU_INACTIVE_ANON);
> -}
> -
> -static inline void __pagevec_lru_add_active_anon(struct pagevec *pvec)
> -{
> - __pagevec_lru_add(pvec, LRU_ACTIVE_ANON);
> -}
> -
> -static inline void __pagevec_lru_add_file(struct pagevec *pvec)
> -{
> - __pagevec_lru_add(pvec, LRU_INACTIVE_FILE);
> -}
> -
> -static inline void __pagevec_lru_add_active_file(struct pagevec *pvec)
> -{
> - __pagevec_lru_add(pvec, LRU_ACTIVE_FILE);
> -}
> -
> -static inline void pagevec_lru_add_file(struct pagevec *pvec)
> -{
> - if (pagevec_count(pvec))
> - __pagevec_lru_add_file(pvec);
> -}
> -
> -static inline void pagevec_lru_add_anon(struct pagevec *pvec)
> -{
> - if (pagevec_count(pvec))
> - __pagevec_lru_add_anon(pvec);
> -}
> -
> #endif /* _LINUX_PAGEVEC_H */
> diff --git a/mm/swap.c b/mm/swap.c
> index 2a10d08..83eff2f 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -474,7 +474,7 @@ void __lru_cache_add(struct page *page, enum lru_list lru)
>
> page_cache_get(page);
> if (!pagevec_space(pvec))
> - __pagevec_lru_add(pvec, lru);
> + __pagevec_lru_add(pvec);
> pagevec_add(pvec, page);
> put_cpu_var(lru_add_pvec);
> }
> @@ -594,7 +594,7 @@ void lru_add_drain_cpu(int cpu)
> struct pagevec *pvec = &per_cpu(lru_add_pvec, cpu);
>
> if (pagevec_count(pvec))
> - __pagevec_lru_add(pvec, NR_LRU_LISTS);
> + __pagevec_lru_add(pvec);
>
> pvec = &per_cpu(lru_rotate_pvecs, cpu);
> if (pagevec_count(pvec)) {
> @@ -793,12 +793,10 @@ void lru_add_page_tail(struct page *page, struct page *page_tail,
> static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
> void *arg)
> {
> - enum lru_list requested_lru = (enum lru_list)arg;
> int file = page_is_file_cache(page);
> int active = PageActive(page);
> enum lru_list lru = page_lru(page);
>
> - WARN_ON_ONCE(requested_lru < NR_LRU_LISTS && requested_lru != lru);
> VM_BUG_ON(PageUnevictable(page));
> VM_BUG_ON(PageLRU(page));
>
> @@ -811,11 +809,9 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
> * Add the passed pages to the LRU, then drop the caller's refcount
> * on them. Reinitialises the caller's pagevec.
> */
> -void __pagevec_lru_add(struct pagevec *pvec, enum lru_list lru)
> +void __pagevec_lru_add(struct pagevec *pvec)
> {
> - VM_BUG_ON(is_unevictable_lru(lru));
> -
> - pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, (void *)lru);
> + pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, NULL);
> }
> EXPORT_SYMBOL(__pagevec_lru_add);
>
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists