linux-kernel - Re: [PATCH v4] vmstat: Kernel stack usage histogram

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2fbbxcsjs7vtzpb6a5wudbppcr2wgc2xwdw3cgs6ejzx6rioze@z2sct6rbulng>
Date: Thu, 18 Jul 2024 16:36:19 -0700
From: Shakeel Butt <shakeel.butt@...ux.dev>
To: Pasha Tatashin <pasha.tatashin@...een.com>
Cc: akpm@...ux-foundation.org, jpoimboe@...nel.org, 
	kent.overstreet@...ux.dev, peterz@...radead.org, nphamcs@...il.com, 
	cerasuolodomenico@...il.com, surenb@...gle.com, lizhijian@...itsu.com, willy@...radead.org, 
	vbabka@...e.cz, ziy@...dia.com, linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v4] vmstat: Kernel stack usage histogram

On Thu, Jul 18, 2024 at 08:26:11PM GMT, Pasha Tatashin wrote:
[...]
> diff --git a/include/linux/sched/task_stack.h b/include/linux/sched/task_stack.h
> index ccd72b978e1f..65e8c9fb7f9b 100644
> --- a/include/linux/sched/task_stack.h
> +++ b/include/linux/sched/task_stack.h
> @@ -95,9 +95,51 @@ static inline int object_is_on_stack(const void *obj)
>  extern void thread_stack_cache_init(void);
>  
>  #ifdef CONFIG_DEBUG_STACK_USAGE
> +#ifdef CONFIG_VM_EVENT_COUNTERS
> +#include <linux/vm_event_item.h>
> +
> +/* Count the maximum pages reached in kernel stacks */
> +static inline void kstack_histogram(unsigned long used_stack)

Any specific reason to add this function in header?

> +{
> +	if (used_stack <= 1024)
> +		this_cpu_inc(vm_event_states.event[KSTACK_1K]);

Why not count_vm_event(KSTACK_1K)? Avoiding header include recursion?

> +#if THREAD_SIZE > 1024
> +	else if (used_stack <= 2048)
> +		this_cpu_inc(vm_event_states.event[KSTACK_2K]);
> +#endif
> +#if THREAD_SIZE > 2048
> +	else if (used_stack <= 4096)
> +		this_cpu_inc(vm_event_states.event[KSTACK_4K]);
> +#endif
> +#if THREAD_SIZE > 4096
> +	else if (used_stack <= 8192)
> +		this_cpu_inc(vm_event_states.event[KSTACK_8K]);
> +#endif
> +#if THREAD_SIZE > 8192
> +	else if (used_stack <= 16384)
> +		this_cpu_inc(vm_event_states.event[KSTACK_16K]);
> +#endif
> +#if THREAD_SIZE > 16384
> +	else if (used_stack <= 32768)
> +		this_cpu_inc(vm_event_states.event[KSTACK_32K]);
> +#endif
> +#if THREAD_SIZE > 32768
> +	else if (used_stack <= 65536)
> +		this_cpu_inc(vm_event_states.event[KSTACK_64K]);
> +#endif
> +#if THREAD_SIZE > 65536
> +	else
> +		this_cpu_inc(vm_event_states.event[KSTACK_REST]);
> +#endif
> +}
> +#else /* !CONFIG_VM_EVENT_COUNTERS */
> +static inline void kstack_histogram(unsigned long used_stack) {}
> +#endif /* CONFIG_VM_EVENT_COUNTERS */
> +
>  static inline unsigned long stack_not_used(struct task_struct *p)
>  {
>  	unsigned long *n = end_of_stack(p);
> +	unsigned long unused_stack;
>  
>  	do { 	/* Skip over canary */
>  # ifdef CONFIG_STACK_GROWSUP
> @@ -108,10 +150,13 @@ static inline unsigned long stack_not_used(struct task_struct *p)
>  	} while (!*n);
>  
>  # ifdef CONFIG_STACK_GROWSUP
> -	return (unsigned long)end_of_stack(p) - (unsigned long)n;
> +	unused_stack = (unsigned long)end_of_stack(p) - (unsigned long)n;
>  # else
> -	return (unsigned long)n - (unsigned long)end_of_stack(p);
> +	unused_stack = (unsigned long)n - (unsigned long)end_of_stack(p);
>  # endif
> +	kstack_histogram(THREAD_SIZE - unused_stack);
> +
> +	return unused_stack;
>  }
>  #endif
>  extern void set_task_stack_end_magic(struct task_struct *tsk);