Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1530486 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 3/3] z3fold: discourage use of pages that weren't compacted Dan Streetman <ddstreet@ieee.org> - 2016-11-25 19:30 +0100
    Re: [PATCH 3/3] z3fold: discourage use of pages that weren't compacted Vitaly Wool <vitalywool@gmail.com> - 2016-11-28 15:20 +0100
      Re: [PATCH 3/3] z3fold: discourage use of pages that weren't compacted Dan Streetman <ddstreet@ieee.org> - 2016-11-29 23:30 +0100

#1530486 — 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@gmail.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@gmail.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

[toc] | [next] | [standalone]

#1531339

On Fri, Nov 25, 2016 at 7:25 PM, Dan Streetman <ddstreet@ieee.org> wrote:
> On Tue, Nov 15, 2016 at 11:00 AM, Vitaly Wool <vitalywool@gmail.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?

When the next attempt to compact this page comes, it's less likely
it's locked so the wait times are slightly lower in average.

~vitaly

>> Signed-off-by: Vitaly Wool <vitalywool@gmail.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

[toc] | [prev] | [next] | [standalone]

#1532820

On Mon, Nov 28, 2016 at 9:14 AM, Vitaly Wool <vitalywool@gmail.com> wrote:
> On Fri, Nov 25, 2016 at 7:25 PM, Dan Streetman <ddstreet@ieee.org> wrote:
>> On Tue, Nov 15, 2016 at 11:00 AM, Vitaly Wool <vitalywool@gmail.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?
>
> When the next attempt to compact this page comes, it's less likely
> it's locked so the wait times are slightly lower in average.

which wait time?  for a page with the middle chunk mapped compact
should exit immediately...?

>
> ~vitaly
>
>>> Signed-off-by: Vitaly Wool <vitalywool@gmail.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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web