Re: [Patch 3.11.7 1/1]mm: remove and free expired data in time in zswap

["changkun.li" <xfishcoder@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3594 bytes --]

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.



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.
>
> --
> Regards,
> -Bob
>

[-- Attachment #2: Type: text/html, Size: 4663 bytes --]