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 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
Signed-off-by: Sasha Levin <[email protected]>
---
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
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 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
Split frontswap_shrink to eliminate the locking issues 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 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
Signed-off-by: Sasha Levin <[email protected]>
---
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
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 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
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 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
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 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
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..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
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 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
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 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
On Wed, 2012-06-06 at 07:30 -0400, Konrad Rzeszutek Wilk wrote:
>
> On Jun 6, 2012 6:55 AM, "Sasha Levin" <[email protected]> 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.
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" <[email protected]> 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"?
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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
On Wed, Jun 06, 2012 at 12:55:12PM +0200, Sasha Levin wrote:
> 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..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
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!
> 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 <[email protected]>
> > ---
> > 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!