mm: remove and free expired data in time in zswap
Subsystems (2)
PATCH 1/1 mm: remove and free expired data in time in zswap
In zswap, store page A to zbud if the compression ratio is high, insert its entry into rbtree. if there is a entry B which has the same offset in the rbtree.Remove and free B before insert the entry of A.
case:
if the compression ratio of page A is not high, return without checking the same offset one in rbtree.
if there is a entry B which has the same offset in the rbtree. Now, we make sure B is invalid or expired. But the entry and compressed memory of B are not freed in time.
Because zswap spaces data in memory, it makes the utilization of memory lower. the other valid data in zbud is writeback to swap device more possibility, when zswap is full.
So if we make sure a entry is expired, free it in time.
--- mm/zswap.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
mm/zswap.c +1 -0
@@ -596,6 +596,7 @@ fail:return ret; } +static void zswap_frontswap_invalidate_page(unsigned type, pgoff_t
offset);
/*********************************
* frontswap hooks
**********************************/
@@ -614,7 +615,7 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
if (!tree) {
ret = -ENODEV;
- goto reject;
+ goto nodev;
}
/* reclaim space if needed */
@@ -695,6 +696,8 @@ freepage:
put_cpu_var(zswap_dstmem);
zswap_entry_cache_free(entry);
reject:
+ zswap_frontswap_invalidate_page(type, offset);
+nodev:
return ret;
}
Signature · 1 line
1.7.1
Quoted 47 lines · changkun.li
In zswap, store page A to zbud if the compression ratio is high, insert its entry into rbtree. if there is a entry B which has the same offset in the rbtree.Remove and free B before insert the entry of A.
case:
if the compression ratio of page A is not high, return without checking the same offset one in rbtree.if there is a entry B which has the same offset in the rbtree. Now, we make sure B is invalid or expired. But the entry and compressed memory of B are not freed in time.
Because zswap spaces data in memory, it makes the utilization of memory lower. the other valid data in zbud is writeback to swap device more possibility, when zswap is full.
So if we make sure a entry is expired, free it in time.
Signed-off-by changkun.li<[email protected]>--- mm/zswap.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)mm/zswap.c +1
-0@@ -596,6 +596,7 @@ fail:return ret; } +static void zswap_frontswap_invalidate_page (unsigned type,pgoff_t offset);/********************************* * frontswap hooks **********************************/
mm/zswap.c +1
-1@@ -614,7 +615,7 @@ static int zswap_frontswap_store(unsigned type,pgoff_t offset, if (!tree) { ret = -ENODEV ; - goto reject; + goto nodev; }mm/zswap.c +0
-0/* reclaim space if needed */
mm/zswap.c freepage: · line 699
Quoted 4 lines
mm/zswap.c +1
-0@@ -695,6 +696,8 @@ freepage:put_cpu_var (zswap_dstmem);zswap_entry_cache_free (entry); reject: +zswap_frontswap_invalidate_page (type, offset);
I'm afraid when arrives here zswap_rb_search(offset) will always return NULL entry. So most of the time, it's just waste time to call zswap_frontswap_invalidate_page() to search rbtree.
Signature · 8 lines
Regards, -Bob -- 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>
I agree with you, I see the __frontswap_store in 'frontswap.c', it knows the same offset entry exist in zswap. we could modify like this?:
int __frontswap_store(struct page *page)
{
int ret = -1, dup = 0;
swp_entry_t entry = { .val = page_private(page), };
int type = swp_type(entry);
struct swap_info_struct *sis = swap_info[type];
pgoff_t offset = swp_offset(entry);
/*
* Return if no backend registed.
* Don't need to inc frontswap_failed_stores here.
*/
if (!frontswap_ops)
return ret;
BUG_ON(!PageLocked(page));
BUG_ON(sis == NULL);
if (__frontswap_test(sis, offset))
dup = 1;
ret = frontswap_ops->store(type, offset, page);
if (ret == 0) {
set_bit(offset, sis->frontswap_map);
inc_frontswap_succ_stores();
if (!dup)
atomic_inc(&sis->frontswap_pages);
} else {
/*
failed dup always results in automatic invalidate of
the (older) page from frontswap
*/
inc_frontswap_failed_stores();
++ if (dup) {
-- if (dup)
__frontswap_clear(sis, offset);
++ __frontswap_invalidate_page(type, offset);
++ }
}
if (frontswap_writethrough_enabled)
/* report failure so swap also writes to swap device */
ret = -1;
return ret;
}but maybe the other frontswap modules is not space memory, they need not call __frontswap_invalidate_page. so the code in here is only better for zswap.
Quoted 61 lines · Bob Liu
Quoted 47 lines · changkun.li
In zswap, store page A to zbud if the compression ratio is high, insert its entry into rbtree. if there is a entry B which has the same offset in the rbtree.Remove and free B before insert the entry of A.
case:
if the compression ratio of page A is not high, return without checking the same offset one in rbtree.if there is a entry B which has the same offset in the rbtree. Now, we make sure B is invalid or expired. But the entry and compressed memory of B are not freed in time.
Because zswap spaces data in memory, it makes the utilization of memory lower. the other valid data in zbud is writeback to swap device more possibility, when zswap is full.
So if we make sure a entry is expired, free it in time.
Signed-off-by changkun.li<[email protected]>--- mm/zswap.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)mm/zswap.c +1
-0@@ -596,6 +596,7 @@ fail:return ret; } +static void zswap_frontswap_invalidate_page (unsigned type,pgoff_t offset);/********************************* * frontswap hooks **********************************/
mm/zswap.c +1
-1@@ -614,7 +615,7 @@ static int zswap_frontswap_store(unsigned type,pgoff_t offset, if (!tree) { ret = -ENODEV ; - goto reject; + goto nodev; }/* reclaim space if needed */mm/zswap.c freepage: · line 699
Quoted 4 lines
mm/zswap.c +1
-0@@ -695,6 +696,8 @@ freepage:put_cpu_var (zswap_dstmem);zswap_entry_cache_free (entry); reject: +zswap_frontswap_invalidate_page (type, offset);I'm afraid when arrives here
zswap_rb_search(offset)will always return NULL entry. So most of the time, it's just waste time to callzswap_frontswap_invalidate_page()to search rbtree.--
Regards,
-Bob
add cc Seth 's new email address.
Quoted 46 lines · Bob Liu
Quoted 45 lines · changkun.li
In zswap, store page A to zbud if the compression ratio is high, insert its entry into rbtree. if there is a entry B which has the same offset in the rbtree.Remove and free B before insert the entry of A.
case:
if the compression ratio of page A is not high, return without checking the same offset one in rbtree.if there is a entry B which has the same offset in the rbtree. Now, we make sure B is invalid or expired. But the entry and compressed memory of B are not freed in time.
Because zswap spaces data in memory, it makes the utilization of memory lower. the other valid data in zbud is writeback to swap device more possibility, when zswap is full.
So if we make sure a entry is expired, free it in time.
Signed-off-by changkun.li<[email protected]>--- mm/zswap.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)mm/zswap.c +1
-0@@ -596,6 +596,7 @@ fail:return ret; } +static void zswap_frontswap_invalidate_page (unsigned type,pgoff_t offset);/********************************* * frontswap hooks **********************************/
mm/zswap.c +1
-1@@ -614,7 +615,7 @@ static int zswap_frontswap_store(unsigned type,pgoff_t offset, if (!tree) { ret = -ENODEV ; - goto reject; + goto nodev; }
Quoted 10 lines
mm/zswap.c freepage: · line 699
Quoted 6 lines
/* reclaim space if needed */mm/zswap.c +1
-0@@ -695,6 +696,8 @@ freepage:put_cpu_var (zswap_dstmem);zswap_entry_cache_free (entry); reject: +zswap_frontswap_invalidate_page (type, offset);I'm afraid when arrives here
zswap_rb_search(offset)will always return NULL entry. So most of the time, it's just waste time to callzswap_frontswap_invalidate_page()to search rbtree.
Yes, it is a bug.
But I agree with Bob, this patch is not efficient.
How about like this?
mm/frontswap.c +3 -1
@@ -244,8 +244,10 @@ int __frontswap_store(struct page *page)the (older) page from frontswap */ inc_frontswap_failed_stores (); -if (dup) +if (dup) { + frontswap_ops->invalidate_page (type, offset);__frontswap_clear (sis, offset); + } }if (frontswap_writethrough_enabled)/* report failure so swap also writes to swap device */
Quoted 8 lines
--
Regards,
-Bob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
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>
Hi, Seth
Would you please check this issue? Thanks
Quoted 84 lines · Weijie Yang
add cc Seth 's new email address.
Quoted 46 lines · Bob Liu
Quoted 45 lines · changkun.li
In zswap, store page A to zbud if the compression ratio is high, insert its entry into rbtree. if there is a entry B which has the same offset in the rbtree.Remove and free B before insert the entry of A.
case:
if the compression ratio of page A is not high, return without checking the same offset one in rbtree.if there is a entry B which has the same offset in the rbtree. Now, we make sure B is invalid or expired. But the entry and compressed memory of B are not freed in time.
Because zswap spaces data in memory, it makes the utilization of memory lower. the other valid data in zbud is writeback to swap device more possibility, when zswap is full.
So if we make sure a entry is expired, free it in time.
Signed-off-by changkun.li<[email protected]>--- mm/zswap.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)mm/zswap.c +1
-0@@ -596,6 +596,7 @@ fail:return ret; } +static void zswap_frontswap_invalidate_page (unsigned type,pgoff_t offset);/********************************* * frontswap hooks **********************************/
mm/zswap.c +1
-1@@ -614,7 +615,7 @@ static int zswap_frontswap_store(unsigned type,pgoff_t offset, if (!tree) { ret = -ENODEV ; - goto reject; + goto nodev; }Quoted 10 lines
mm/zswap.c freepage: · line 699
Quoted 6 lines
/* reclaim space if needed */mm/zswap.c +1
-0@@ -695,6 +696,8 @@ freepage:put_cpu_var (zswap_dstmem);zswap_entry_cache_free (entry); reject: +zswap_frontswap_invalidate_page (type, offset);I'm afraid when arrives here
zswap_rb_search(offset)will always return NULL entry. So most of the time, it's just waste time to callzswap_frontswap_invalidate_page()to search rbtree.Yes, it is a bug.
But I agree with Bob, this patch is not efficient.
How about like this?
mm/frontswap.c +3
-1@@ -244,8 +244,10 @@ int __frontswap_store(struct page *page)the (older) page from frontswap */ inc_frontswap_failed_stores (); -if (dup) +if (dup) { + frontswap_ops->invalidate_page (type, offset);__frontswap_clear (sis, offset); + } }if (frontswap_writethrough_enabled)/* report failure so swap also writes to swap device */
Quoted 8 lines
Quoted 8 lines
--
Regards,
-Bob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
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>