2020-10-22 17:18:53

by Vijayanand Jitta

[permalink] [raw]
Subject: [PATCH] lib: stackdepot: Add support to configure STACK_HASH_SIZE

From: Yogesh Lal <[email protected]>

Use STACK_HASH_ORDER_SHIFT to configure STACK_HASH_SIZE.

Aim is to have configurable value for STACK_HASH_SIZE,
so depend on use case one can configure it.

One example is of Page Owner, default value of
STACK_HASH_SIZE lead stack depot to consume 8MB of static memory.
Making it configurable and use lower value helps to enable features like
CONFIG_PAGE_OWNER without any significant overhead.

Signed-off-by: Yogesh Lal <[email protected]>
Signed-off-by: Vinayak Menon <[email protected]>
Signed-off-by: Vijayanand Jitta <[email protected]>
---
lib/Kconfig | 9 +++++++++
lib/stackdepot.c | 3 +--
2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index 18d76b6..b3f8259 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -651,6 +651,15 @@ config STACKDEPOT
bool
select STACKTRACE

+config STACK_HASH_ORDER_SHIFT
+ int "stack depot hash size (12 => 4KB, 20 => 1024KB)"
+ range 12 20
+ default 20
+ depends on STACKDEPOT
+ help
+ Select the hash size as a power of 2 for the stackdepot hash table.
+ Choose a lower value to reduce the memory impact.
+
config SBITMAP
bool

diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index 2caffc6..413c20b 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -142,8 +142,7 @@ static struct stack_record *depot_alloc_stack(unsigned long *entries, int size,
return stack;
}

-#define STACK_HASH_ORDER 20
-#define STACK_HASH_SIZE (1L << STACK_HASH_ORDER)
+#define STACK_HASH_SIZE (1L << CONFIG_STACK_HASH_ORDER_SHIFT)
#define STACK_HASH_MASK (STACK_HASH_SIZE - 1)
#define STACK_HASH_SEED 0x9747b28c

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
2.7.4


2020-11-03 23:32:49

by Minchan Kim

[permalink] [raw]
Subject: Re: [PATCH] lib: stackdepot: Add support to configure STACK_HASH_SIZE

Sorry if this mail corrupts the mail thread or had heavy mangling
since I lost this mail from my mailbox so I am sending this mail by
web gmail.

On Thu, Oct 22, 2020 at 10:18 AM <[email protected]> wrote:
>
> From: Yogesh Lal <[email protected]>
>
> Use STACK_HASH_ORDER_SHIFT to configure STACK_HASH_SIZE.
>
> Aim is to have configurable value for STACK_HASH_SIZE,
> so depend on use case one can configure it.
>
> One example is of Page Owner, default value of
> STACK_HASH_SIZE lead stack depot to consume 8MB of static memory.
> Making it configurable and use lower value helps to enable features like
> CONFIG_PAGE_OWNER without any significant overhead.
>
> Signed-off-by: Yogesh Lal <[email protected]>
> Signed-off-by: Vinayak Menon <[email protected]>
> Signed-off-by: Vijayanand Jitta <[email protected]>
> ---
> lib/Kconfig | 9 +++++++++
> lib/stackdepot.c | 3 +--
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 18d76b6..b3f8259 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -651,6 +651,15 @@ config STACKDEPOT
> bool
> select STACKTRACE
>
> +config STACK_HASH_ORDER_SHIFT
> + int "stack depot hash size (12 => 4KB, 20 => 1024KB)"
> + range 12 20
> + default 20
> + depends on STACKDEPOT
> + help
> + Select the hash size as a power of 2 for the stackdepot hash table.
> + Choose a lower value to reduce the memory impact.
> +
> config SBITMAP
> bool
>
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index 2caffc6..413c20b 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -142,8 +142,7 @@ static struct stack_record *depot_alloc_stack(unsigned long *entries, int size,
> return stack;
> }
>
> -#define STACK_HASH_ORDER 20
> -#define STACK_HASH_SIZE (1L << STACK_HASH_ORDER)
> +#define STACK_HASH_SIZE (1L << CONFIG_STACK_HASH_ORDER_SHIFT)
> #define STACK_HASH_MASK (STACK_HASH_SIZE - 1)
> #define STACK_HASH_SEED 0x9747b28c
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> 2.7.4
>

1. When we don't use page_owner, we don't want to waste any memory for
stackdepot hash array.
2. When we use page_owner, we want to have reasonable stackdeport hash array

With this configuration, it couldn't meet since we always need to
reserve a reasonable size for the array.
Can't we make the hash size as a kernel parameter?
With it, we could use it like this.

1. page_owner=off, stackdepot_stack_hash=0 -> no more wasted memory
when we don't use page_owner
2. page_owner=on, stackdepot_stack_hash=8M -> reasonable hash size
when we use page_owner.


--
Kind regards,
Minchan Kim

2020-11-04 10:34:04

by Vijayanand Jitta

[permalink] [raw]
Subject: Re: [PATCH] lib: stackdepot: Add support to configure STACK_HASH_SIZE



On 11/4/2020 4:57 AM, Minchan Kim wrote:
> Sorry if this mail corrupts the mail thread or had heavy mangling
> since I lost this mail from my mailbox so I am sending this mail by
> web gmail.
>
> On Thu, Oct 22, 2020 at 10:18 AM <[email protected]> wrote:
>>
>> From: Yogesh Lal <[email protected]>
>>
>> Use STACK_HASH_ORDER_SHIFT to configure STACK_HASH_SIZE.
>>
>> Aim is to have configurable value for STACK_HASH_SIZE,
>> so depend on use case one can configure it.
>>
>> One example is of Page Owner, default value of
>> STACK_HASH_SIZE lead stack depot to consume 8MB of static memory.
>> Making it configurable and use lower value helps to enable features like
>> CONFIG_PAGE_OWNER without any significant overhead.
>>
>> Signed-off-by: Yogesh Lal <[email protected]>
>> Signed-off-by: Vinayak Menon <[email protected]>
>> Signed-off-by: Vijayanand Jitta <[email protected]>
>> ---
>> lib/Kconfig | 9 +++++++++
>> lib/stackdepot.c | 3 +--
>> 2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/Kconfig b/lib/Kconfig
>> index 18d76b6..b3f8259 100644
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -651,6 +651,15 @@ config STACKDEPOT
>> bool
>> select STACKTRACE
>>
>> +config STACK_HASH_ORDER_SHIFT
>> + int "stack depot hash size (12 => 4KB, 20 => 1024KB)"
>> + range 12 20
>> + default 20
>> + depends on STACKDEPOT
>> + help
>> + Select the hash size as a power of 2 for the stackdepot hash table.
>> + Choose a lower value to reduce the memory impact.
>> +
>> config SBITMAP
>> bool
>>
>> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
>> index 2caffc6..413c20b 100644
>> --- a/lib/stackdepot.c
>> +++ b/lib/stackdepot.c
>> @@ -142,8 +142,7 @@ static struct stack_record *depot_alloc_stack(unsigned long *entries, int size,
>> return stack;
>> }
>>
>> -#define STACK_HASH_ORDER 20
>> -#define STACK_HASH_SIZE (1L << STACK_HASH_ORDER)
>> +#define STACK_HASH_SIZE (1L << CONFIG_STACK_HASH_ORDER_SHIFT)
>> #define STACK_HASH_MASK (STACK_HASH_SIZE - 1)
>> #define STACK_HASH_SEED 0x9747b28c
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>> 2.7.4
>>
>
> 1. When we don't use page_owner, we don't want to waste any memory for
> stackdepot hash array.
> 2. When we use page_owner, we want to have reasonable stackdeport hash array
>
> With this configuration, it couldn't meet since we always need to
> reserve a reasonable size for the array.
> Can't we make the hash size as a kernel parameter?
> With it, we could use it like this.
>
> 1. page_owner=off, stackdepot_stack_hash=0 -> no more wasted memory
> when we don't use page_owner
> 2. page_owner=on, stackdepot_stack_hash=8M -> reasonable hash size
> when we use page_owner.
>
>

This idea looks fine to me. Andrew and others would like to hear your
comments as well on this before implementing.

Thanks,
Vijay
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation

2020-11-12 13:01:02

by Vijayanand Jitta

[permalink] [raw]
Subject: Re: [PATCH] lib: stackdepot: Add support to configure STACK_HASH_SIZE



On 11/4/2020 3:59 PM, Vijayanand Jitta wrote:
>
>
> On 11/4/2020 4:57 AM, Minchan Kim wrote:
>> Sorry if this mail corrupts the mail thread or had heavy mangling
>> since I lost this mail from my mailbox so I am sending this mail by
>> web gmail.
>>
>> On Thu, Oct 22, 2020 at 10:18 AM <[email protected]> wrote:
>>>
>>> From: Yogesh Lal <[email protected]>
>>>
>>> Use STACK_HASH_ORDER_SHIFT to configure STACK_HASH_SIZE.
>>>
>>> Aim is to have configurable value for STACK_HASH_SIZE,
>>> so depend on use case one can configure it.
>>>
>>> One example is of Page Owner, default value of
>>> STACK_HASH_SIZE lead stack depot to consume 8MB of static memory.
>>> Making it configurable and use lower value helps to enable features like
>>> CONFIG_PAGE_OWNER without any significant overhead.
>>>
>>> Signed-off-by: Yogesh Lal <[email protected]>
>>> Signed-off-by: Vinayak Menon <[email protected]>
>>> Signed-off-by: Vijayanand Jitta <[email protected]>
>>> ---
>>> lib/Kconfig | 9 +++++++++
>>> lib/stackdepot.c | 3 +--
>>> 2 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/Kconfig b/lib/Kconfig
>>> index 18d76b6..b3f8259 100644
>>> --- a/lib/Kconfig
>>> +++ b/lib/Kconfig
>>> @@ -651,6 +651,15 @@ config STACKDEPOT
>>> bool
>>> select STACKTRACE
>>>
>>> +config STACK_HASH_ORDER_SHIFT
>>> + int "stack depot hash size (12 => 4KB, 20 => 1024KB)"
>>> + range 12 20
>>> + default 20
>>> + depends on STACKDEPOT
>>> + help
>>> + Select the hash size as a power of 2 for the stackdepot hash table.
>>> + Choose a lower value to reduce the memory impact.
>>> +
>>> config SBITMAP
>>> bool
>>>
>>> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
>>> index 2caffc6..413c20b 100644
>>> --- a/lib/stackdepot.c
>>> +++ b/lib/stackdepot.c
>>> @@ -142,8 +142,7 @@ static struct stack_record *depot_alloc_stack(unsigned long *entries, int size,
>>> return stack;
>>> }
>>>
>>> -#define STACK_HASH_ORDER 20
>>> -#define STACK_HASH_SIZE (1L << STACK_HASH_ORDER)
>>> +#define STACK_HASH_SIZE (1L << CONFIG_STACK_HASH_ORDER_SHIFT)
>>> #define STACK_HASH_MASK (STACK_HASH_SIZE - 1)
>>> #define STACK_HASH_SEED 0x9747b28c
>>>
>>> --
>>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>> 2.7.4
>>>
>>
>> 1. When we don't use page_owner, we don't want to waste any memory for
>> stackdepot hash array.
>> 2. When we use page_owner, we want to have reasonable stackdeport hash array
>>
>> With this configuration, it couldn't meet since we always need to
>> reserve a reasonable size for the array.
>> Can't we make the hash size as a kernel parameter?
>> With it, we could use it like this.
>>
>> 1. page_owner=off, stackdepot_stack_hash=0 -> no more wasted memory
>> when we don't use page_owner
>> 2. page_owner=on, stackdepot_stack_hash=8M -> reasonable hash size
>> when we use page_owner.
>>
>>
>
> This idea looks fine to me. Andrew and others would like to hear your
> comments as well on this before implementing.
>
> Thanks,
> Vijay
>

Awaiting for comments from Andrew and others.

Thanks,
Vijay
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation

2020-11-12 22:59:34

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] lib: stackdepot: Add support to configure STACK_HASH_SIZE

On Thu, 12 Nov 2020 18:26:24 +0530 Vijayanand Jitta <[email protected]> wrote:

> >> 1. page_owner=off, stackdepot_stack_hash=0 -> no more wasted memory
> >> when we don't use page_owner
> >> 2. page_owner=on, stackdepot_stack_hash=8M -> reasonable hash size
> >> when we use page_owner.
> >>
> >>
> >
> > This idea looks fine to me. Andrew and others would like to hear your
> > comments as well on this before implementing.
> >
> > Thanks,
> > Vijay
> >
>
> Awaiting for comments from Andrew and others.

I don't actually understand the problem.

What is it about page-owner that causes stackdepot to consume
additional memory? As far as I can tell, sizeof(struct stack_record)
isn't affected by page-owner?

2020-11-17 20:46:12

by Minchan Kim

[permalink] [raw]
Subject: Re: [PATCH] lib: stackdepot: Add support to configure STACK_HASH_SIZE

On Thu, Nov 12, 2020 at 02:56:49PM -0800, Andrew Morton wrote:
> On Thu, 12 Nov 2020 18:26:24 +0530 Vijayanand Jitta <[email protected]> wrote:
>
> > >> 1. page_owner=off, stackdepot_stack_hash=0 -> no more wasted memory
> > >> when we don't use page_owner
> > >> 2. page_owner=on, stackdepot_stack_hash=8M -> reasonable hash size
> > >> when we use page_owner.
> > >>
> > >>
> > >
> > > This idea looks fine to me. Andrew and others would like to hear your
> > > comments as well on this before implementing.
> > >
> > > Thanks,
> > > Vijay
> > >
> >
> > Awaiting for comments from Andrew and others.
>
> I don't actually understand the problem.
>
> What is it about page-owner that causes stackdepot to consume
> additional memory? As far as I can tell, sizeof(struct stack_record)
> isn't affected by page-owner?
>

Thing is once we build stackdepot due to the dependency from page_owner,
it will consume 8M regardless of using page_owner.

#define STACK_HASH_SIZE (1L << CONFIG_STACK_HASH_ORDER_SHIFT)

static struct stack_record *stack_table[STACK_HASH_SIZE] = {
[0 ... STACK_HASH_SIZE - 1] = NULL
};

So if we decide the size option at build time, we should consume
the memory anyway regardless of page_owner enabling in runtime.

2020-11-19 03:37:25

by Zhenhua Huang

[permalink] [raw]
Subject: Re: [PATCH] lib: stackdepot: Add support to configure STACK_HASH_SIZE

On Wed, Nov 04, 2020 at 07:27:03AM +0800, Minchan Kim wrote:
> Sorry if this mail corrupts the mail thread or had heavy mangling
> since I lost this mail from my mailbox so I am sending this mail by
> web gmail.
>
> On Thu, Oct 22, 2020 at 10:18 AM <[email protected]> wrote:
> >
> > From: Yogesh Lal <[email protected]>
> >
> > Use STACK_HASH_ORDER_SHIFT to configure STACK_HASH_SIZE.
> >
> > Aim is to have configurable value for STACK_HASH_SIZE,
> > so depend on use case one can configure it.
> >
> > One example is of Page Owner, default value of
> > STACK_HASH_SIZE lead stack depot to consume 8MB of static memory.
> > Making it configurable and use lower value helps to enable features like
> > CONFIG_PAGE_OWNER without any significant overhead.
> >
> > Signed-off-by: Yogesh Lal <[email protected]>
> > Signed-off-by: Vinayak Menon <[email protected]>
> > Signed-off-by: Vijayanand Jitta <[email protected]>
> > ---
> > lib/Kconfig | 9 +++++++++
> > lib/stackdepot.c | 3 +--
> > 2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 18d76b6..b3f8259 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -651,6 +651,15 @@ config STACKDEPOT
> > bool
> > select STACKTRACE
> >
> > +config STACK_HASH_ORDER_SHIFT
> > + int "stack depot hash size (12 => 4KB, 20 => 1024KB)"
> > + range 12 20
> > + default 20
> > + depends on STACKDEPOT
> > + help
> > + Select the hash size as a power of 2 for the stackdepot hash
> > table.
> > + Choose a lower value to reduce the memory impact.
> > +
> > config SBITMAP
> > bool
> >
> > diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> > index 2caffc6..413c20b 100644
> > --- a/lib/stackdepot.c
> > +++ b/lib/stackdepot.c
> > @@ -142,8 +142,7 @@ static struct stack_record *depot_alloc_stack(unsigned
> > long *entries, int size,
> > return stack;
> > }
> >
> > -#define STACK_HASH_ORDER 20
> > -#define STACK_HASH_SIZE (1L << STACK_HASH_ORDER)
> > +#define STACK_HASH_SIZE (1L << CONFIG_STACK_HASH_ORDER_SHIFT)
> > #define STACK_HASH_MASK (STACK_HASH_SIZE - 1)
> > #define STACK_HASH_SEED 0x9747b28c
> >
> > --
> > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> > of Code Aurora Forum, hosted by The Linux Foundation
> > 2.7.4
> >
>
> 1. When we don't use page_owner, we don't want to waste any memory for
> stackdepot hash array.
> 2. When we use page_owner, we want to have reasonable stackdeport hash array
>
> With this configuration, it couldn't meet since we always need to
> reserve a reasonable size for the array.
> Can't we make the hash size as a kernel parameter?
> With it, we could use it like this.
>
> 1. page_owner=off, stackdepot_stack_hash=0 -> no more wasted memory
> when we don't use page_owner
> 2. page_owner=on, stackdepot_stack_hash=8M -> reasonable hash size
> when we use page_owner.
Seems we have other users like kasan, and dma_buf_ref which we introduced.
Also we can't guarantee there will not be any other users for stackdepot, so
it's better we not depend on only page owner?
>
>
> --
> Kind regards,
> Minchan Kim
>

2020-11-20 05:07:27

by Minchan Kim

[permalink] [raw]
Subject: Re: [PATCH] lib: stackdepot: Add support to configure STACK_HASH_SIZE

On Thu, Nov 19, 2020 at 11:34:32AM +0800, Zhenhua Huang wrote:
> On Wed, Nov 04, 2020 at 07:27:03AM +0800, Minchan Kim wrote:
> > Sorry if this mail corrupts the mail thread or had heavy mangling
> > since I lost this mail from my mailbox so I am sending this mail by
> > web gmail.
> >
> > On Thu, Oct 22, 2020 at 10:18 AM <[email protected]> wrote:
> > >
> > > From: Yogesh Lal <[email protected]>
> > >
> > > Use STACK_HASH_ORDER_SHIFT to configure STACK_HASH_SIZE.
> > >
> > > Aim is to have configurable value for STACK_HASH_SIZE,
> > > so depend on use case one can configure it.
> > >
> > > One example is of Page Owner, default value of
> > > STACK_HASH_SIZE lead stack depot to consume 8MB of static memory.
> > > Making it configurable and use lower value helps to enable features like
> > > CONFIG_PAGE_OWNER without any significant overhead.
> > >
> > > Signed-off-by: Yogesh Lal <[email protected]>
> > > Signed-off-by: Vinayak Menon <[email protected]>
> > > Signed-off-by: Vijayanand Jitta <[email protected]>
> > > ---
> > > lib/Kconfig | 9 +++++++++
> > > lib/stackdepot.c | 3 +--
> > > 2 files changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/Kconfig b/lib/Kconfig
> > > index 18d76b6..b3f8259 100644
> > > --- a/lib/Kconfig
> > > +++ b/lib/Kconfig
> > > @@ -651,6 +651,15 @@ config STACKDEPOT
> > > bool
> > > select STACKTRACE
> > >
> > > +config STACK_HASH_ORDER_SHIFT
> > > + int "stack depot hash size (12 => 4KB, 20 => 1024KB)"
> > > + range 12 20
> > > + default 20
> > > + depends on STACKDEPOT
> > > + help
> > > + Select the hash size as a power of 2 for the stackdepot hash
> > > table.
> > > + Choose a lower value to reduce the memory impact.
> > > +
> > > config SBITMAP
> > > bool
> > >
> > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> > > index 2caffc6..413c20b 100644
> > > --- a/lib/stackdepot.c
> > > +++ b/lib/stackdepot.c
> > > @@ -142,8 +142,7 @@ static struct stack_record *depot_alloc_stack(unsigned
> > > long *entries, int size,
> > > return stack;
> > > }
> > >
> > > -#define STACK_HASH_ORDER 20
> > > -#define STACK_HASH_SIZE (1L << STACK_HASH_ORDER)
> > > +#define STACK_HASH_SIZE (1L << CONFIG_STACK_HASH_ORDER_SHIFT)
> > > #define STACK_HASH_MASK (STACK_HASH_SIZE - 1)
> > > #define STACK_HASH_SEED 0x9747b28c
> > >
> > > --
> > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> > > of Code Aurora Forum, hosted by The Linux Foundation
> > > 2.7.4
> > >
> >
> > 1. When we don't use page_owner, we don't want to waste any memory for
> > stackdepot hash array.
> > 2. When we use page_owner, we want to have reasonable stackdeport hash array
> >
> > With this configuration, it couldn't meet since we always need to
> > reserve a reasonable size for the array.
> > Can't we make the hash size as a kernel parameter?
> > With it, we could use it like this.
> >
> > 1. page_owner=off, stackdepot_stack_hash=0 -> no more wasted memory
> > when we don't use page_owner
> > 2. page_owner=on, stackdepot_stack_hash=8M -> reasonable hash size
> > when we use page_owner.
> Seems we have other users like kasan, and dma_buf_ref which we introduced.
> Also we can't guarantee there will not be any other users for stackdepot, so
> it's better we not depend on only page owner?

I didn't mean to make it page_owner dependent. What I suggested is just
to define kernel parameter for stackdeport hash size so admin could
override it at right size when we really need it.

2020-11-20 07:11:20

by Zhenhua Huang

[permalink] [raw]
Subject: Re: [PATCH] lib: stackdepot: Add support to configure STACK_HASH_SIZE

On Fri, Nov 20, 2020 at 01:04:23PM +0800, Minchan Kim wrote:
> On Thu, Nov 19, 2020 at 11:34:32AM +0800, Zhenhua Huang wrote:
> > On Wed, Nov 04, 2020 at 07:27:03AM +0800, Minchan Kim wrote:
> > > Sorry if this mail corrupts the mail thread or had heavy mangling
> > > since I lost this mail from my mailbox so I am sending this mail by
> > > web gmail.
> > >
> > > On Thu, Oct 22, 2020 at 10:18 AM <[email protected]> wrote:
> > > >
> > > > From: Yogesh Lal <[email protected]>
> > > >
> > > > Use STACK_HASH_ORDER_SHIFT to configure STACK_HASH_SIZE.
> > > >
> > > > Aim is to have configurable value for STACK_HASH_SIZE,
> > > > so depend on use case one can configure it.
> > > >
> > > > One example is of Page Owner, default value of
> > > > STACK_HASH_SIZE lead stack depot to consume 8MB of static memory.
> > > > Making it configurable and use lower value helps to enable features
> like
> > > > CONFIG_PAGE_OWNER without any significant overhead.
> > > >
> > > > Signed-off-by: Yogesh Lal <[email protected]>
> > > > Signed-off-by: Vinayak Menon <[email protected]>
> > > > Signed-off-by: Vijayanand Jitta <[email protected]>
> > > > ---
> > > > lib/Kconfig | 9 +++++++++
> > > > lib/stackdepot.c | 3 +--
> > > > 2 files changed, 10 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/lib/Kconfig b/lib/Kconfig
> > > > index 18d76b6..b3f8259 100644
> > > > --- a/lib/Kconfig
> > > > +++ b/lib/Kconfig
> > > > @@ -651,6 +651,15 @@ config STACKDEPOT
> > > > bool
> > > > select STACKTRACE
> > > >
> > > > +config STACK_HASH_ORDER_SHIFT
> > > > + int "stack depot hash size (12 => 4KB, 20 => 1024KB)"
> > > > + range 12 20
> > > > + default 20
> > > > + depends on STACKDEPOT
> > > > + help
> > > > + Select the hash size as a power of 2 for the stackdepot
> hash
> > > > table.
> > > > + Choose a lower value to reduce the memory impact.
> > > > +
> > > > config SBITMAP
> > > > bool
> > > >
> > > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> > > > index 2caffc6..413c20b 100644
> > > > --- a/lib/stackdepot.c
> > > > +++ b/lib/stackdepot.c
> > > > @@ -142,8 +142,7 @@ static struct stack_record
> *depot_alloc_stack(unsigned
> > > > long *entries, int size,
> > > > return stack;
> > > > }
> > > >
> > > > -#define STACK_HASH_ORDER 20
> > > > -#define STACK_HASH_SIZE (1L << STACK_HASH_ORDER)
> > > > +#define STACK_HASH_SIZE (1L << CONFIG_STACK_HASH_ORDER_SHIFT)
> > > > #define STACK_HASH_MASK (STACK_HASH_SIZE - 1)
> > > > #define STACK_HASH_SEED 0x9747b28c
> > > >
> > > > --
> > > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member
> > > > of Code Aurora Forum, hosted by The Linux Foundation
> > > > 2.7.4
> > > >
> > >
> > > 1. When we don't use page_owner, we don't want to waste any memory for
> > > stackdepot hash array.
> > > 2. When we use page_owner, we want to have reasonable stackdeport hash
> array
> > >
> > > With this configuration, it couldn't meet since we always need to
> > > reserve a reasonable size for the array.
> > > Can't we make the hash size as a kernel parameter?
> > > With it, we could use it like this.
> > >
> > > 1. page_owner=off, stackdepot_stack_hash=0 -> no more wasted memory
> > > when we don't use page_owner
> > > 2. page_owner=on, stackdepot_stack_hash=8M -> reasonable hash size
> > > when we use page_owner.
> > Seems we have other users like kasan, and dma_buf_ref which we
> introduced.
> > Also we can't guarantee there will not be any other users for
> stackdepot, so
> > it's better we not depend on only page owner?
>
> I didn't mean to make it page_owner dependent. What I suggested is just
> to define kernel parameter for stackdeport hash size so admin could
> override it at right size when we really need it.
OK, Thanks Minchan for explanation. It's a good idea then, admin needs to
consider all users but, especailly when setting it to 0...

Thanks,
Zhenhua