All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cody P Schafer <cody@linux.vnet.ibm.com>
To: Dave Hansen <dave@sr71.net>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, cl@linux.com
Subject: Re: [RFC][PATCH] mm: percpu pages: up batch size to fix arithmetic?? errror
Date: Wed, 11 Sep 2013 16:21:46 -0700	[thread overview]
Message-ID: <5230FB0A.70901@linux.vnet.ibm.com> (raw)
In-Reply-To: <5230F7DD.90905@linux.vnet.ibm.com>

On 09/11/2013 04:08 PM, Cody P Schafer wrote:
> On 09/11/2013 03:08 PM, Dave Hansen wrote:
>> I really don't know where the:
>>
>>     batch /= 4;             /* We effectively *= 4 below */
>>     ...
>>     batch = rounddown_pow_of_two(batch + batch/2) - 1;
>>
>> came from.  The round down code at *MOST* does a *= 1.5, but
>> *averages* out to be just under 1.
>>
>> On a system with 128GB in a zone, this means that we've got
>> (you can see in /proc/zoneinfo for yourself):
>>
>>                high:  186 (744kB)
>>                batch: 31  (124kB)
>>
>> That 124kB is almost precisely 1/4 of the "1/2 of a meg" that we
>> were shooting for.  We're under-sizing the batches by about 4x.
>> This patch kills the /=4.
>>
>> ---
>> diff -puN mm/page_alloc.c~debug-pcp-sizes-1 mm/page_alloc.c
>> --- linux.git/mm/page_alloc.c~debug-pcp-sizes-1    2013-09-11
>> 14:41:08.532445664 -0700
>> +++ linux.git-davehans/mm/page_alloc.c    2013-09-11
>> 15:03:47.403912683 -0700
>> @@ -4103,7 +4103,6 @@ static int __meminit zone_batchsize(stru
>>       batch = zone->managed_pages / 1024;
>>       if (batch * PAGE_SIZE > 512 * 1024)
>>           batch = (512 * 1024) / PAGE_SIZE;
>> -    batch /= 4;        /* We effectively *= 4 below */
>>       if (batch < 1)
>>           batch = 1;
>>
>> _
>>
>
> Looking back at the first git commit (way before my time), it appears
> that the percpu pagesets initially had a ->high and ->low (now removed),
> set to batch*6 and batch*2 respectively. I assume the idea was to keep
> the number of pages in the percpu pagesets around batch*4, hence the
> comment.
>
> So we have this variable called "batch", and the code is trying to store
> the _average_ number of pcp pages we want into it (not the batchsize),
> and then we divide our "average" goal by 4 to get a batchsize. All the
> comments refer to the size of the pcp pagesets, not to the pcp pageset
> batchsize.
>
> Looking further, in current code we don't refill the pcp pagesets unless
> they are completely empty (->low was removed a while ago), and then we
> only add ->batch pages.
>
> Has anyone looked at what type of average pcp sizing the current code
> results in?

Also, we may want to consider shrinking pcp->high down from 6*pcp->batch 
given that the original "6*" choice was based upon ->batch actually 
being 1/4th of the average pageset size, where now it appears closer to 
being the average.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Cody P Schafer <cody@linux.vnet.ibm.com>
To: Dave Hansen <dave@sr71.net>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, cl@linux.com
Subject: Re: [RFC][PATCH] mm: percpu pages: up batch size to fix arithmetic?? errror
Date: Wed, 11 Sep 2013 16:21:46 -0700	[thread overview]
Message-ID: <5230FB0A.70901@linux.vnet.ibm.com> (raw)
In-Reply-To: <5230F7DD.90905@linux.vnet.ibm.com>

On 09/11/2013 04:08 PM, Cody P Schafer wrote:
> On 09/11/2013 03:08 PM, Dave Hansen wrote:
>> I really don't know where the:
>>
>>     batch /= 4;             /* We effectively *= 4 below */
>>     ...
>>     batch = rounddown_pow_of_two(batch + batch/2) - 1;
>>
>> came from.  The round down code at *MOST* does a *= 1.5, but
>> *averages* out to be just under 1.
>>
>> On a system with 128GB in a zone, this means that we've got
>> (you can see in /proc/zoneinfo for yourself):
>>
>>                high:  186 (744kB)
>>                batch: 31  (124kB)
>>
>> That 124kB is almost precisely 1/4 of the "1/2 of a meg" that we
>> were shooting for.  We're under-sizing the batches by about 4x.
>> This patch kills the /=4.
>>
>> ---
>> diff -puN mm/page_alloc.c~debug-pcp-sizes-1 mm/page_alloc.c
>> --- linux.git/mm/page_alloc.c~debug-pcp-sizes-1    2013-09-11
>> 14:41:08.532445664 -0700
>> +++ linux.git-davehans/mm/page_alloc.c    2013-09-11
>> 15:03:47.403912683 -0700
>> @@ -4103,7 +4103,6 @@ static int __meminit zone_batchsize(stru
>>       batch = zone->managed_pages / 1024;
>>       if (batch * PAGE_SIZE > 512 * 1024)
>>           batch = (512 * 1024) / PAGE_SIZE;
>> -    batch /= 4;        /* We effectively *= 4 below */
>>       if (batch < 1)
>>           batch = 1;
>>
>> _
>>
>
> Looking back at the first git commit (way before my time), it appears
> that the percpu pagesets initially had a ->high and ->low (now removed),
> set to batch*6 and batch*2 respectively. I assume the idea was to keep
> the number of pages in the percpu pagesets around batch*4, hence the
> comment.
>
> So we have this variable called "batch", and the code is trying to store
> the _average_ number of pcp pages we want into it (not the batchsize),
> and then we divide our "average" goal by 4 to get a batchsize. All the
> comments refer to the size of the pcp pagesets, not to the pcp pageset
> batchsize.
>
> Looking further, in current code we don't refill the pcp pagesets unless
> they are completely empty (->low was removed a while ago), and then we
> only add ->batch pages.
>
> Has anyone looked at what type of average pcp sizing the current code
> results in?

Also, we may want to consider shrinking pcp->high down from 6*pcp->batch 
given that the original "6*" choice was based upon ->batch actually 
being 1/4th of the average pageset size, where now it appears closer to 
being the average.


  reply	other threads:[~2013-09-11 23:21 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-11 22:08 [RFC][PATCH] mm: percpu pages: up batch size to fix arithmetic?? errror Dave Hansen
2013-09-11 22:08 ` Dave Hansen
2013-09-11 23:08 ` Cody P Schafer
2013-09-11 23:08   ` Cody P Schafer
2013-09-11 23:21   ` Cody P Schafer [this message]
2013-09-11 23:21     ` Cody P Schafer
2013-09-12  0:20     ` Dave Hansen
2013-09-12  0:20       ` Dave Hansen
2013-09-12 14:16       ` Christoph Lameter
2013-09-12 14:16         ` Christoph Lameter
2013-09-12 15:21         ` Dave Hansen
2013-09-12 15:21           ` Dave Hansen
2013-09-11 23:58   ` Dave Hansen
2013-09-11 23:58     ` Dave Hansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5230FB0A.70901@linux.vnet.ibm.com \
    --to=cody@linux.vnet.ibm.com \
    --cc=cl@linux.com \
    --cc=dave@sr71.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.