linux-kernel - [PATCH for mmotm] remove pagevec_swap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 1 Mar 2009 19:37:37 +0900 (JST)
From:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
To:	Hugh Dickins <hugh@...itas.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Rik van Riel <riel@...hat.com>
Cc:	kosaki.motohiro@...fujitsu.com,
	Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH for mmotm] remove pagevec_swap_free()

> On Wed, 25 Feb 2009, Johannes Weiner wrote:
> 
> > The pagevec_swap_free() at the end of shrink_active_list() was
> > introduced in 68a22394 "vmscan: free swap space on swap-in/activation"
> > when shrink_active_list() was still rotating referenced active pages.
> > 
> > In 7e9cd48 "vmscan: fix pagecache reclaim referenced bit check" this
> > was changed, the rotating removed but the pagevec_swap_free() after
> > the rotation loop was forgotten, applying now to the pagevec of the
> > deactivation loop instead.
> > 
> > Now swap space is freed for deactivated pages.  And only for those
> > that happen to be on the pagevec after the deactivation loop.
> > 
> > Complete 7e9cd48 and remove the rest of the swap freeing.
> > 
> > Signed-off-by: Johannes Weiner <hannes@...xchg.org>
> > Cc: Rik van Riel <riel@...hat.com>
> 
> Nice observation.  I was going to object that the original code was
> indifferent to whether it was freeing swap from active or inactive,
> they all got lumped into the same pvec.  But that was just an oversight
> in the original code: you're right that if it was our intention to free
> swap from inactive pages here (when vm_swap_full), then we'd be freeing
> it from them in the loop above (where the buffer_heads_over_limit
> pagevec_strip is done).
> 
> Once upon a time (early 2007), testing an earlier incarnation of that
> code, I did find almost nothing being freed by that pagevec_swap_free
> anyway: other vm_swap_full frees were being effective, effective
> enough to render this one rather pointless, even when it was operating
> as intended.  But I never got around to checking on that in 2008's
> splitLRU patches, and a lot changed in between: I may be misleading.
>
> If Rik agrees (I think these do need his Ack), note that there are
> no other users of pagevec_swap_free, so you'd do well to remove it
> from mm/swap.c and include/linux/pagevec.h - I can well imagine us
> wanting to bring it back some time, but can easily look it up when
> and if we do need it again in the future.

Yup, removing is better.


==========
Subject: [PATCH] remove pagevec_swap_free()

pagevec_swap_free() is unused. 
then it can be removed.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Cc: Johannes Weiner <hannes@...xchg.org>
Cc: Rik van Riel <riel@...hat.com>
Cc: Hugh Dickins <hugh@...itas.com>
---
 include/linux/pagevec.h |    1 -
 mm/swap.c               |   23 -----------------------
 2 files changed, 0 insertions(+), 24 deletions(-)

diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h
index 7b2886f..bab82f4 100644
--- a/include/linux/pagevec.h
+++ b/include/linux/pagevec.h
@@ -24,7 +24,6 @@ void __pagevec_release(struct pagevec *pvec);
 void __pagevec_free(struct pagevec *pvec);
 void ____pagevec_lru_add(struct pagevec *pvec, enum lru_list lru);
 void pagevec_strip(struct pagevec *pvec);
-void pagevec_swap_free(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,
diff --git a/mm/swap.c b/mm/swap.c
index eee08df..d33e499 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -471,29 +471,6 @@ void pagevec_strip(struct pagevec *pvec)
 }
 
 /**
- * pagevec_swap_free - try to free swap space from the pages in a pagevec
- * @pvec: pagevec with swapcache pages to free the swap space of
- *
- * The caller needs to hold an extra reference to each page and
- * not hold the page lock on the pages.  This function uses a
- * trylock on the page lock so it may not always free the swap
- * space associated with a page.
- */
-void pagevec_swap_free(struct pagevec *pvec)
-{
-	int i;
-
-	for (i = 0; i < pagevec_count(pvec); i++) {
-		struct page *page = pvec->pages[i];
-
-		if (PageSwapCache(page) && trylock_page(page)) {
-			try_to_free_swap(page);
-			unlock_page(page);
-		}
-	}
-}
-
-/**
  * pagevec_lookup - gang pagecache lookup
  * @pvec:	Where the resulting pages are placed
  * @mapping:	The address_space to search
-- 
1.6.0.6



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/