linux-kernel - Re: [PATCH 3/3] z3fold: discourage use of pages that weren't compacted

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 25 Nov 2016 13:25:55 -0500
From:   Dan Streetman <ddstreet@...e.org>
To:     Vitaly Wool <vitalywool@...il.com>
Cc:     Linux-MM <linux-mm@...ck.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 3/3] z3fold: discourage use of pages that weren't compacted

On Tue, Nov 15, 2016 at 11:00 AM, Vitaly Wool <vitalywool@...il.com> wrote:
> If a z3fold page couldn't be compacted, we don't want it to be
> used for next object allocation in the first place.

why?  !compacted can only mean 1) already compact or 2) middle chunks
is mapped.  #1 is as good compaction-wise as the page can get, so do
you mean that if a page couldn't be compacted because of #2, we
shouldn't use it for next allocation?  if so, that isn't quite what
this patch does.

> It makes more
> sense to add it to the end of the relevant unbuddied list. If that
> page gets compacted later, it will be added to the beginning of
> the list then.
>
> This simple idea gives 5-7% improvement in randrw fio tests and
> about 10% improvement in fio sequential read/write.

i don't understand why there is any improvement - the unbuddied lists
are grouped by the amount of free chunks, so all pages in a specific
unbuddied list should have exactly that number of free chunks
available, and it shouldn't matter if a page gets put into the front
or back...where is the performance improvement coming from?

>
> Signed-off-by: Vitaly Wool <vitalywool@...il.com>
> ---
>  mm/z3fold.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/mm/z3fold.c b/mm/z3fold.c
> index ffd9353..e282ba0 100644
> --- a/mm/z3fold.c
> +++ b/mm/z3fold.c
> @@ -539,11 +539,19 @@ static void z3fold_free(struct z3fold_pool *pool, unsigned long handle)
>                 free_z3fold_page(zhdr);
>                 atomic64_dec(&pool->pages_nr);
>         } else {
> -               z3fold_compact_page(zhdr);
> +               int compacted = z3fold_compact_page(zhdr);
>                 /* Add to the unbuddied list */
>                 spin_lock(&pool->lock);
>                 freechunks = num_free_chunks(zhdr);
> -               list_add(&zhdr->buddy, &pool->unbuddied[freechunks]);
> +               /*
> +                * If the page has been compacted, we want to use it
> +                * in the first place.
> +                */
> +               if (compacted)
> +                       list_add(&zhdr->buddy, &pool->unbuddied[freechunks]);
> +               else
> +                       list_add_tail(&zhdr->buddy,
> +                                     &pool->unbuddied[freechunks]);
>                 spin_unlock(&pool->lock);
>                 z3fold_page_unlock(zhdr);
>         }
> @@ -672,12 +680,16 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries)
>                                 spin_lock(&pool->lock);
>                                 list_add(&zhdr->buddy, &pool->buddied);
>                         } else {
> -                               z3fold_compact_page(zhdr);
> +                               int compacted = z3fold_compact_page(zhdr);
>                                 /* add to unbuddied list */
>                                 spin_lock(&pool->lock);
>                                 freechunks = num_free_chunks(zhdr);
> -                               list_add(&zhdr->buddy,
> -                                        &pool->unbuddied[freechunks]);
> +                               if (compacted)
> +                                       list_add(&zhdr->buddy,
> +                                               &pool->unbuddied[freechunks]);
> +                               else
> +                                       list_add_tail(&zhdr->buddy,
> +                                               &pool->unbuddied[freechunks]);
>                         }
>                 }
>
> --
> 2.4.2