* [Patch 3.11.7 1/1]mm: remove and free expired data in time in zswap @ 2013-11-08 9:50 changkun.li 2013-11-08 10:28 ` Bob Liu 0 siblings, 1 reply; 4+ messages in thread From: changkun.li @ 2013-11-08 9:50 UTC (permalink / raw) To: sjenning, linux-mm; +Cc: luyi, lichangkun, linux-kernel 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<xfishcoder@gmail.com> --- mm/zswap.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index cbd9578..90a2813 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -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; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Patch 3.11.7 1/1]mm: remove and free expired data in time in zswap 2013-11-08 9:50 [Patch 3.11.7 1/1]mm: remove and free expired data in time in zswap changkun.li @ 2013-11-08 10:28 ` Bob Liu 2013-11-18 7:06 ` Weijie Yang 0 siblings, 1 reply; 4+ messages in thread From: Bob Liu @ 2013-11-08 10:28 UTC (permalink / raw) To: changkun.li; +Cc: sjenning, linux-mm, luyi, lichangkun, linux-kernel On 11/08/2013 05:50 PM, changkun.li wrote: > 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<xfishcoder@gmail.com> > --- > mm/zswap.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index cbd9578..90a2813 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -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); 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. -- Regards, -Bob ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch 3.11.7 1/1]mm: remove and free expired data in time in zswap 2013-11-08 10:28 ` Bob Liu @ 2013-11-18 7:06 ` Weijie Yang 2014-01-10 9:43 ` Weijie Yang 0 siblings, 1 reply; 4+ messages in thread From: Weijie Yang @ 2013-11-18 7:06 UTC (permalink / raw) To: Bob Liu Cc: changkun.li, sjenning, linux-mm, luyi, lichangkun, linux-kernel, Seth Jennings add cc Seth 's new email address. On Fri, Nov 8, 2013 at 6:28 PM, Bob Liu <bob.liu@oracle.com> wrote: > On 11/08/2013 05:50 PM, changkun.li wrote: >> 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<xfishcoder@gmail.com> >> --- >> mm/zswap.c | 5 ++++- >> 1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/mm/zswap.c b/mm/zswap.c >> index cbd9578..90a2813 100644 >> --- a/mm/zswap.c >> +++ b/mm/zswap.c >> @@ -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); > > 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. Yes, it is a bug. But I agree with Bob, this patch is not efficient. How about like this? diff --git a/mm/frontswap.c b/mm/frontswap.c index 1b24bdc..1227896 --- a/mm/frontswap.c +++ b/mm/frontswap.c @@ -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 */ > -- > Regards, > -Bob > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Patch 3.11.7 1/1]mm: remove and free expired data in time in zswap 2013-11-18 7:06 ` Weijie Yang @ 2014-01-10 9:43 ` Weijie Yang 0 siblings, 0 replies; 4+ messages in thread From: Weijie Yang @ 2014-01-10 9:43 UTC (permalink / raw) To: Seth Jennings Cc: changkun.li, Linux-MM, luyi, lichangkun, Linux-Kernel, Bob Liu, Seth Jennings Hi, Seth Would you please check this issue? Thanks On Mon, Nov 18, 2013 at 3:06 PM, Weijie Yang <weijie.yang.kh@gmail.com> wrote: > add cc Seth 's new email address. > > On Fri, Nov 8, 2013 at 6:28 PM, Bob Liu <bob.liu@oracle.com> wrote: >> On 11/08/2013 05:50 PM, changkun.li wrote: >>> 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<xfishcoder@gmail.com> >>> --- >>> mm/zswap.c | 5 ++++- >>> 1 files changed, 4 insertions(+), 1 deletions(-) >>> >>> diff --git a/mm/zswap.c b/mm/zswap.c >>> index cbd9578..90a2813 100644 >>> --- a/mm/zswap.c >>> +++ b/mm/zswap.c >>> @@ -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); >> >> 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. > > Yes, it is a bug. > > But I agree with Bob, this patch is not efficient. > > How about like this? > > diff --git a/mm/frontswap.c b/mm/frontswap.c > index 1b24bdc..1227896 > --- a/mm/frontswap.c > +++ b/mm/frontswap.c > @@ -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 */ > > >> -- >> Regards, >> -Bob >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-10 9:48 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-08 9:50 [Patch 3.11.7 1/1]mm: remove and free expired data in time in zswap changkun.li 2013-11-08 10:28 ` Bob Liu 2013-11-18 7:06 ` Weijie Yang 2014-01-10 9:43 ` Weijie Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox