Re: [PATCH v5 1/2] lib: stackdepot: Add support to configure STACK_HASH_SIZE

2021-01-22 Thread Alexander Potapenko
On Mon, Jan 18, 2021 at 11:54 PM Randy Dunlap  wrote:
>
> On 1/18/21 1:56 AM, vji...@codeaurora.org wrote:
> > From: Yogesh Lal 
> >
> > Use CONFIG_STACK_HASH_ORDER 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 
> > Signed-off-by: Vinayak Menon 
> > Signed-off-by: Vijayanand Jitta 
>
> Hi,
>
> Did you see
> https://lore.kernel.org/lkml/202101050729.cwtd47yw-...@intel.com/
>
> It seems that arch/arc/ does not have:
>arc-elf-ld: lib/stackdepot.o: in function `filter_irq_stacks':
>(.text+0x6): undefined reference to `__irqentry_text_start'
> >> arc-elf-ld: (.text+0x6): undefined reference to `__irqentry_text_start'
> >> arc-elf-ld: (.text+0x26): undefined reference to `__irqentry_text_end'
> >> arc-elf-ld: (.text+0x26): undefined reference to `__irqentry_text_end'
> >> arc-elf-ld: (.text+0x34): undefined reference to 
> >> `__softirqentry_text_start'
> >> arc-elf-ld: (.text+0x34): undefined reference to 
> >> `__softirqentry_text_start'
> >> arc-elf-ld: (.text+0x3c): undefined reference to `__softirqentry_text_end'
> >> arc-elf-ld: (.text+0x3c): undefined reference to `__softirqentry_text_end'
>
Hi Randy,

Could you try out the following patch?

Thanks,
Alex

diff --git a/arch/arc/kernel/vmlinux.lds.S b/arch/arc/kernel/vmlinux.lds.S
index 33ce59d91461..94d3f9620d0b 100644
--- a/arch/arc/kernel/vmlinux.lds.S
+++ b/arch/arc/kernel/vmlinux.lds.S
@@ -83,6 +83,8 @@ SECTIONS

.text : {
_text = .;
+   IRQENTRY_TEXT
+   SOFTIRQENTRY_TEXT
TEXT_TEXT
SCHED_TEXT
CPUIDLE_TEXT


Re: [PATCH v5 1/2] lib: stackdepot: Add support to configure STACK_HASH_SIZE

2021-01-22 Thread Alexander Potapenko
On Mon, Jan 18, 2021 at 10:57 AM  wrote:
>
> From: Yogesh Lal 
>
> Use CONFIG_STACK_HASH_ORDER 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 
> Signed-off-by: Vinayak Menon 
> Signed-off-by: Vijayanand Jitta 
Reviewed-by: Alexander Potapenko 

> ---
>  lib/Kconfig  | 9 +
>  lib/stackdepot.c | 3 +--
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/lib/Kconfig b/lib/Kconfig
> index b46a9fd..96ee125 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -651,6 +651,15 @@ config STACKDEPOT
> bool
> select STACKTRACE
>
> +config STACK_HASH_ORDER
> +   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..dff8521 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)
>  #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
>


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg


Re: [PATCH v5 1/2] lib: stackdepot: Add support to configure STACK_HASH_SIZE

2021-01-21 Thread Vijayanand Jitta



On 1/22/2021 5:49 AM, Minchan Kim wrote:
> On Mon, Jan 18, 2021 at 03:26:41PM +0530, vji...@codeaurora.org wrote:
>> From: Yogesh Lal 
>>
>> Use CONFIG_STACK_HASH_ORDER 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.
> 
> The description could be improved to prevent confusing.
> CONFIG_PAGE_OWNER works only if page_owner=on via kernel parameter
> on CONFIG_PAGE_OWNER configured system.
> Thus, unless admin enable it via command line option, the stackdepot
> will just waste 8M memory without any customer.
> 

Sure, will update the commit text as suggested.

Thanks,
Vijay
>>
>> Signed-off-by: Yogesh Lal 
>> Signed-off-by: Vinayak Menon 
>> Signed-off-by: Vijayanand Jitta 
> Reviewed-by: Minchan Kim 
> 

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


Re: [PATCH v5 1/2] lib: stackdepot: Add support to configure STACK_HASH_SIZE

2021-01-21 Thread Minchan Kim
On Mon, Jan 18, 2021 at 03:26:41PM +0530, vji...@codeaurora.org wrote:
> From: Yogesh Lal 
> 
> Use CONFIG_STACK_HASH_ORDER 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.

The description could be improved to prevent confusing.
CONFIG_PAGE_OWNER works only if page_owner=on via kernel parameter
on CONFIG_PAGE_OWNER configured system.
Thus, unless admin enable it via command line option, the stackdepot
will just waste 8M memory without any customer.

> 
> Signed-off-by: Yogesh Lal 
> Signed-off-by: Vinayak Menon 
> Signed-off-by: Vijayanand Jitta 
Reviewed-by: Minchan Kim 


Re: [PATCH v5 1/2] lib: stackdepot: Add support to configure STACK_HASH_SIZE

2021-01-18 Thread Randy Dunlap
On 1/18/21 10:52 PM, Vijayanand Jitta wrote:
> 
> 
> On 1/19/2021 4:23 AM, Randy Dunlap wrote:
>> On 1/18/21 1:56 AM, vji...@codeaurora.org wrote:
>>> From: Yogesh Lal 
>>>
>>> Use CONFIG_STACK_HASH_ORDER 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 
>>> Signed-off-by: Vinayak Menon 
>>> Signed-off-by: Vijayanand Jitta 
>>
>> Hi,
>>
>> Did you see
>> https://lore.kernel.org/lkml/202101050729.cwtd47yw-...@intel.com/
>>
>> It seems that arch/arc/ does not have:
>>arc-elf-ld: lib/stackdepot.o: in function `filter_irq_stacks':
>>(.text+0x6): undefined reference to `__irqentry_text_start'
 arc-elf-ld: (.text+0x6): undefined reference to `__irqentry_text_start'
 arc-elf-ld: (.text+0x26): undefined reference to `__irqentry_text_end'
 arc-elf-ld: (.text+0x26): undefined reference to `__irqentry_text_end'
 arc-elf-ld: (.text+0x34): undefined reference to 
 `__softirqentry_text_start'
 arc-elf-ld: (.text+0x34): undefined reference to 
 `__softirqentry_text_start'
 arc-elf-ld: (.text+0x3c): undefined reference to `__softirqentry_text_end'
 arc-elf-ld: (.text+0x3c): undefined reference to `__softirqentry_text_end'
>>
>>
>>
>>
> 
> The above issue seems to be because of a different patch.
> This one
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=505a0ef15f96c6c43ec719c9fc1833d98957bb39
> 
> I didn't really get why you referred that here.

Yes, I noticed that later. Sorry about that.

Maybe Alexander P. can look into it...

thanks.
-- 
~Randy
You can't do anything without having to do something else first.
-- Belefant's Law


Re: [PATCH v5 1/2] lib: stackdepot: Add support to configure STACK_HASH_SIZE

2021-01-18 Thread Vijayanand Jitta



On 1/19/2021 4:23 AM, Randy Dunlap wrote:
> On 1/18/21 1:56 AM, vji...@codeaurora.org wrote:
>> From: Yogesh Lal 
>>
>> Use CONFIG_STACK_HASH_ORDER 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 
>> Signed-off-by: Vinayak Menon 
>> Signed-off-by: Vijayanand Jitta 
> 
> Hi,
> 
> Did you see
> https://lore.kernel.org/lkml/202101050729.cwtd47yw-...@intel.com/
> 
> It seems that arch/arc/ does not have:
>arc-elf-ld: lib/stackdepot.o: in function `filter_irq_stacks':
>(.text+0x6): undefined reference to `__irqentry_text_start'
>>> arc-elf-ld: (.text+0x6): undefined reference to `__irqentry_text_start'
>>> arc-elf-ld: (.text+0x26): undefined reference to `__irqentry_text_end'
>>> arc-elf-ld: (.text+0x26): undefined reference to `__irqentry_text_end'
>>> arc-elf-ld: (.text+0x34): undefined reference to `__softirqentry_text_start'
>>> arc-elf-ld: (.text+0x34): undefined reference to `__softirqentry_text_start'
>>> arc-elf-ld: (.text+0x3c): undefined reference to `__softirqentry_text_end'
>>> arc-elf-ld: (.text+0x3c): undefined reference to `__softirqentry_text_end'
> 
> 
> 
> 

The above issue seems to be because of a different patch.
This one
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=505a0ef15f96c6c43ec719c9fc1833d98957bb39

I didn't really get why you referred that here.

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


Re: [PATCH v5 1/2] lib: stackdepot: Add support to configure STACK_HASH_SIZE

2021-01-18 Thread Randy Dunlap
On 1/18/21 1:56 AM, vji...@codeaurora.org wrote:
> From: Yogesh Lal 
> 
> Use CONFIG_STACK_HASH_ORDER 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 
> Signed-off-by: Vinayak Menon 
> Signed-off-by: Vijayanand Jitta 

Hi,

Did you see
https://lore.kernel.org/lkml/202101050729.cwtd47yw-...@intel.com/

It seems that arch/arc/ does not have:
   arc-elf-ld: lib/stackdepot.o: in function `filter_irq_stacks':
   (.text+0x6): undefined reference to `__irqentry_text_start'
>> arc-elf-ld: (.text+0x6): undefined reference to `__irqentry_text_start'
>> arc-elf-ld: (.text+0x26): undefined reference to `__irqentry_text_end'
>> arc-elf-ld: (.text+0x26): undefined reference to `__irqentry_text_end'
>> arc-elf-ld: (.text+0x34): undefined reference to `__softirqentry_text_start'
>> arc-elf-ld: (.text+0x34): undefined reference to `__softirqentry_text_start'
>> arc-elf-ld: (.text+0x3c): undefined reference to `__softirqentry_text_end'
>> arc-elf-ld: (.text+0x3c): undefined reference to `__softirqentry_text_end'




-- 
~Randy
You can't do anything without having to do something else first.
-- Belefant's Law


[PATCH v5 1/2] lib: stackdepot: Add support to configure STACK_HASH_SIZE

2021-01-18 Thread vjitta
From: Yogesh Lal 

Use CONFIG_STACK_HASH_ORDER 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 
Signed-off-by: Vinayak Menon 
Signed-off-by: Vijayanand Jitta 
---
 lib/Kconfig  | 9 +
 lib/stackdepot.c | 3 +--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index b46a9fd..96ee125 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -651,6 +651,15 @@ config STACKDEPOT
bool
select STACKTRACE
 
+config STACK_HASH_ORDER
+   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..dff8521 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)
 #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