[PATCH 01/11] mm: frontswap: remove casting from function calls through ops structure

[PATCH 01/11] mm: frontswap: remove casting from function calls through ops structure

[Sasha Levin]

Removes unneeded casts.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 mm/frontswap.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 15b79fb..07c0eee 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -90,7 +90,7 @@ void __frontswap_init(unsigned type)
 	if (sis->frontswap_map == NULL)
 		return;
 	if (frontswap_enabled)
-		(*frontswap_ops.init)(type);
+		frontswap_ops.init(type);
 }
 EXPORT_SYMBOL(__frontswap_init);
 
@@ -113,7 +113,7 @@ int __frontswap_put_page(struct page *page)
 	BUG_ON(sis == NULL);
 	if (frontswap_test(sis, offset))
 		dup = 1;
-	ret = (*frontswap_ops.put_page)(type, offset, page);
+	ret = frontswap_ops.put_page(type, offset, page);
 	if (ret == 0) {
 		frontswap_set(sis, offset);
 		frontswap_succ_puts++;
@@ -152,7 +152,7 @@ int __frontswap_get_page(struct page *page)
 	BUG_ON(!PageLocked(page));
 	BUG_ON(sis == NULL);
 	if (frontswap_test(sis, offset))
-		ret = (*frontswap_ops.get_page)(type, offset, page);
+		ret = frontswap_ops.get_page(type, offset, page);
 	if (ret == 0)
 		frontswap_gets++;
 	return ret;
@@ -169,7 +169,7 @@ void __frontswap_invalidate_page(unsigned type, pgoff_t offset)
 
 	BUG_ON(sis == NULL);
 	if (frontswap_test(sis, offset)) {
-		(*frontswap_ops.invalidate_page)(type, offset);
+		frontswap_ops.invalidate_page(type, offset);
 		atomic_dec(&sis->frontswap_pages);
 		frontswap_clear(sis, offset);
 		frontswap_invalidates++;
@@ -188,7 +188,7 @@ void __frontswap_invalidate_area(unsigned type)
 	BUG_ON(sis == NULL);
 	if (sis->frontswap_map == NULL)
 		return;
-	(*frontswap_ops.invalidate_area)(type);
+	frontswap_ops.invalidate_area(type);
 	atomic_set(&sis->frontswap_pages, 0);
 	memset(sis->frontswap_map, 0, sis->max / sizeof(long));
 }
-- 
1.7.8.6

[PATCH 02/11] mm: frontswap: trivial coding convention issues

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 mm/frontswap.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 07c0eee..844d6a6 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -127,8 +127,9 @@ int __frontswap_put_page(struct page *page)
 		frontswap_clear(sis, offset);
 		atomic_dec(&sis->frontswap_pages);
 		frontswap_failed_puts++;
-	} else
+	} else {
 		frontswap_failed_puts++;
+	}
 	if (frontswap_writethrough_enabled)
 		/* report failure so swap also writes to swap device */
 		ret = -1;
@@ -229,9 +230,9 @@ void frontswap_shrink(unsigned long target_pages)
 	for (type = swap_list.head; type >= 0; type = si->next) {
 		si = swap_info[type];
 		si_frontswap_pages = atomic_read(&si->frontswap_pages);
-		if (total_pages_to_unuse < si_frontswap_pages)
+		if (total_pages_to_unuse < si_frontswap_pages) {
 			pages = pages_to_unuse = total_pages_to_unuse;
-		else {
+		} else {
 			pages = si_frontswap_pages;
 			pages_to_unuse = 0; /* unuse all */
 		}
-- 
1.7.8.6

[PATCH 03/11] mm: frontswap: split out __frontswap_curr_pages

[Sasha Levin]

Code was duplicated in two functions, clean it up.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 mm/frontswap.c |   28 +++++++++++++++++-----------
 1 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 844d6a6..52b9dab 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -195,6 +195,20 @@ void __frontswap_invalidate_area(unsigned type)
 }
 EXPORT_SYMBOL(__frontswap_invalidate_area);
 
+static unsigned long __frontswap_curr_pages(void)
+{
+	int type;
+	unsigned long totalpages = 0;
+	struct swap_info_struct *si = NULL;
+
+	lockdep_assert_held(&swap_lock);
+	for (type = swap_list.head; type >= 0; type = si->next) {
+		si = swap_info[type];
+		totalpages += atomic_read(&si->frontswap_pages);
+	}
+	return totalpages;
+}
+
 /*
  * Frontswap, like a true swap device, may unnecessarily retain pages
  * under certain circumstances; "shrink" frontswap is essentially a
@@ -219,11 +233,7 @@ void frontswap_shrink(unsigned long target_pages)
 	 */
 	spin_lock(&swap_lock);
 	locked = true;
-	total_pages = 0;
-	for (type = swap_list.head; type >= 0; type = si->next) {
-		si = swap_info[type];
-		total_pages += atomic_read(&si->frontswap_pages);
-	}
+	total_pages = __frontswap_curr_pages();
 	if (total_pages <= target_pages)
 		goto out;
 	total_pages_to_unuse = total_pages - target_pages;
@@ -261,16 +271,12 @@ EXPORT_SYMBOL(frontswap_shrink);
  */
 unsigned long frontswap_curr_pages(void)
 {
-	int type;
 	unsigned long totalpages = 0;
-	struct swap_info_struct *si = NULL;
 
 	spin_lock(&swap_lock);
-	for (type = swap_list.head; type >= 0; type = si->next) {
-		si = swap_info[type];
-		totalpages += atomic_read(&si->frontswap_pages);
-	}
+	totalpages = __frontswap_curr_pages();
 	spin_unlock(&swap_lock);
+
 	return totalpages;
 }
 EXPORT_SYMBOL(frontswap_curr_pages);
-- 
1.7.8.6

[PATCH 04/11] mm: frontswap: split out __frontswap_unuse_pages

[Sasha Levin]

An attempt at making frontswap_shrink shorter and more readable. This patch
splits out walking through the swap list to find an entry with enough
pages to unuse.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 mm/frontswap.c |   59 +++++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 52b9dab..a9b76cb 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -209,6 +209,41 @@ static unsigned long __frontswap_curr_pages(void)
 	return totalpages;
 }
 
+static int __frontswap_unuse_pages(unsigned long total, unsigned long *unused,
+					int *swapid)
+{
+	int ret = -EINVAL;
+	struct swap_info_struct *si = NULL;
+	int si_frontswap_pages;
+	unsigned long total_pages_to_unuse = total;
+	unsigned long pages = 0, pages_to_unuse = 0;
+	int type;
+
+	lockdep_assert_held(&swap_lock);
+	for (type = swap_list.head; type >= 0; type = si->next) {
+		si = swap_info[type];
+		si_frontswap_pages = atomic_read(&si->frontswap_pages);
+		if (total_pages_to_unuse < si_frontswap_pages) {
+			pages = pages_to_unuse = total_pages_to_unuse;
+		} else {
+			pages = si_frontswap_pages;
+			pages_to_unuse = 0; /* unuse all */
+		}
+		/* ensure there is enough RAM to fetch pages from frontswap */
+		if (security_vm_enough_memory_mm(current->mm, pages)) {
+			ret = -ENOMEM;
+			continue;
+		}
+		vm_unacct_memory(pages);
+		*unused = pages_to_unuse;
+		*swapid = type;
+		ret = 0;
+		break;
+	}
+
+	return ret;
+}
+
 /*
  * Frontswap, like a true swap device, may unnecessarily retain pages
  * under certain circumstances; "shrink" frontswap is essentially a
@@ -219,11 +254,9 @@ static unsigned long __frontswap_curr_pages(void)
  */
 void frontswap_shrink(unsigned long target_pages)
 {
-	struct swap_info_struct *si = NULL;
-	int si_frontswap_pages;
 	unsigned long total_pages = 0, total_pages_to_unuse;
-	unsigned long pages = 0, pages_to_unuse = 0;
-	int type;
+	unsigned long pages_to_unuse = 0;
+	int type, ret;
 	bool locked = false;
 
 	/*
@@ -237,22 +270,8 @@ void frontswap_shrink(unsigned long target_pages)
 	if (total_pages <= target_pages)
 		goto out;
 	total_pages_to_unuse = total_pages - target_pages;
-	for (type = swap_list.head; type >= 0; type = si->next) {
-		si = swap_info[type];
-		si_frontswap_pages = atomic_read(&si->frontswap_pages);
-		if (total_pages_to_unuse < si_frontswap_pages) {
-			pages = pages_to_unuse = total_pages_to_unuse;
-		} else {
-			pages = si_frontswap_pages;
-			pages_to_unuse = 0; /* unuse all */
-		}
-		/* ensure there is enough RAM to fetch pages from frontswap */
-		if (security_vm_enough_memory_mm(current->mm, pages))
-			continue;
-		vm_unacct_memory(pages);
-		break;
-	}
-	if (type < 0)
+	ret = __frontswap_unuse_pages(total_pages_to_unuse, &pages_to_unuse, &type);
+	if (ret < 0)
 		goto out;
 	locked = false;
 	spin_unlock(&swap_lock);
-- 
1.7.8.6

[PATCH 05/11] mm: frontswap: split frontswap_shrink further to eliminate locking games

[Sasha Levin]

Split frontswap_shrink to eliminate the locking issues in the original code.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 mm/frontswap.c |   36 +++++++++++++++++++++---------------
 1 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index a9b76cb..618ef91 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -244,6 +244,24 @@ static int __frontswap_unuse_pages(unsigned long total, unsigned long *unused,
 	return ret;
 }
 
+static int __frontswap_shrink(unsigned long target_pages,
+				unsigned long *pages_to_unuse,
+				int *type)
+{
+	unsigned long total_pages = 0, total_pages_to_unuse;
+
+	lockdep_assert_held(&swap_lock);
+
+	total_pages = __frontswap_curr_pages();
+	if (total_pages <= target_pages) {
+		/* Nothing to do */
+		*pages_to_unuse = 0;
+		return 0;
+	}
+	total_pages_to_unuse = total_pages - target_pages;
+	return __frontswap_unuse_pages(total_pages_to_unuse, pages_to_unuse, type);
+}
+
 /*
  * Frontswap, like a true swap device, may unnecessarily retain pages
  * under certain circumstances; "shrink" frontswap is essentially a
@@ -254,10 +272,8 @@ static int __frontswap_unuse_pages(unsigned long total, unsigned long *unused,
  */
 void frontswap_shrink(unsigned long target_pages)
 {
-	unsigned long total_pages = 0, total_pages_to_unuse;
 	unsigned long pages_to_unuse = 0;
 	int type, ret;
-	bool locked = false;
 
 	/*
 	 * we don't want to hold swap_lock while doing a very
@@ -265,20 +281,10 @@ void frontswap_shrink(unsigned long target_pages)
 	 * so restart scan from swap_list.head each time
 	 */
 	spin_lock(&swap_lock);
-	locked = true;
-	total_pages = __frontswap_curr_pages();
-	if (total_pages <= target_pages)
-		goto out;
-	total_pages_to_unuse = total_pages - target_pages;
-	ret = __frontswap_unuse_pages(total_pages_to_unuse, &pages_to_unuse, &type);
-	if (ret < 0)
-		goto out;
-	locked = false;
+	ret = __frontswap_shrink(target_pages, &pages_to_unuse, &type);
 	spin_unlock(&swap_lock);
-	try_to_unuse(type, true, pages_to_unuse);
-out:
-	if (locked)
-		spin_unlock(&swap_lock);
+	if (ret == 0 && pages_to_unuse)
+		try_to_unuse(type, true, pages_to_unuse);
 	return;
 }
 EXPORT_SYMBOL(frontswap_shrink);
-- 
1.7.8.6

Re: [PATCH 05/11] mm: frontswap: split frontswap_shrink further to eliminate locking games

[Sasha Levin]

On Wed, 2012-06-06 at 07:30 -0400, Konrad Rzeszutek Wilk wrote:
> 
> On Jun 6, 2012 6:55 AM, "Sasha Levin" <levinsasha928@gmail.com> wrote:
> >
> > Split frontswap_shrink to eliminate the locking issues in the
> original code.
> 
> Can you describe the locking issue please?

I may have worded that wrong, it's less of an "issue" and more of a
complicated code to deal with locking, specifically the extra local
variable to keep track whether we are locking or not.

Re: [PATCH 05/11] mm: frontswap: split frontswap_shrink further to eliminate locking games

[Konrad Rzeszutek Wilk]

On Wed, Jun 06, 2012 at 01:37:17PM +0200, Sasha Levin wrote:
> On Wed, 2012-06-06 at 07:30 -0400, Konrad Rzeszutek Wilk wrote:
> > 
> > On Jun 6, 2012 6:55 AM, "Sasha Levin" <levinsasha928@gmail.com> wrote:
> > >
> > > Split frontswap_shrink to eliminate the locking issues in the
> > original code.
> > 
> > Can you describe the locking issue please?
> 
> I may have worded that wrong, it's less of an "issue" and more of a
> complicated code to deal with locking, specifically the extra local
> variable to keep track whether we are locking or not.

So "simplying the locking"?

[PATCH 06/11] mm: frontswap: make all branches of if statement in put page consistent

[Sasha Levin]

Currently it has a complex structure where different things are compared
at each branch. Simplify that and make both branches look similar.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 mm/frontswap.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 618ef91..f2f4685 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -119,16 +119,16 @@ int __frontswap_put_page(struct page *page)
 		frontswap_succ_puts++;
 		if (!dup)
 			atomic_inc(&sis->frontswap_pages);
-	} else if (dup) {
+	} else {
 		/*
 		  failed dup always results in automatic invalidate of
 		  the (older) page from frontswap
 		 */
-		frontswap_clear(sis, offset);
-		atomic_dec(&sis->frontswap_pages);
-		frontswap_failed_puts++;
-	} else {
 		frontswap_failed_puts++;
+		if (dup) {
+			frontswap_clear(sis, offset);
+			atomic_dec(&sis->frontswap_pages);
+		}
 	}
 	if (frontswap_writethrough_enabled)
 		/* report failure so swap also writes to swap device */
-- 
1.7.8.6

Re: [PATCH 06/11] mm: frontswap: make all branches of if statement in put page consistent

[Konrad Rzeszutek Wilk]

On Wed, Jun 06, 2012 at 12:55:10PM +0200, Sasha Levin wrote:
> Currently it has a complex structure where different things are compared
> at each branch. Simplify that and make both branches look similar.
> 
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
>  mm/frontswap.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/frontswap.c b/mm/frontswap.c
> index 618ef91..f2f4685 100644
> --- a/mm/frontswap.c
> +++ b/mm/frontswap.c
> @@ -119,16 +119,16 @@ int __frontswap_put_page(struct page *page)
>  		frontswap_succ_puts++;
>  		if (!dup)
>  			atomic_inc(&sis->frontswap_pages);
> -	} else if (dup) {
> +	} else {
>  		/*
>  		  failed dup always results in automatic invalidate of
>  		  the (older) page from frontswap
>  		 */
> -		frontswap_clear(sis, offset);
> -		atomic_dec(&sis->frontswap_pages);
> -		frontswap_failed_puts++;

Hmm, you must be using an older branch b/c the frontswap_failed_puts++
doesn't exist anymore. Could you rebase on top of linus/master please.

> -	} else {
>  		frontswap_failed_puts++;
> +		if (dup) {
> +			frontswap_clear(sis, offset);
> +			atomic_dec(&sis->frontswap_pages);
> +		}
>  	}
>  	if (frontswap_writethrough_enabled)
>  		/* report failure so swap also writes to swap device */
> -- 
> 1.7.8.6

RE: [PATCH 06/11] mm: frontswap: make all branches of if statement in put page consistent

[Dan Magenheimer]

> From: Konrad Rzeszutek Wilk
> Subject: Re: [PATCH 06/11] mm: frontswap: make all branches of if statement in put page consistent
> 
> On Wed, Jun 06, 2012 at 12:55:10PM +0200, Sasha Levin wrote:
> > Currently it has a complex structure where different things are compared
> > at each branch. Simplify that and make both branches look similar.
> >
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> >  mm/frontswap.c |   10 +++++-----
> >  1 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/frontswap.c b/mm/frontswap.c
> > index 618ef91..f2f4685 100644
> > --- a/mm/frontswap.c
> > +++ b/mm/frontswap.c
> > @@ -119,16 +119,16 @@ int __frontswap_put_page(struct page *page)
> >  		frontswap_succ_puts++;
> >  		if (!dup)
> >  			atomic_inc(&sis->frontswap_pages);
> > -	} else if (dup) {
> > +	} else {
> >  		/*
> >  		  failed dup always results in automatic invalidate of
> >  		  the (older) page from frontswap
> >  		 */
> > -		frontswap_clear(sis, offset);
> > -		atomic_dec(&sis->frontswap_pages);
> > -		frontswap_failed_puts++;
> 
> Hmm, you must be using an older branch b/c the frontswap_failed_puts++
> doesn't exist anymore. Could you rebase on top of linus/master please.

Reminds me... at some point I removed the ability to observe
and set frontswap_curr_pages from userland, which is very useful
in testing and could be useful for future userland sysadmin tools.
IIRC, when transitioning from sysfs to debugfs (akpm's feedback),
I couldn't figure out how to make it writeable from debugfs, so
just dropped it and never added it back.

Writing the value is like a partial (or full) swapoff of the
pages stored via frontswap into zcache/ramster/tmem.  So kinda
similar to drop_caches?

I'd sure welcome a patch to add that back in!

[PATCH 07/11] mm: frontswap: remove unnecessary check during initialization

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 mm/frontswap.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index f2f4685..bf99c7d 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -89,8 +89,7 @@ void __frontswap_init(unsigned type)
 	BUG_ON(sis == NULL);
 	if (sis->frontswap_map == NULL)
 		return;
-	if (frontswap_enabled)
-		frontswap_ops.init(type);
+	frontswap_ops.init(type);
 }
 EXPORT_SYMBOL(__frontswap_init);
 
-- 
1.7.8.6

Re: [PATCH 07/11] mm: frontswap: remove unnecessary check during initialization

[Konrad Rzeszutek Wilk]

On Wed, Jun 06, 2012 at 12:55:11PM +0200, Sasha Levin wrote:

Could you explain in the git commit why it is unnecessary?
I am pretty sure I know - it is b/c frontswap_init already
does the check, but the git commit should mention it.

> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
>  mm/frontswap.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/frontswap.c b/mm/frontswap.c
> index f2f4685..bf99c7d 100644
> --- a/mm/frontswap.c
> +++ b/mm/frontswap.c
> @@ -89,8 +89,7 @@ void __frontswap_init(unsigned type)
>  	BUG_ON(sis == NULL);
>  	if (sis->frontswap_map == NULL)
>  		return;
> -	if (frontswap_enabled)
> -		frontswap_ops.init(type);
> +	frontswap_ops.init(type);
>  }
>  EXPORT_SYMBOL(__frontswap_init);
>  
> -- 
> 1.7.8.6

[PATCH 08/11] mm: frontswap: add tracing support

[Sasha Levin]

Add tracepoints to frontswap API.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/trace/events/frontswap.h |  167 ++++++++++++++++++++++++++++++++++++++
 mm/frontswap.c                   |   14 +++
 2 files changed, 181 insertions(+), 0 deletions(-)
 create mode 100644 include/trace/events/frontswap.h

diff --git a/include/trace/events/frontswap.h b/include/trace/events/frontswap.h
new file mode 100644
index 0000000..d6c4934
--- /dev/null
+++ b/include/trace/events/frontswap.h
@@ -0,0 +1,167 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM frontswap
+
+#if !defined(_TRACE_FRONTSWAP_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_FRONTSWAP_H
+
+#include <linux/tracepoint.h>
+
+struct frontswap_ops;
+
+TRACE_EVENT(frontswap_init,
+	TP_PROTO(unsigned int type, void *sis, void *frontswap_map),
+	TP_ARGS(type, sis, frontswap_map),
+
+	TP_STRUCT__entry(
+		__field(	unsigned int,	type		)
+		__field(	void *,		sis		)
+		__field(	void *,		frontswap_map	)
+	),
+
+	TP_fast_assign(
+		__entry->type		= type;
+		__entry->sis		= sis;
+		__entry->frontswap_map	= frontswap_map;
+	),
+
+	TP_printk("type: %u sis: %p frontswap_map: %p",
+		  __entry->type, __entry->sis, __entry->frontswap_map)
+);
+
+TRACE_EVENT(frontswap_register_ops,
+	TP_PROTO(struct frontswap_ops *old, struct frontswap_ops *new),
+	TP_ARGS(old, new),
+
+	TP_STRUCT__entry(
+		__field(struct frontswap_ops *,		old		)
+		__field(struct frontswap_ops *,		new		)
+	),
+
+	TP_fast_assign(
+		__entry->old		= old;
+		__entry->new		= new;
+	),
+
+	TP_printk("old: {init=%p put_page=%p get_page=%p invalidate_page=%p invalidate_area=%p}"
+		" new: {init=%p put_page=%p get_page=%p invalidate_page=%p invalidate_area=%p}",
+		__entry->old->init,__entry->old->put_page,__entry->old->get_page,
+		__entry->old->invalidate_page,__entry->old->invalidate_area,__entry->new->init,
+		__entry->new->put_page,__entry->new->get_page,__entry->new->invalidate_page,
+		__entry->new->invalidate_area)
+);
+
+TRACE_EVENT(frontswap_put_page,
+	TP_PROTO(void *page, int dup, int ret),
+	TP_ARGS(page, dup, ret),
+
+	TP_STRUCT__entry(
+		__field(	int,		dup		)
+		__field(	int,		ret		)
+		__field(	void *,		page		)
+	),
+
+	TP_fast_assign(
+		__entry->dup		= dup;
+		__entry->ret		= ret;
+		__entry->page		= page;
+	),
+
+	TP_printk("page: %p dup: %d ret: %d",
+		  __entry->page, __entry->dup, __entry->ret)
+);
+
+TRACE_EVENT(frontswap_get_page,
+	TP_PROTO(void *page, int ret),
+	TP_ARGS(page, ret),
+
+	TP_STRUCT__entry(
+		__field(	int,		ret		)
+		__field(	void *,		page		)
+	),
+
+	TP_fast_assign(
+		__entry->ret		= ret;
+		__entry->page		= page;
+	),
+
+	TP_printk("page: %p ret: %d",
+		  __entry->page, __entry->ret)
+);
+
+TRACE_EVENT(frontswap_invalidate_page,
+	TP_PROTO(int type, unsigned long offset, void *sis, int test),
+	TP_ARGS(type, offset, sis, test),
+
+	TP_STRUCT__entry(
+		__field(	int,		type		)
+		__field(	unsigned long,	offset		)
+		__field(	void *,		sis		)
+		__field(	int,		test		)
+	),
+
+	TP_fast_assign(
+		__entry->type		= type;
+		__entry->offset		= offset;
+		__entry->sis		= sis;
+		__entry->test		= test;
+	),
+
+	TP_printk("type: %d offset: %lu sys: %p frontswap_test: %d",
+		  __entry->type, __entry->offset, __entry->sis, __entry->test)
+);
+
+TRACE_EVENT(frontswap_invalidate_area,
+	TP_PROTO(int type, void *sis, void *map),
+	TP_ARGS(type, sis, map),
+
+	TP_STRUCT__entry(
+		__field(	int,		type		)
+		__field(	void *,		map		)
+		__field(	void *,		sis		)
+	),
+
+	TP_fast_assign(
+		__entry->type		= type;
+		__entry->sis		= sis;
+		__entry->map		= map;
+	),
+
+	TP_printk("type: %d sys: %p map: %p",
+		  __entry->type, __entry->sis, __entry->map)
+);
+
+TRACE_EVENT(frontswap_curr_pages,
+	TP_PROTO(unsigned long totalpages),
+	TP_ARGS(totalpages),
+
+	TP_STRUCT__entry(
+		__field(unsigned long,		totalpages	)
+	),
+
+	TP_fast_assign(
+		__entry->totalpages	= totalpages;
+	),
+
+	TP_printk("total pages: %lu",
+		  __entry->totalpages)
+);
+
+TRACE_EVENT(frontswap_shrink,
+	TP_PROTO(unsigned long target_pages),
+	TP_ARGS(target_pages),
+
+	TP_STRUCT__entry(
+		__field(unsigned long,		target_pages	)
+	),
+
+	TP_fast_assign(
+		__entry->target_pages	= target_pages;
+	),
+
+	TP_printk("target pages: %lu",
+		  __entry->target_pages)
+);
+
+#endif /* _TRACE_FRONTSWAP_H */
+
+#include <trace/define_trace.h>
diff --git a/mm/frontswap.c b/mm/frontswap.c
index bf99c7d..b98df99 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -11,6 +11,7 @@
  * This work is licensed under the terms of the GNU GPL, version 2.
  */
 
+#define CREATE_TRACE_POINTS
 #include <linux/mm.h>
 #include <linux/mman.h>
 #include <linux/swap.h>
@@ -23,6 +24,7 @@
 #include <linux/debugfs.h>
 #include <linux/frontswap.h>
 #include <linux/swapfile.h>
+#include <trace/events/frontswap.h>
 
 /*
  * frontswap_ops is set by frontswap_register_ops to contain the pointers
@@ -66,6 +68,7 @@ struct frontswap_ops frontswap_register_ops(struct frontswap_ops *ops)
 {
 	struct frontswap_ops old = frontswap_ops;
 
+	trace_frontswap_register_ops(&old, ops);
 	frontswap_ops = *ops;
 	frontswap_enabled = 1;
 	return old;
@@ -87,6 +90,9 @@ void __frontswap_init(unsigned type)
 	struct swap_info_struct *sis = swap_info[type];
 
 	BUG_ON(sis == NULL);
+
+	trace_frontswap_init(type, sis, sis->frontswap_map);
+
 	if (sis->frontswap_map == NULL)
 		return;
 	frontswap_ops.init(type);
@@ -113,6 +119,7 @@ int __frontswap_put_page(struct page *page)
 	if (frontswap_test(sis, offset))
 		dup = 1;
 	ret = frontswap_ops.put_page(type, offset, page);
+	trace_frontswap_put_page(page, dup, ret);
 	if (ret == 0) {
 		frontswap_set(sis, offset);
 		frontswap_succ_puts++;
@@ -153,6 +160,7 @@ int __frontswap_get_page(struct page *page)
 	BUG_ON(sis == NULL);
 	if (frontswap_test(sis, offset))
 		ret = frontswap_ops.get_page(type, offset, page);
+	trace_frontswap_get_page(page, ret);
 	if (ret == 0)
 		frontswap_gets++;
 	return ret;
@@ -168,6 +176,7 @@ void __frontswap_invalidate_page(unsigned type, pgoff_t offset)
 	struct swap_info_struct *sis = swap_info[type];
 
 	BUG_ON(sis == NULL);
+	trace_frontswap_invalidate_page(type, offset, sis, frontswap_test(sis, offset));
 	if (frontswap_test(sis, offset)) {
 		frontswap_ops.invalidate_page(type, offset);
 		atomic_dec(&sis->frontswap_pages);
@@ -186,6 +195,7 @@ void __frontswap_invalidate_area(unsigned type)
 	struct swap_info_struct *sis = swap_info[type];
 
 	BUG_ON(sis == NULL);
+	trace_frontswap_invalidate_area(type, sis, sis->frontswap_map);
 	if (sis->frontswap_map == NULL)
 		return;
 	frontswap_ops.invalidate_area(type);
@@ -274,6 +284,8 @@ void frontswap_shrink(unsigned long target_pages)
 	unsigned long pages_to_unuse = 0;
 	int type, ret;
 
+	trace_frontswap_shrink(target_pages);
+
 	/*
 	 * we don't want to hold swap_lock while doing a very
 	 * lengthy try_to_unuse, but swap_list may change
@@ -301,6 +313,8 @@ unsigned long frontswap_curr_pages(void)
 	totalpages = __frontswap_curr_pages();
 	spin_unlock(&swap_lock);
 
+	trace_frontswap_curr_pages(totalpages);
+
 	return totalpages;
 }
 EXPORT_SYMBOL(frontswap_curr_pages);
-- 
1.7.8.6

Re: [PATCH 08/11] mm: frontswap: add tracing support

[Konrad Rzeszutek Wilk]

On Wed, Jun 06, 2012 at 12:55:12PM +0200, Sasha Levin wrote:
> Add tracepoints to frontswap API.
> 
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
>  include/trace/events/frontswap.h |  167 ++++++++++++++++++++++++++++++++++++++
>  mm/frontswap.c                   |   14 +++
>  2 files changed, 181 insertions(+), 0 deletions(-)
>  create mode 100644 include/trace/events/frontswap.h
> 
> diff --git a/include/trace/events/frontswap.h b/include/trace/events/frontswap.h
> new file mode 100644
> index 0000000..d6c4934
> --- /dev/null
> +++ b/include/trace/events/frontswap.h
> @@ -0,0 +1,167 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM frontswap
> +
> +#if !defined(_TRACE_FRONTSWAP_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_FRONTSWAP_H
> +
> +#include <linux/tracepoint.h>
> +
> +struct frontswap_ops;
> +
> +TRACE_EVENT(frontswap_init,
> +	TP_PROTO(unsigned int type, void *sis, void *frontswap_map),
> +	TP_ARGS(type, sis, frontswap_map),
> +
> +	TP_STRUCT__entry(
> +		__field(	unsigned int,	type		)
> +		__field(	void *,		sis		)
> +		__field(	void *,		frontswap_map	)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->type		= type;
> +		__entry->sis		= sis;
> +		__entry->frontswap_map	= frontswap_map;
> +	),
> +
> +	TP_printk("type: %u sis: %p frontswap_map: %p",
> +		  __entry->type, __entry->sis, __entry->frontswap_map)
> +);
> +
> +TRACE_EVENT(frontswap_register_ops,
> +	TP_PROTO(struct frontswap_ops *old, struct frontswap_ops *new),
> +	TP_ARGS(old, new),
> +
> +	TP_STRUCT__entry(
> +		__field(struct frontswap_ops *,		old		)
> +		__field(struct frontswap_ops *,		new		)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->old		= old;
> +		__entry->new		= new;
> +	),
> +
> +	TP_printk("old: {init=%p put_page=%p get_page=%p invalidate_page=%p invalidate_area=%p}"
> +		" new: {init=%p put_page=%p get_page=%p invalidate_page=%p invalidate_area=%p}",
> +		__entry->old->init,__entry->old->put_page,__entry->old->get_page,
> +		__entry->old->invalidate_page,__entry->old->invalidate_area,__entry->new->init,
> +		__entry->new->put_page,__entry->new->get_page,__entry->new->invalidate_page,

s/get_page/load
s/put_page/store

> +		__entry->new->invalidate_area)
> +);
> +
> +TRACE_EVENT(frontswap_put_page,

Its not called put_page anymore.


> +	TP_PROTO(void *page, int dup, int ret),
> +	TP_ARGS(page, dup, ret),
> +
> +	TP_STRUCT__entry(
> +		__field(	int,		dup		)
> +		__field(	int,		ret		)
> +		__field(	void *,		page		)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->dup		= dup;
> +		__entry->ret		= ret;
> +		__entry->page		= page;
> +	),
> +
> +	TP_printk("page: %p dup: %d ret: %d",
> +		  __entry->page, __entry->dup, __entry->ret)
> +);
> +
> +TRACE_EVENT(frontswap_get_page,

Ditto
> +	TP_PROTO(void *page, int ret),
> +	TP_ARGS(page, ret),
> +
> +	TP_STRUCT__entry(
> +		__field(	int,		ret		)
> +		__field(	void *,		page		)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->ret		= ret;
> +		__entry->page		= page;
> +	),
> +
> +	TP_printk("page: %p ret: %d",
> +		  __entry->page, __entry->ret)
> +);
> +
> +TRACE_EVENT(frontswap_invalidate_page,
> +	TP_PROTO(int type, unsigned long offset, void *sis, int test),
> +	TP_ARGS(type, offset, sis, test),
> +
> +	TP_STRUCT__entry(
> +		__field(	int,		type		)
> +		__field(	unsigned long,	offset		)
> +		__field(	void *,		sis		)
> +		__field(	int,		test		)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->type		= type;
> +		__entry->offset		= offset;
> +		__entry->sis		= sis;
> +		__entry->test		= test;
> +	),
> +
> +	TP_printk("type: %d offset: %lu sys: %p frontswap_test: %d",
> +		  __entry->type, __entry->offset, __entry->sis, __entry->test)
> +);
> +
> +TRACE_EVENT(frontswap_invalidate_area,

Ditto.

> +	TP_PROTO(int type, void *sis, void *map),
> +	TP_ARGS(type, sis, map),
> +
> +	TP_STRUCT__entry(
> +		__field(	int,		type		)
> +		__field(	void *,		map		)
> +		__field(	void *,		sis		)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->type		= type;
> +		__entry->sis		= sis;
> +		__entry->map		= map;
> +	),
> +
> +	TP_printk("type: %d sys: %p map: %p",
> +		  __entry->type, __entry->sis, __entry->map)
> +);
> +
> +TRACE_EVENT(frontswap_curr_pages,
> +	TP_PROTO(unsigned long totalpages),
> +	TP_ARGS(totalpages),
> +
> +	TP_STRUCT__entry(
> +		__field(unsigned long,		totalpages	)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->totalpages	= totalpages;
> +	),
> +
> +	TP_printk("total pages: %lu",
> +		  __entry->totalpages)
> +);
> +
> +TRACE_EVENT(frontswap_shrink,
> +	TP_PROTO(unsigned long target_pages),
> +	TP_ARGS(target_pages),
> +
> +	TP_STRUCT__entry(
> +		__field(unsigned long,		target_pages	)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->target_pages	= target_pages;
> +	),
> +
> +	TP_printk("target pages: %lu",
> +		  __entry->target_pages)
> +);
> +
> +#endif /* _TRACE_FRONTSWAP_H */
> +
> +#include <trace/define_trace.h>
> diff --git a/mm/frontswap.c b/mm/frontswap.c
> index bf99c7d..b98df99 100644
> --- a/mm/frontswap.c
> +++ b/mm/frontswap.c
> @@ -11,6 +11,7 @@
>   * This work is licensed under the terms of the GNU GPL, version 2.
>   */
>  
> +#define CREATE_TRACE_POINTS
>  #include <linux/mm.h>
>  #include <linux/mman.h>
>  #include <linux/swap.h>
> @@ -23,6 +24,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/frontswap.h>
>  #include <linux/swapfile.h>
> +#include <trace/events/frontswap.h>
>  
>  /*
>   * frontswap_ops is set by frontswap_register_ops to contain the pointers
> @@ -66,6 +68,7 @@ struct frontswap_ops frontswap_register_ops(struct frontswap_ops *ops)
>  {
>  	struct frontswap_ops old = frontswap_ops;
>  
> +	trace_frontswap_register_ops(&old, ops);
>  	frontswap_ops = *ops;
>  	frontswap_enabled = 1;
>  	return old;
> @@ -87,6 +90,9 @@ void __frontswap_init(unsigned type)
>  	struct swap_info_struct *sis = swap_info[type];
>  
>  	BUG_ON(sis == NULL);
> +
> +	trace_frontswap_init(type, sis, sis->frontswap_map);
> +
>  	if (sis->frontswap_map == NULL)
>  		return;
>  	frontswap_ops.init(type);
> @@ -113,6 +119,7 @@ int __frontswap_put_page(struct page *page)
>  	if (frontswap_test(sis, offset))
>  		dup = 1;
>  	ret = frontswap_ops.put_page(type, offset, page);
> +	trace_frontswap_put_page(page, dup, ret);

Uh, why a different parameter layout?

>  	if (ret == 0) {
>  		frontswap_set(sis, offset);
>  		frontswap_succ_puts++;
> @@ -153,6 +160,7 @@ int __frontswap_get_page(struct page *page)
>  	BUG_ON(sis == NULL);
>  	if (frontswap_test(sis, offset))
>  		ret = frontswap_ops.get_page(type, offset, page);
> +	trace_frontswap_get_page(page, ret);
>  	if (ret == 0)
>  		frontswap_gets++;
>  	return ret;
> @@ -168,6 +176,7 @@ void __frontswap_invalidate_page(unsigned type, pgoff_t offset)
>  	struct swap_info_struct *sis = swap_info[type];
>  
>  	BUG_ON(sis == NULL);
> +	trace_frontswap_invalidate_page(type, offset, sis, frontswap_test(sis, offset));
>  	if (frontswap_test(sis, offset)) {
>  		frontswap_ops.invalidate_page(type, offset);
>  		atomic_dec(&sis->frontswap_pages);
> @@ -186,6 +195,7 @@ void __frontswap_invalidate_area(unsigned type)
>  	struct swap_info_struct *sis = swap_info[type];
>  
>  	BUG_ON(sis == NULL);
> +	trace_frontswap_invalidate_area(type, sis, sis->frontswap_map);
>  	if (sis->frontswap_map == NULL)
>  		return;
>  	frontswap_ops.invalidate_area(type);
> @@ -274,6 +284,8 @@ void frontswap_shrink(unsigned long target_pages)
>  	unsigned long pages_to_unuse = 0;
>  	int type, ret;
>  
> +	trace_frontswap_shrink(target_pages);
> +
>  	/*
>  	 * we don't want to hold swap_lock while doing a very
>  	 * lengthy try_to_unuse, but swap_list may change
> @@ -301,6 +313,8 @@ unsigned long frontswap_curr_pages(void)
>  	totalpages = __frontswap_curr_pages();
>  	spin_unlock(&swap_lock);
>  
> +	trace_frontswap_curr_pages(totalpages);
> +
>  	return totalpages;
>  }
>  EXPORT_SYMBOL(frontswap_curr_pages);
> -- 
> 1.7.8.6

[PATCH 09/11] mm: frontswap: remove unused variable in init

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 mm/frontswap.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index b98df99..c0cd8bc 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -321,8 +321,6 @@ EXPORT_SYMBOL(frontswap_curr_pages);
 
 static int __init init_frontswap(void)
 {
-	int err = 0;
-
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *root = debugfs_create_dir("frontswap", NULL);
 	if (root == NULL)
@@ -334,7 +332,7 @@ static int __init init_frontswap(void)
 	debugfs_create_u64("invalidates", S_IRUGO,
 				root, &frontswap_invalidates);
 #endif
-	return err;
+	return 0;
 }
 
 module_init(init_frontswap);
-- 
1.7.8.6

[PATCH 10/11] mm: frontswap: split out function to clear a page out

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 mm/frontswap.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index c0cd8bc..92e2e7b 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -99,6 +99,12 @@ void __frontswap_init(unsigned type)
 }
 EXPORT_SYMBOL(__frontswap_init);
 
+static inline void __frontswap_clear(struct swap_info_struct *sis, pgoff_t offset)
+{
+	frontswap_clear(sis, offset);
+	atomic_dec(&sis->frontswap_pages);
+}
+
 /*
  * "Put" data from a page to frontswap and associate it with the page's
  * swaptype and offset.  Page must be locked and in the swap cache.
@@ -131,10 +137,8 @@ int __frontswap_put_page(struct page *page)
 		  the (older) page from frontswap
 		 */
 		frontswap_failed_puts++;
-		if (dup) {
-			frontswap_clear(sis, offset);
-			atomic_dec(&sis->frontswap_pages);
-		}
+		if (dup)
+			__frontswap_clear(sis, offset);
 	}
 	if (frontswap_writethrough_enabled)
 		/* report failure so swap also writes to swap device */
@@ -179,8 +183,7 @@ void __frontswap_invalidate_page(unsigned type, pgoff_t offset)
 	trace_frontswap_invalidate_page(type, offset, sis, frontswap_test(sis, offset));
 	if (frontswap_test(sis, offset)) {
 		frontswap_ops.invalidate_page(type, offset);
-		atomic_dec(&sis->frontswap_pages);
-		frontswap_clear(sis, offset);
+		__frontswap_clear(sis, offset);
 		frontswap_invalidates++;
 	}
 }
-- 
1.7.8.6

[PATCH 11/11] mm: frontswap: remove unneeded headers

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 mm/frontswap.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 92e2e7b..2a5d421 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -12,15 +12,11 @@
  */
 
 #define CREATE_TRACE_POINTS
-#include <linux/mm.h>
 #include <linux/mman.h>
 #include <linux/swap.h>
 #include <linux/swapops.h>
-#include <linux/proc_fs.h>
 #include <linux/security.h>
-#include <linux/capability.h>
 #include <linux/module.h>
-#include <linux/uaccess.h>
 #include <linux/debugfs.h>
 #include <linux/frontswap.h>
 #include <linux/swapfile.h>
-- 
1.7.8.6

Re: [PATCH 01/11] mm: frontswap: remove casting from function calls through ops structure

[Konrad Rzeszutek Wilk]

On Wed, Jun 06, 2012 at 12:55:05PM +0200, Sasha Levin wrote:
> Removes unneeded casts.

On the cover letter can you do a git diff --stat linus/master.. so
that at a quick glance I can figure out how much we are shaving off
the code?

Thanks!