[PATCH] mm, vmacache: Add kconfig VMACACHE_SHIFT

[PATCH] mm, vmacache: Add kconfig VMACACHE_SHIFT

[WANG Chao]

Add a new kconfig option VMACACHE_SHIFT (as a power of 2) to specify the
number of slots vma cache has for each thread. Range is chosen 0-4 (1-16
slots) to consider both overhead and performance penalty. Default is 2
(4 slots) as it originally is, which provides good enough balance.

Signed-off-by: WANG Chao <chaowang@redhat.com>
---
 include/linux/sched.h | 2 +-
 mm/Kconfig            | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 8db31ef..56fd96d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -134,7 +134,7 @@ struct perf_event_context;
 struct blk_plug;
 struct filename;
 
-#define VMACACHE_BITS 2
+#define VMACACHE_BITS CONFIG_VMACACHE_SHIFT
 #define VMACACHE_SIZE (1U << VMACACHE_BITS)
 #define VMACACHE_MASK (VMACACHE_SIZE - 1)
 
diff --git a/mm/Kconfig b/mm/Kconfig
index 1d1ae6b..7b82a52 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -618,3 +618,10 @@ config MAX_STACK_SIZE_MB
 	  changed to a smaller value in which case that is used.
 
 	  A sane initial value is 80 MB.
+
+config VMACACHE_SHIFT
+	int "Number of slots in per-thread VMA cache (as a power of 2)"
+	range 0 4
+	default 2
+	help
+	  This is the number of slots VMA cache has for each thread.
-- 
2.1.0

Re: [PATCH] mm, vmacache: Add kconfig VMACACHE_SHIFT

[Davidlohr Bueso]

On Thu, 2015-01-22 at 14:29 +0800, WANG Chao wrote:
> Add a new kconfig option VMACACHE_SHIFT (as a power of 2) to specify the
> number of slots vma cache has for each thread. Range is chosen 0-4 (1-16
> slots) to consider both overhead and performance penalty. Default is 2
> (4 slots) as it originally is, which provides good enough balance.
> 

Nack. I don't feel comfortable making scalability features of core code
configurable.

Thanks,
Davidlohr

Re: [PATCH] mm, vmacache: Add kconfig VMACACHE_SHIFT

[WANG Chao]

Hi, Davidlohr

On 01/21/15 at 11:46pm, Davidlohr Bueso wrote:
> On Thu, 2015-01-22 at 14:29 +0800, WANG Chao wrote:
> > Add a new kconfig option VMACACHE_SHIFT (as a power of 2) to specify the
> > number of slots vma cache has for each thread. Range is chosen 0-4 (1-16
> > slots) to consider both overhead and performance penalty. Default is 2
> > (4 slots) as it originally is, which provides good enough balance.
> > 
> 
> Nack. I don't feel comfortable making scalability features of core code
> configurable.

Out of respect, is this a general rule not making scalability features
of core code configurable?

Thanks
WANG Chao

Re: [PATCH] mm, vmacache: Add kconfig VMACACHE_SHIFT

[Vlastimil Babka]

On 01/22/2015 07:29 AM, WANG Chao wrote:
> Add a new kconfig option VMACACHE_SHIFT (as a power of 2) to specify the
> number of slots vma cache has for each thread. Range is chosen 0-4 (1-16
> slots) to consider both overhead and performance penalty. Default is 2

One could say that overhead and performance penalty is the same thing. 
Please elaborate?

Also, got any performance numbers to share for workloads benefiting from 
more/less than the default?

> (4 slots) as it originally is, which provides good enough balance.
>
> Signed-off-by: WANG Chao <chaowang@redhat.com>
> ---
>   include/linux/sched.h | 2 +-
>   mm/Kconfig            | 7 +++++++
>   2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 8db31ef..56fd96d 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -134,7 +134,7 @@ struct perf_event_context;
>   struct blk_plug;
>   struct filename;
>
> -#define VMACACHE_BITS 2
> +#define VMACACHE_BITS CONFIG_VMACACHE_SHIFT
>   #define VMACACHE_SIZE (1U << VMACACHE_BITS)
>   #define VMACACHE_MASK (VMACACHE_SIZE - 1)
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 1d1ae6b..7b82a52 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -618,3 +618,10 @@ config MAX_STACK_SIZE_MB
>   	  changed to a smaller value in which case that is used.
>
>   	  A sane initial value is 80 MB.
> +
> +config VMACACHE_SHIFT
> +	int "Number of slots in per-thread VMA cache (as a power of 2)"
> +	range 0 4
> +	default 2
> +	help
> +	  This is the number of slots VMA cache has for each thread.

As a user, I wouldn't find this informative enough to make the decision.

Re: [PATCH] mm, vmacache: Add kconfig VMACACHE_SHIFT

[Davidlohr Bueso]

On Thu, 2015-01-22 at 15:57 +0800, WANG Chao wrote:
> Hi, Davidlohr
> 
> On 01/21/15 at 11:46pm, Davidlohr Bueso wrote:
> > On Thu, 2015-01-22 at 14:29 +0800, WANG Chao wrote:
> > > Add a new kconfig option VMACACHE_SHIFT (as a power of 2) to specify the
> > > number of slots vma cache has for each thread. Range is chosen 0-4 (1-16
> > > slots) to consider both overhead and performance penalty. Default is 2
> > > (4 slots) as it originally is, which provides good enough balance.
> > > 
> > 
> > Nack. I don't feel comfortable making scalability features of core code
> > configurable.
> 
> Out of respect, is this a general rule not making scalability features
> of core code configurable?

I doubt its a rule, just common sense. Users have no business
configuring such low level details. The optimizations need to
transparently work for everyone.

Thanks,
Davidlohr

Re: [PATCH] mm, vmacache: Add kconfig VMACACHE_SHIFT

[Rik van Riel]

On 01/22/2015 11:19 AM, Davidlohr Bueso wrote:
> On Thu, 2015-01-22 at 15:57 +0800, WANG Chao wrote:
>> Hi, Davidlohr
>>
>> On 01/21/15 at 11:46pm, Davidlohr Bueso wrote:
>>> On Thu, 2015-01-22 at 14:29 +0800, WANG Chao wrote:
>>>> Add a new kconfig option VMACACHE_SHIFT (as a power of 2) to specify the
>>>> number of slots vma cache has for each thread. Range is chosen 0-4 (1-16
>>>> slots) to consider both overhead and performance penalty. Default is 2
>>>> (4 slots) as it originally is, which provides good enough balance.
>>>>
>>>
>>> Nack. I don't feel comfortable making scalability features of core code
>>> configurable.
>>
>> Out of respect, is this a general rule not making scalability features
>> of core code configurable?
> 
> I doubt its a rule, just common sense. Users have no business
> configuring such low level details. The optimizations need to
> transparently work for everyone.

There may sometimes be a good reason for making this kind of
thing configurable, but since there were no performance
numbers in the changelog, I have not seen any such reason for
this particular change :)

Re: [PATCH] mm, vmacache: Add kconfig VMACACHE_SHIFT

[WANG Chao]

On 01/22/15 at 11:22am, Rik van Riel wrote:
> On 01/22/2015 11:19 AM, Davidlohr Bueso wrote:
> > On Thu, 2015-01-22 at 15:57 +0800, WANG Chao wrote:
> >> Hi, Davidlohr
> >>
> >> On 01/21/15 at 11:46pm, Davidlohr Bueso wrote:
> >>> On Thu, 2015-01-22 at 14:29 +0800, WANG Chao wrote:
> >>>> Add a new kconfig option VMACACHE_SHIFT (as a power of 2) to specify the
> >>>> number of slots vma cache has for each thread. Range is chosen 0-4 (1-16
> >>>> slots) to consider both overhead and performance penalty. Default is 2
> >>>> (4 slots) as it originally is, which provides good enough balance.
> >>>>
> >>>
> >>> Nack. I don't feel comfortable making scalability features of core code
> >>> configurable.
> >>
> >> Out of respect, is this a general rule not making scalability features
> >> of core code configurable?
> > 
> > I doubt its a rule, just common sense. Users have no business
> > configuring such low level details. The optimizations need to
> > transparently work for everyone.
> 
> There may sometimes be a good reason for making this kind of
> thing configurable, but since there were no performance
> numbers in the changelog, I have not seen any such reason for
> this particular change :)

True. I didn't run any kind of benchmark, thus no numbers here. This is
purely hypothetical.

I'm glad to run some tests. For the sake of consistency, could you
please show me a hint how do you measure at the first place? I can do
hit-rate, but I don't know how you measure cpu cycles. Could you
elaborate?

Thanks
WANG Chao

Re: [PATCH] mm, vmacache: Add kconfig VMACACHE_SHIFT

[Sasha Levin]

On 01/23/2015 12:14 AM, WANG Chao wrote:
> On 01/22/15 at 11:22am, Rik van Riel wrote:
>> > On 01/22/2015 11:19 AM, Davidlohr Bueso wrote:
>>> > > On Thu, 2015-01-22 at 15:57 +0800, WANG Chao wrote:
>>>> > >> Hi, Davidlohr
>>>> > >>
>>>> > >> On 01/21/15 at 11:46pm, Davidlohr Bueso wrote:
>>>>> > >>> On Thu, 2015-01-22 at 14:29 +0800, WANG Chao wrote:
>>>>>> > >>>> Add a new kconfig option VMACACHE_SHIFT (as a power of 2) to specify the
>>>>>> > >>>> number of slots vma cache has for each thread. Range is chosen 0-4 (1-16
>>>>>> > >>>> slots) to consider both overhead and performance penalty. Default is 2
>>>>>> > >>>> (4 slots) as it originally is, which provides good enough balance.
>>>>>> > >>>>
>>>>> > >>>
>>>>> > >>> Nack. I don't feel comfortable making scalability features of core code
>>>>> > >>> configurable.
>>>> > >>
>>>> > >> Out of respect, is this a general rule not making scalability features
>>>> > >> of core code configurable?
>>> > > 
>>> > > I doubt its a rule, just common sense. Users have no business
>>> > > configuring such low level details. The optimizations need to
>>> > > transparently work for everyone.
>> > 
>> > There may sometimes be a good reason for making this kind of
>> > thing configurable, but since there were no performance
>> > numbers in the changelog, I have not seen any such reason for
>> > this particular change :)
> True. I didn't run any kind of benchmark, thus no numbers here. This is
> purely hypothetical.
> 
> I'm glad to run some tests. For the sake of consistency, could you
> please show me a hint how do you measure at the first place? I can do
> hit-rate, but I don't know how you measure cpu cycles. Could you
> elaborate?

I don't think there's a need to look for problems where there are none.

Have you observed a performance issue that might be improved by changing
the shift here?


Thanks,
Sasha