2012-06-08 19:14:40

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 00/10] minor frontswap cleanups and tracing support

Most of these patches are minor cleanups to the mm/frontswap.c code, the big
chunk of new code can be attributed to the new tracing support.


Changes in v2:
- Rebase to current version
- Address Konrad's comments

Sasha Levin (10):
mm: frontswap: remove casting from function calls through ops
structure
mm: frontswap: trivial coding convention issues
mm: frontswap: split out __frontswap_curr_pages
mm: frontswap: split out __frontswap_unuse_pages
mm: frontswap: split frontswap_shrink further to simplify locking
mm: frontswap: make all branches of if statement in put page
consistent
mm: frontswap: remove unnecessary check during initialization
mm: frontswap: add tracing support
mm: frontswap: split out function to clear a page out
mm: frontswap: remove unneeded headers

include/trace/events/frontswap.h | 167 ++++++++++++++++++++++++++++++++++++++
mm/frontswap.c | 162 +++++++++++++++++++++++-------------
2 files changed, 270 insertions(+), 59 deletions(-)
create mode 100644 include/trace/events/frontswap.h

--
1.7.8.6


2012-06-08 19:14:42

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 01/10] mm: frontswap: remove casting from function calls through ops structure

Removes unneeded casts.

Signed-off-by: Sasha Levin <[email protected]>
---
mm/frontswap.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index e250255..557e8af4 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -111,7 +111,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);

@@ -134,7 +134,7 @@ int __frontswap_store(struct page *page)
BUG_ON(sis == NULL);
if (frontswap_test(sis, offset))
dup = 1;
- ret = (*frontswap_ops.store)(type, offset, page);
+ ret = frontswap_ops.store(type, offset, page);
if (ret == 0) {
frontswap_set(sis, offset);
inc_frontswap_succ_stores();
@@ -173,7 +173,7 @@ int __frontswap_load(struct page *page)
BUG_ON(!PageLocked(page));
BUG_ON(sis == NULL);
if (frontswap_test(sis, offset))
- ret = (*frontswap_ops.load)(type, offset, page);
+ ret = frontswap_ops.load(type, offset, page);
if (ret == 0)
inc_frontswap_loads();
return ret;
@@ -190,7 +190,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);
inc_frontswap_invalidates();
@@ -209,7 +209,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

2012-06-08 19:14:47

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 02/10] mm: frontswap: trivial coding convention issues

Signed-off-by: Sasha Levin <[email protected]>
---
mm/frontswap.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 557e8af4..b619d29 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -150,6 +150,7 @@ int __frontswap_store(struct page *page)
inc_frontswap_failed_stores();
} else
inc_frontswap_failed_stores();
+ }
if (frontswap_writethrough_enabled)
/* report failure so swap also writes to swap device */
ret = -1;
@@ -250,9 +251,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

2012-06-08 19:14:56

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 04/10] mm: frontswap: split out __frontswap_unuse_pages

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 <[email protected]>
---
mm/frontswap.c | 59 +++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 37277e6..5d9ed36 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -230,6 +230,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
@@ -240,11 +275,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;

/*
@@ -258,22 +291,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

2012-06-08 19:15:06

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 07/10] mm: frontswap: remove unnecessary check during initialization

The check whether frontswap is enabled or not is done in the API functions in
the frontswap header, before they are passed to the internal
double-underscored frontswap functions.

Remove the check from __frontswap_init for consistency.

Signed-off-by: Sasha Levin <[email protected]>
---
mm/frontswap.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index ee1763d..0319fc5 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -110,8 +110,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

2012-06-08 19:15:15

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 10/10] mm: frontswap: remove unneeded headers

Signed-off-by: Sasha Levin <[email protected]>
---
mm/frontswap.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 0930c39..b25aae5 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

2012-06-08 19:15:11

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 08/10] mm: frontswap: add tracing support

Add tracepoints to frontswap API.

Signed-off-by: Sasha Levin <[email protected]>
---
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..2e5efab
--- /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 store=%p load=%p invalidate_page=%p invalidate_area=%p}"
+ " new: {init=%p store=%p load=%p invalidate_page=%p invalidate_area=%p}",
+ __entry->old->init,__entry->old->store,__entry->old->load,
+ __entry->old->invalidate_page,__entry->old->invalidate_area,__entry->new->init,
+ __entry->new->store,__entry->new->load,__entry->new->invalidate_page,
+ __entry->new->invalidate_area)
+);
+
+TRACE_EVENT(frontswap_store,
+ 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_load,
+ 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 0319fc5..0250c20 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
@@ -85,6 +87,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 = true;
return old;
@@ -108,6 +111,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);
@@ -134,6 +140,7 @@ int __frontswap_store(struct page *page)
if (frontswap_test(sis, offset))
dup = 1;
ret = frontswap_ops.store(type, offset, page);
+ trace_frontswap_store(page, dup, ret);
if (ret == 0) {
frontswap_set(sis, offset);
inc_frontswap_succ_stores();
@@ -174,6 +181,7 @@ int __frontswap_load(struct page *page)
BUG_ON(sis == NULL);
if (frontswap_test(sis, offset))
ret = frontswap_ops.load(type, offset, page);
+ trace_frontswap_load(page, ret);
if (ret == 0)
inc_frontswap_loads();
return ret;
@@ -189,6 +197,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);
@@ -207,6 +216,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);
@@ -295,6 +305,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
@@ -322,6 +334,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

2012-06-08 19:16:00

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 09/10] mm: frontswap: split out function to clear a page out

Signed-off-by: Sasha Levin <[email protected]>
---
mm/frontswap.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 0250c20..0930c39 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -120,6 +120,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);
+}
+
/*
* "Store" 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.
@@ -152,10 +158,8 @@ int __frontswap_store(struct page *page)
the (older) page from frontswap
*/
inc_frontswap_failed_stores();
- 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 */
@@ -200,8 +204,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);
inc_frontswap_invalidates();
}
}
--
1.7.8.6

2012-06-08 19:16:20

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 06/10] mm: frontswap: make all branches of if statement in put page consistent

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 <[email protected]>
---
mm/frontswap.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 1f1af0e..ee1763d 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -140,16 +140,16 @@ int __frontswap_store(struct page *page)
inc_frontswap_succ_stores();
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);
- inc_frontswap_failed_stores();
- } else
inc_frontswap_failed_stores();
+ 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

2012-06-08 19:14:53

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 03/10] mm: frontswap: split out __frontswap_curr_pages

Code was duplicated in two functions, clean it up.

Signed-off-by: Sasha Levin <[email protected]>
---
mm/frontswap.c | 28 +++++++++++++++++-----------
1 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index b619d29..37277e6 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -216,6 +216,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
@@ -240,11 +254,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;
@@ -282,16 +292,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

2012-06-08 19:16:47

by Sasha Levin

[permalink] [raw]
Subject: [PATCH v2 05/10] mm: frontswap: split frontswap_shrink further to simplify locking

Split frontswap_shrink to simplify the locking in the original code.

Signed-off-by: Sasha Levin <[email protected]>
---
mm/frontswap.c | 36 +++++++++++++++++++++---------------
1 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 5d9ed36..1f1af0e 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -265,6 +265,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
@@ -275,10 +293,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
@@ -286,20 +302,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

2012-06-08 20:16:41

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH v2 02/10] mm: frontswap: trivial coding convention issues

On Fri, Jun 8, 2012 at 10:15 PM, Sasha Levin <[email protected]> wrote:
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> ?mm/frontswap.c | ? ?5 +++--
> ?1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/frontswap.c b/mm/frontswap.c
> index 557e8af4..b619d29 100644
> --- a/mm/frontswap.c
> +++ b/mm/frontswap.c
> @@ -150,6 +150,7 @@ int __frontswap_store(struct page *page)
> ? ? ? ? ? ? ? ?inc_frontswap_failed_stores();
> ? ? ? ?} else
> ? ? ? ? ? ? ? ?inc_frontswap_failed_stores();
> + ? ? ? }

This looks wrong. Did you compile it?

2012-06-08 20:36:41

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH v2 02/10] mm: frontswap: trivial coding convention issues

On Fri, Jun 8, 2012 at 10:16 PM, Pekka Enberg <[email protected]> wrote:
> On Fri, Jun 8, 2012 at 10:15 PM, Sasha Levin <[email protected]> wrote:
>> Signed-off-by: Sasha Levin <[email protected]>
>> ---
>> ?mm/frontswap.c | ? ?5 +++--
>> ?1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/frontswap.c b/mm/frontswap.c
>> index 557e8af4..b619d29 100644
>> --- a/mm/frontswap.c
>> +++ b/mm/frontswap.c
>> @@ -150,6 +150,7 @@ int __frontswap_store(struct page *page)
>> ? ? ? ? ? ? ? ?inc_frontswap_failed_stores();
>> ? ? ? ?} else
>> ? ? ? ? ? ? ? ?inc_frontswap_failed_stores();
>> + ? ? ? }
>
> This looks wrong. Did you compile it?

gah... yes, I've had an error in the merge resolution, but since that
code is completely changed in next couple of patches I didn't notice
it when building the final version. I really should build after each
merge failure...

Konrad, would it be ok if I send just a new version of this patch (if
everything else is fine)?

2012-06-08 22:04:32

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] minor frontswap cleanups and tracing support

On Fri, Jun 08, 2012 at 09:15:09PM +0200, Sasha Levin wrote:
> Most of these patches are minor cleanups to the mm/frontswap.c code, the big
> chunk of new code can be attributed to the new tracing support.
>
>
> Changes in v2:
> - Rebase to current version
> - Address Konrad's comments

There was one comment that I am not sure if it was emailed and that
was about adding the "lockdep_assert_held(&swap_lock);".

You added that in two patches, while the git commit only talks about
"move that code" . Please remove it out of the "move the code" patches
and add it as a seperate git commit with an explanation of why it
is added.

Otherwise (well, the compile issue that was spotted) the patches
look great. Could you repost them with those two fixes please?

>
> Sasha Levin (10):
> mm: frontswap: remove casting from function calls through ops
> structure
> mm: frontswap: trivial coding convention issues
> mm: frontswap: split out __frontswap_curr_pages
> mm: frontswap: split out __frontswap_unuse_pages
> mm: frontswap: split frontswap_shrink further to simplify locking
> mm: frontswap: make all branches of if statement in put page
> consistent
> mm: frontswap: remove unnecessary check during initialization
> mm: frontswap: add tracing support
> mm: frontswap: split out function to clear a page out
> mm: frontswap: remove unneeded headers
>
> include/trace/events/frontswap.h | 167 ++++++++++++++++++++++++++++++++++++++
> mm/frontswap.c | 162 +++++++++++++++++++++++-------------
> 2 files changed, 270 insertions(+), 59 deletions(-)
> create mode 100644 include/trace/events/frontswap.h
>
> --
> 1.7.8.6
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>

2012-06-08 22:36:54

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] minor frontswap cleanups and tracing support

On Fri, 2012-06-08 at 18:04 -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Jun 08, 2012 at 09:15:09PM +0200, Sasha Levin wrote:
> > Most of these patches are minor cleanups to the mm/frontswap.c code, the big
> > chunk of new code can be attributed to the new tracing support.
> >
> >
> > Changes in v2:
> > - Rebase to current version
> > - Address Konrad's comments
>
> There was one comment that I am not sure if it was emailed and that
> was about adding the "lockdep_assert_held(&swap_lock);".
>
> You added that in two patches, while the git commit only talks about
> "move that code" . Please remove it out of the "move the code" patches
> and add it as a seperate git commit with an explanation of why it
> is added.

argh, I forgot to comment on that as well. Sorry.

> Otherwise (well, the compile issue that was spotted) the patches
> look great. Could you repost them with those two fixes please?

Will do.