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

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

[vjitta]

From: Yogesh Lal <ylal@codeaurora.org>

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 <ylal@codeaurora.org>
Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
---
 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

[PATCH v5 2/2] lib: stackdepot: Add support to disable stack depot

[vjitta]

From: Vijayanand Jitta <vjitta@codeaurora.org>

Add a kernel parameter stack_depot_disable to disable
stack depot. So that stack hash table doesn't consume
any memory when stack depot is disabled.

Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
---
 include/linux/stackdepot.h |  1 +
 init/main.c                |  2 ++
 lib/stackdepot.c           | 33 +++++++++++++++++++++++++++++----
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index 24d49c7..eafd9aa 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -21,4 +21,5 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle,
 
 unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries);
 
+int stack_depot_init(void);
 #endif
diff --git a/init/main.c b/init/main.c
index 32b2a8a..8fcf9bb 100644
--- a/init/main.c
+++ b/init/main.c
@@ -98,6 +98,7 @@
 #include <linux/mem_encrypt.h>
 #include <linux/kcsan.h>
 #include <linux/init_syscalls.h>
+#include <linux/stackdepot.h>
 
 #include <asm/io.h>
 #include <asm/bugs.h>
@@ -827,6 +828,7 @@ static void __init mm_init(void)
 	page_ext_init_flatmem();
 	init_debug_pagealloc();
 	report_meminit();
+	stack_depot_init();
 	mem_init();
 	kmem_cache_init();
 	kmemleak_init();
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index dff8521..d20e6fd 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -31,6 +31,8 @@
 #include <linux/stackdepot.h>
 #include <linux/string.h>
 #include <linux/types.h>
+#include <linux/vmalloc.h>
+#include <linux/memblock.h>
 
 #define DEPOT_STACK_BITS (sizeof(depot_stack_handle_t) * 8)
 
@@ -146,9 +148,32 @@ static struct stack_record *depot_alloc_stack(unsigned long *entries, int size,
 #define STACK_HASH_MASK (STACK_HASH_SIZE - 1)
 #define STACK_HASH_SEED 0x9747b28c
 
-static struct stack_record *stack_table[STACK_HASH_SIZE] = {
-	[0 ...	STACK_HASH_SIZE - 1] = NULL
-};
+static bool stack_depot_disable;
+static struct stack_record **stack_table;
+
+static int __init is_stack_depot_disabled(char *str)
+{
+	kstrtobool(str, &stack_depot_disable);
+	if (stack_depot_disable) {
+		pr_info("Stack Depot is disabled\n");
+		stack_table = NULL;
+	}
+	return 0;
+}
+early_param("stack_depot_disable", is_stack_depot_disabled);
+
+int __init stack_depot_init(void)
+{
+	if (!stack_depot_disable) {
+		size_t size = (STACK_HASH_SIZE * sizeof(struct stack_record *));
+		int i;
+
+		stack_table = memblock_alloc(size, size);
+		for (i = 0; i < STACK_HASH_SIZE;  i++)
+			stack_table[i] = NULL;
+	}
+	return 0;
+}
 
 /* Calculate hash for a stack */
 static inline u32 hash_stack(unsigned long *entries, unsigned int size)
@@ -242,7 +267,7 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries,
 	unsigned long flags;
 	u32 hash;
 
-	if (unlikely(nr_entries == 0))
+	if (unlikely(nr_entries == 0) || stack_depot_disable)
 		goto fast_exit;
 
 	hash = hash_stack(entries, nr_entries);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
2.7.4

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

On 1/18/21 1:56 AM, vjitta@codeaurora.org wrote:
> From: Yogesh Lal <ylal@codeaurora.org>
> 
> 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 <ylal@codeaurora.org>
> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>

Hi,

Did you see
https://lore.kernel.org/lkml/202101050729.cwTd47Yw-lkp@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

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

[Randy Dunlap]

On 1/18/21 1:56 AM, vjitta@codeaurora.org wrote:
> From: Yogesh Lal <ylal@codeaurora.org>
> 
> 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 <ylal@codeaurora.org>
> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>

Hi,

Did you see
https://lore.kernel.org/lkml/202101050729.cwTd47Yw-lkp@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

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

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

[Vijayanand Jitta]

On 1/19/2021 4:23 AM, Randy Dunlap wrote:
> On 1/18/21 1:56 AM, vjitta@codeaurora.org wrote:
>> From: Yogesh Lal <ylal@codeaurora.org>
>>
>> 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 <ylal@codeaurora.org>
>> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
>> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
> 
> Hi,
> 
> Did you see
> https://lore.kernel.org/lkml/202101050729.cwTd47Yw-lkp@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

[Vijayanand Jitta]

On 1/19/2021 4:23 AM, Randy Dunlap wrote:
> On 1/18/21 1:56 AM, vjitta@codeaurora.org wrote:
>> From: Yogesh Lal <ylal@codeaurora.org>
>>
>> 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 <ylal@codeaurora.org>
>> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
>> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
> 
> Hi,
> 
> Did you see
> https://lore.kernel.org/lkml/202101050729.cwTd47Yw-lkp@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

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

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

[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, vjitta@codeaurora.org wrote:
>>> From: Yogesh Lal <ylal@codeaurora.org>
>>>
>>> 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 <ylal@codeaurora.org>
>>> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
>>> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
>>
>> Hi,
>>
>> Did you see
>> https://lore.kernel.org/lkml/202101050729.cwTd47Yw-lkp@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

[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, vjitta@codeaurora.org wrote:
>>> From: Yogesh Lal <ylal@codeaurora.org>
>>>
>>> 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 <ylal@codeaurora.org>
>>> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
>>> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
>>
>> Hi,
>>
>> Did you see
>> https://lore.kernel.org/lkml/202101050729.cwTd47Yw-lkp@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

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

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

[Minchan Kim]

On Mon, Jan 18, 2021 at 03:26:41PM +0530, vjitta@codeaurora.org wrote:
> From: Yogesh Lal <ylal@codeaurora.org>
> 
> 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 <ylal@codeaurora.org>
> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
Reviewed-by: Minchan Kim <minchan@kernel.org>

Re: [PATCH v5 2/2] lib: stackdepot: Add support to disable stack depot

[Minchan Kim]

On Mon, Jan 18, 2021 at 03:26:42PM +0530, vjitta@codeaurora.org wrote:
> From: Vijayanand Jitta <vjitta@codeaurora.org>
> 
> Add a kernel parameter stack_depot_disable to disable
> stack depot. So that stack hash table doesn't consume
> any memory when stack depot is disabled.

The usecase is CONFIG_PAGE_OWNER without page_owner=on.
Without this patch, stackdepot will consume the memory
for the hashtable. By default, it's 8M which is never trivial.

With this option, in CONFIG_PAGE_OWNER configured system,
page_owner=off, stack_depot_disable in kernel command line,
we could save the wasted memory for the hashtable.

> 
> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>

Please also update kernel-parameters.txt.

> ---
>  include/linux/stackdepot.h |  1 +
>  init/main.c                |  2 ++
>  lib/stackdepot.c           | 33 +++++++++++++++++++++++++++++----
>  3 files changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
> index 24d49c7..eafd9aa 100644
> --- a/include/linux/stackdepot.h
> +++ b/include/linux/stackdepot.h
> @@ -21,4 +21,5 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle,
>  
>  unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries);
>  
> +int stack_depot_init(void);
>  #endif
> diff --git a/init/main.c b/init/main.c
> index 32b2a8a..8fcf9bb 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -98,6 +98,7 @@
>  #include <linux/mem_encrypt.h>
>  #include <linux/kcsan.h>
>  #include <linux/init_syscalls.h>
> +#include <linux/stackdepot.h>
>  
>  #include <asm/io.h>
>  #include <asm/bugs.h>
> @@ -827,6 +828,7 @@ static void __init mm_init(void)
>  	page_ext_init_flatmem();
>  	init_debug_pagealloc();
>  	report_meminit();
> +	stack_depot_init();
>  	mem_init();
>  	kmem_cache_init();
>  	kmemleak_init();
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index dff8521..d20e6fd 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -31,6 +31,8 @@
>  #include <linux/stackdepot.h>
>  #include <linux/string.h>
>  #include <linux/types.h>
> +#include <linux/vmalloc.h>

Why do we need vmalloc?

Otherwise, looks good to me.
Thank you!

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

[Vijayanand Jitta]

On 1/22/2021 5:49 AM, Minchan Kim wrote:
> On Mon, Jan 18, 2021 at 03:26:41PM +0530, vjitta@codeaurora.org wrote:
>> From: Yogesh Lal <ylal@codeaurora.org>
>>
>> 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 <ylal@codeaurora.org>
>> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
>> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
> Reviewed-by: Minchan Kim <minchan@kernel.org>
> 

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

Re: [PATCH v5 2/2] lib: stackdepot: Add support to disable stack depot

[Vijayanand Jitta]

On 1/22/2021 5:55 AM, Minchan Kim wrote:
> On Mon, Jan 18, 2021 at 03:26:42PM +0530, vjitta@codeaurora.org wrote:
>> From: Vijayanand Jitta <vjitta@codeaurora.org>
>>
>> Add a kernel parameter stack_depot_disable to disable
>> stack depot. So that stack hash table doesn't consume
>> any memory when stack depot is disabled.
> 
> The usecase is CONFIG_PAGE_OWNER without page_owner=on.
> Without this patch, stackdepot will consume the memory
> for the hashtable. By default, it's 8M which is never trivial.
> 
> With this option, in CONFIG_PAGE_OWNER configured system,
> page_owner=off, stack_depot_disable in kernel command line,
> we could save the wasted memory for the hashtable.
> 

Sure, will update the commit text with above details.
>>
>> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
>> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
> 
> Please also update kernel-parameters.txt.
> 

Sure.
>> ---
>>  include/linux/stackdepot.h |  1 +
>>  init/main.c                |  2 ++
>>  lib/stackdepot.c           | 33 +++++++++++++++++++++++++++++----
>>  3 files changed, 32 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
>> index 24d49c7..eafd9aa 100644
>> --- a/include/linux/stackdepot.h
>> +++ b/include/linux/stackdepot.h
>> @@ -21,4 +21,5 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle,
>>  
>>  unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries);
>>  
>> +int stack_depot_init(void);
>>  #endif
>> diff --git a/init/main.c b/init/main.c
>> index 32b2a8a..8fcf9bb 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -98,6 +98,7 @@
>>  #include <linux/mem_encrypt.h>
>>  #include <linux/kcsan.h>
>>  #include <linux/init_syscalls.h>
>> +#include <linux/stackdepot.h>
>>  
>>  #include <asm/io.h>
>>  #include <asm/bugs.h>
>> @@ -827,6 +828,7 @@ static void __init mm_init(void)
>>  	page_ext_init_flatmem();
>>  	init_debug_pagealloc();
>>  	report_meminit();
>> +	stack_depot_init();
>>  	mem_init();
>>  	kmem_cache_init();
>>  	kmemleak_init();
>> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
>> index dff8521..d20e6fd 100644
>> --- a/lib/stackdepot.c
>> +++ b/lib/stackdepot.c
>> @@ -31,6 +31,8 @@
>>  #include <linux/stackdepot.h>
>>  #include <linux/string.h>
>>  #include <linux/types.h>
>> +#include <linux/vmalloc.h>
> 
> Why do we need vmalloc?
> 

Its not needed, will remove it.

> Otherwise, looks good to me.
> Thank you!
> 

Thanks for the review.

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

[Alexander Potapenko]

On Mon, Jan 18, 2021 at 10:57 AM <vjitta@codeaurora.org> wrote:
>
> From: Yogesh Lal <ylal@codeaurora.org>
>
> 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 <ylal@codeaurora.org>
> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
Reviewed-by: Alexander Potapenko <glider@google.com>

> ---
>  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

[Alexander Potapenko]

On Mon, Jan 18, 2021 at 11:54 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 1/18/21 1:56 AM, vjitta@codeaurora.org wrote:
> > From: Yogesh Lal <ylal@codeaurora.org>
> >
> > 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 <ylal@codeaurora.org>
> > Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> > Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
>
> Hi,
>
> Did you see
> https://lore.kernel.org/lkml/202101050729.cwTd47Yw-lkp@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

[Alexander Potapenko]

On Mon, Jan 18, 2021 at 11:54 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 1/18/21 1:56 AM, vjitta@codeaurora.org wrote:
> > From: Yogesh Lal <ylal@codeaurora.org>
> >
> > 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 <ylal@codeaurora.org>
> > Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> > Signed-off-by: Vijayanand Jitta <vjitta@codeaurora.org>
>
> Hi,
>
> Did you see
> https://lore.kernel.org/lkml/202101050729.cwTd47Yw-lkp@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

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc