[PATCH 0/2] Introduce panic function when slub leaks

[PATCH 0/2] Introduce panic function when slub leaks

[Fangzheng Zhang]

Hi all,

A method to detect slub leaks by monitoring its usage in real time
on the page allocation path of the slub. When the slub occupancy
exceeds the user-set value, it is considered that the slub is leaking
at this time, and a panic operation will be triggered immediately.

Fangzheng Zhang (2):
  mm/slub: Add panic function when slub leaks
  Documentation: admin-guide: kernel-parameters: Add parameter
    description for slub_leak_panic function

 .../admin-guide/kernel-parameters.txt         | 15 ++++
 mm/Kconfig                                    | 11 ++++++++
 mm/slub.c                                     | 76 +++++++++++++++++++

 3 files changed, 102 insertions(+)

-- 
2.17.1

[PATCH 1/2] mm/slub: Add panic function when slub leaks

[Fangzheng Zhang]

Perform real-time memory usage monitoring on the slub page
allocation paths, ie, kmalloc_large_alloced and alloc_slab_page.
When the usage exceeds the set threshole value, the panic function
will be triggered.

Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
---
 mm/Kconfig | 11 ++++++++
 mm/slub.c  | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index 09aebca1cae3..60cf72d4f0da 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -255,6 +255,17 @@ config SLUB_TINY
 
 	   If unsure, say N.
 
+config SLUB_LEAK_PANIC
+	bool "Trigger panic when slub leaks"
+	default y
+	help
+	  Detect slub leaks by monitoring its usage in real time on the page
+	  allocation path of the slub. When the slub occupancy exceeds the
+	  user-set value, it is considered that the slub is leaking at this
+	  time, and a panic operation will be triggered immediately. Uers
+	  can enable and set leak threshold by using the kernel command line
+	  parameters "slub.leak_panic" and "slub.leak_panic_threshold".
+
 config SLAB_MERGE_DEFAULT
 	bool "Allow slab caches to be merged"
 	default y
diff --git a/mm/slub.c b/mm/slub.c
index 21f71cb6cc06..91049f87ab98 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -42,6 +42,9 @@
 #include <kunit/test.h>
 #include <kunit/test-bug.h>
 #include <linux/sort.h>
+#ifdef CONFIG_SLUB_LEAK_PANIC
+#include <linux/vmstat.h>
+#endif
 
 #include <linux/debugfs.h>
 #include <trace/events/kmem.h>
@@ -218,6 +221,15 @@ DEFINE_STATIC_KEY_FALSE(slub_debug_enabled);
 #endif
 #endif		/* CONFIG_SLUB_DEBUG */
 
+/* Internal slub_leak_panic definitions */
+#ifdef CONFIG_SLUB_LEAK_PANIC
+#define K(x) ((x) << (PAGE_SHIFT-10))
+static bool __read_mostly slub_leak_panic_enabled;
+static unsigned int __read_mostly slub_leak_panic_threshold;
+static long max_slab_count, temp_slab_count;
+#endif
+
+
 /* Structure holding parameters for get_partial() call chain */
 struct partial_context {
 	gfp_t flags;
@@ -2424,6 +2436,21 @@ static inline struct slab *alloc_slab_page(gfp_t flags, int node,
 	if (folio_is_pfmemalloc(folio))
 		slab_set_pfmemalloc(slab);
 
+#ifdef CONFIG_SLUB_LEAK_PANIC
+	if (likely(slub_leak_panic_enabled) && slub_leak_panic_threshold > 0) {
+		max_slab_count = K(totalram_pages()) * slub_leak_panic_threshold / 100;
+		temp_slab_count = K(global_node_page_state_pages(NR_SLAB_RECLAIMABLE_B))
+				+ K(global_node_page_state_pages(NR_SLAB_UNRECLAIMABLE_B))
+				+ K(1 << order);
+		if (temp_slab_count > max_slab_count)
+			panic("SLAB LEAK: %s(temp_count %6luKB > max_count %6luKB):\n"
+				"%s gfp_mask=%#x(%pGg), order=%d kB, oom_score_adj=%d\n",
+				__func__, temp_slab_count, max_slab_count,
+				current->comm, flags, &flags, order,
+				current->signal->oom_score_adj);
+	}
+#endif
+
 	return slab;
 }
 
@@ -4212,6 +4239,19 @@ static void *___kmalloc_large_node(size_t size, gfp_t flags, int node)
 		ptr = folio_address(folio);
 		lruvec_stat_mod_folio(folio, NR_SLAB_UNRECLAIMABLE_B,
 				      PAGE_SIZE << order);
+#ifdef CONFIG_SLUB_LEAK_PANIC
+		if (likely(slub_leak_panic_enabled) && slub_leak_panic_threshold > 0) {
+			max_slab_count = K(totalram_pages()) * slub_leak_panic_threshold / 100;
+			temp_slab_count = K(global_node_page_state_pages(NR_SLAB_RECLAIMABLE_B))
+					+ K(global_node_page_state_pages(NR_SLAB_UNRECLAIMABLE_B));
+			if (temp_slab_count > max_slab_count)
+				panic("SLAB LEAK: %s(temp_count %6luKB > max_count %6luKB):\n"
+					"%s gfp_mask=%#x(%pGg), order=%d kB, oom_score_adj=%d\n",
+					__func__, temp_slab_count, max_slab_count,
+					current->comm, flags, &flags, order,
+					current->signal->oom_score_adj);
+		}
+#endif
 	}
 
 	ptr = kasan_kmalloc_large(ptr, size, flags);
@@ -7443,3 +7483,39 @@ void get_slabinfo(struct kmem_cache *s, struct slabinfo *sinfo)
 	sinfo->cache_order = oo_order(s->oo);
 }
 #endif /* CONFIG_SLUB_DEBUG */
+
+/*
+ * The /sys/module/slub ABI
+ */
+#ifdef CONFIG_SLUB_LEAK_PANIC
+/*
+ * What:          /sys/module/slub/parameters/leak_panic
+ *                /sys/module/slub/parameters/leak_panic_threshold
+ * Date:          Sep 2024
+ * KernelVersion: v6.6+
+ * Description:   Used for slub memory leak check. When the user
+ *                successfully allocates the slub page, it also performs
+ *                statistics on the total slub usage in the system.
+ *                When the usage exceeds the set value
+ *                (threshold * memtotal / 100), it is considered that
+ *                there is a risk of slub leakage in the system at this time.
+ *                A panic operation will be triggered.
+ * Users:         userspace
+ */
+MODULE_PARM_DESC(leak_panic, "Disable/Enable slub_leak_panic");
+module_param_named(leak_panic, slub_leak_panic_enabled, bool, 0644);
+
+static int slub_leak_panic_threshold_set(const char *val, const struct kernel_param *kp)
+{
+	return param_set_uint_minmax(val, kp, 0, 100);
+}
+
+static const struct kernel_param_ops slub_leak_panic_threshold_ops = {
+	.set = slub_leak_panic_threshold_set,
+	.get = param_get_uint,
+};
+
+MODULE_PARM_DESC(leak_panic_threshold,
+		"Upper limit value of slub, expressed as a percentage of memtotal (0 ~ 100)");
+module_param_cb(leak_panic_threshold,
+		&slub_leak_panic_threshold_ops, &slub_leak_panic_threshold, 0644);
+#endif /* CONFIG_SLUB_LEAK_PANIC */
-- 
2.17.1

Re: [PATCH 1/2] mm/slub: Add panic function when slub leaks

[Greg KH]

On Wed, Sep 25, 2024 at 11:22:55AM +0800, Fangzheng Zhang wrote:
> Perform real-time memory usage monitoring on the slub page
> allocation paths, ie, kmalloc_large_alloced and alloc_slab_page.
> When the usage exceeds the set threshole value, the panic function
> will be triggered.
> 
> Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
> ---
>  mm/Kconfig | 11 ++++++++
>  mm/slub.c  | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 87 insertions(+)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 09aebca1cae3..60cf72d4f0da 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -255,6 +255,17 @@ config SLUB_TINY
>  
>  	   If unsure, say N.
>  
> +config SLUB_LEAK_PANIC
> +	bool "Trigger panic when slub leaks"
> +	default y

"default y" is only there if the feature is required to boot the machine
properly.  That's not the case here, sorry.

thanks,

greg k-h

Re: [PATCH 1/2] mm/slub: Add panic function when slub leaks

[zhang fangzheng]

On Wed, Sep 25, 2024 at 4:10 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Sep 25, 2024 at 11:22:55AM +0800, Fangzheng Zhang wrote:
> > Perform real-time memory usage monitoring on the slub page
> > allocation paths, ie, kmalloc_large_alloced and alloc_slab_page.
> > When the usage exceeds the set threshole value, the panic function
> > will be triggered.
> >
> > Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
> > ---
> >  mm/Kconfig | 11 ++++++++
> >  mm/slub.c  | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 87 insertions(+)
> >
> > diff --git a/mm/Kconfig b/mm/Kconfig
> > index 09aebca1cae3..60cf72d4f0da 100644
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -255,6 +255,17 @@ config SLUB_TINY
> >
> >          If unsure, say N.
> >
> > +config SLUB_LEAK_PANIC
> > +     bool "Trigger panic when slub leaks"
> > +     default y
>
> "default y" is only there if the feature is required to boot the machine
> properly.  That's not the case here, sorry.
>
> thanks,
>
> greg k-h

Hi Greg,

The reason why the "default y" is set here is that config is only valid for
the functional code that works. To truly implement slub_leak_panic, the
parameters must be set effectively. And, the current parameters are all
in the default n.

Thanks.

Re: [PATCH 1/2] mm/slub: Add panic function when slub leaks

[Greg KH]

On Wed, Sep 25, 2024 at 11:22:55AM +0800, Fangzheng Zhang wrote:
> Perform real-time memory usage monitoring on the slub page
> allocation paths, ie, kmalloc_large_alloced and alloc_slab_page.
> When the usage exceeds the set threshole value, the panic function
> will be triggered.
> 
> Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
> ---
>  mm/Kconfig | 11 ++++++++
>  mm/slub.c  | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 87 insertions(+)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 09aebca1cae3..60cf72d4f0da 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -255,6 +255,17 @@ config SLUB_TINY
>  
>  	   If unsure, say N.
>  
> +config SLUB_LEAK_PANIC
> +	bool "Trigger panic when slub leaks"
> +	default y
> +	help
> +	  Detect slub leaks by monitoring its usage in real time on the page
> +	  allocation path of the slub. When the slub occupancy exceeds the
> +	  user-set value, it is considered that the slub is leaking at this
> +	  time, and a panic operation will be triggered immediately. Uers
> +	  can enable and set leak threshold by using the kernel command line
> +	  parameters "slub.leak_panic" and "slub.leak_panic_threshold".
> +
>  config SLAB_MERGE_DEFAULT
>  	bool "Allow slab caches to be merged"
>  	default y
> diff --git a/mm/slub.c b/mm/slub.c
> index 21f71cb6cc06..91049f87ab98 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -42,6 +42,9 @@
>  #include <kunit/test.h>
>  #include <kunit/test-bug.h>
>  #include <linux/sort.h>
> +#ifdef CONFIG_SLUB_LEAK_PANIC
> +#include <linux/vmstat.h>
> +#endif

Please redo this to not require #ifdef in .c files, otherwise it gets
very unmaintainable over time.

thanks,

greg k-h

Re: [PATCH 1/2] mm/slub: Add panic function when slub leaks

[kernel test robot]

Hi Fangzheng,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v6.11 next-20240925]
[cannot apply to vbabka-slab/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Fangzheng-Zhang/mm-slub-Add-panic-function-when-slub-leaks/20240925-112601
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240925032256.1782-2-fangzheng.zhang%40unisoc.com
patch subject: [PATCH 1/2] mm/slub: Add panic function when slub leaks
config: alpha-allnoconfig (https://download.01.org/0day-ci/archive/20240925/202409251901.XFGbDalC-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240925/202409251901.XFGbDalC-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409251901.XFGbDalC-lkp@intel.com/

All errors (new ones prefixed by >>):

>> mm/slub.c:7354: error: unterminated #ifdef
    7354 | #ifdef CONFIG_SLUB_LEAK_PANIC
         | 


vim +7354 mm/slub.c

  7350	
  7351	/*
  7352	 * The /sys/module/slub ABI
  7353	 */
> 7354	#ifdef CONFIG_SLUB_LEAK_PANIC
  7355	/*
  7356	 * What:          /sys/module/slub/parameters/leak_panic
  7357	 *                /sys/module/slub/parameters/leak_panic_threshold
  7358	 * Date:          Sep 2024
  7359	 * KernelVersion: v6.6+
  7360	 * Description:   Used for slub memory leak check. When the user
  7361	 *                successfully allocates the slub page, it also performs
  7362	 *                statistics on the total slub usage in the system.
  7363	 *                When the usage exceeds the set value
  7364	 *                (threshold * memtotal / 100), it is considered that
  7365	 *                there is a risk of slub leakage in the system at this time.
  7366	 *                A panic operation will be triggered.
  7367	 * Users:         userspace
  7368	 */
  7369	MODULE_PARM_DESC(leak_panic, "Disable/Enable slub_leak_panic");
  7370	module_param_named(leak_panic, slub_leak_panic_enabled, bool, 0644);
  7371	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] mm/slub: Add panic function when slub leaks

[kernel test robot]

Hi Fangzheng,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v6.11 next-20240925]
[cannot apply to vbabka-slab/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Fangzheng-Zhang/mm-slub-Add-panic-function-when-slub-leaks/20240925-112601
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240925032256.1782-2-fangzheng.zhang%40unisoc.com
patch subject: [PATCH 1/2] mm/slub: Add panic function when slub leaks
config: x86_64-allnoconfig (https://download.01.org/0day-ci/archive/20240925/202409251929.fHee67vM-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240925/202409251929.fHee67vM-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409251929.fHee67vM-lkp@intel.com/

All errors (new ones prefixed by >>):

>> mm/slub.c:7354:2: error: unterminated conditional directive
    7354 | #ifdef CONFIG_SLUB_LEAK_PANIC
         |  ^
   1 error generated.


vim +7354 mm/slub.c

  7350	
  7351	/*
  7352	 * The /sys/module/slub ABI
  7353	 */
> 7354	#ifdef CONFIG_SLUB_LEAK_PANIC
  7355	/*
  7356	 * What:          /sys/module/slub/parameters/leak_panic
  7357	 *                /sys/module/slub/parameters/leak_panic_threshold
  7358	 * Date:          Sep 2024
  7359	 * KernelVersion: v6.6+
  7360	 * Description:   Used for slub memory leak check. When the user
  7361	 *                successfully allocates the slub page, it also performs
  7362	 *                statistics on the total slub usage in the system.
  7363	 *                When the usage exceeds the set value
  7364	 *                (threshold * memtotal / 100), it is considered that
  7365	 *                there is a risk of slub leakage in the system at this time.
  7366	 *                A panic operation will be triggered.
  7367	 * Users:         userspace
  7368	 */
  7369	MODULE_PARM_DESC(leak_panic, "Disable/Enable slub_leak_panic");
  7370	module_param_named(leak_panic, slub_leak_panic_enabled, bool, 0644);
  7371	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 2/2] Documentation: admin-guide: kernel-parameters: Add parameter description for slub_leak_panic function

[Fangzheng Zhang]

Introduce the details of the slub_leak_panic parameters in
kernel-parameters.txt, so that users can understand how to use
the parameters more quickly.

Signed-off-by: Fangzheng Zhang <fangzheng.zhang@unisoc.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index bb48ae24ae69..8a672766f05f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6130,6 +6130,21 @@
 			For more information see Documentation/mm/slub.rst.
 			(slub_nomerge legacy name also accepted for now)
 
+	slub.leak_panic= [MM, SLUB]
+			Enabling slub.leak_panic can monitor the slub usage
+			in real time on the slub allocation path. When the
+			slub occupancy is abnormal, the alarm or panic will
+			be triggered.
+			Default: N
+
+	slub.leak_panic_threshold= [MM, SLUB]
+			Upper limit value of the slub usage, expressed as
+			a percentage of memtotal. When the usage exceeds
+			the set value (threshold * memtotal / 100), the
+			slub_leak_panic function will be trigger. The
+			value setting range is [0, 100].
+			Default: 0
+
 	slram=		[HW,MTD]
 
 	smart2=		[HW]
-- 
2.17.1

Re: [PATCH 0/2] Introduce panic function when slub leaks

[Hyeonggon Yoo]

On Wed, Sep 25, 2024 at 12:23 PM Fangzheng Zhang
<fangzheng.zhang@unisoc.com> wrote:
>
> Hi all,

Hi Fangzheng,

> A method to detect slub leaks by monitoring its usage in real time
> on the page allocation path of the slub. When the slub occupancy
> exceeds the user-set value, it is considered that the slub is leaking
> at this time

I'm not sure why this should be a kernel feature. Why not write a user
script that parses
MemTotal: and Slab: part of /proc/meminfo file and generates a log
entry or an alarm?

> and a panic operation will be triggered immediately.

I don't think it would be a good idea to panic unnecessarily.
IMO it is not proper to panic when the kernel can still run.

Any thoughts?

Thanks,
Hyeonggon

Re: [PATCH 0/2] Introduce panic function when slub leaks

[Vlastimil Babka]

On 9/25/24 15:18, Hyeonggon Yoo wrote:
> On Wed, Sep 25, 2024 at 12:23 PM Fangzheng Zhang
> <fangzheng.zhang@unisoc.com> wrote:
>>
>> Hi all,
> 
> Hi Fangzheng,
> 
>> A method to detect slub leaks by monitoring its usage in real time
>> on the page allocation path of the slub. When the slub occupancy
>> exceeds the user-set value, it is considered that the slub is leaking
>> at this time
> 
> I'm not sure why this should be a kernel feature. Why not write a user
> script that parses
> MemTotal: and Slab: part of /proc/meminfo file and generates a log
> entry or an alarm?

Yes very much agreed. It seems rather arbitrary. Why slab, why not any other
kernel-specific counter in /proc/meminfo? Why include NR_SLAB_RECLAIMABLE_B
when that's used by caches with shrinkers?
A userspace solution should be straightforward and universal - easily
configurable for different scenarios.

>> and a panic operation will be triggered immediately.
> 
> I don't think it would be a good idea to panic unnecessarily.
> IMO it is not proper to panic when the kernel can still run.

Yes these days it's practically impossible to add a BUG_ON() for more
serious conditions than this.

Please don't post new versions addressing specific implementation details
until this fundamental issue is addressed.

Thanks,
Vlastimil

> Any thoughts?
> 
> Thanks,
> Hyeonggon

Re: [PATCH 0/2] Introduce panic function when slub leaks

[zhang fangzheng]

On Thu, Sep 26, 2024 at 8:30 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 9/25/24 15:18, Hyeonggon Yoo wrote:
> > On Wed, Sep 25, 2024 at 12:23 PM Fangzheng Zhang
> > <fangzheng.zhang@unisoc.com> wrote:
> >>
> >> Hi all,
> >
> > Hi Fangzheng,
> >
> >> A method to detect slub leaks by monitoring its usage in real time
> >> on the page allocation path of the slub. When the slub occupancy
> >> exceeds the user-set value, it is considered that the slub is leaking
> >> at this time
> >
> > I'm not sure why this should be a kernel feature. Why not write a user
> > script that parses
> > MemTotal: and Slab: part of /proc/meminfo file and generates a log
> > entry or an alarm?
>
> Yes very much agreed. It seems rather arbitrary. Why slab, why not any other
> kernel-specific counter in /proc/meminfo? Why include NR_SLAB_RECLAIMABLE_B
> when that's used by caches with shrinkers?

Ok, this is because the current consideration is to specifically
track the memory usage of the slab module.
In the stability test, ie, monkey test,
the anr or reboot problem occurs, there is a high probability
that the slab occupancy is high when it comes to memory analysis.
In addition to directly monitoring leaks in the allocation path, it is
also convenient to record the allocation stack information
when an exception occurs.

> A userspace solution should be straightforward and universal - easily
> configurable for different scenarios.
>
> >> and a panic operation will be triggered immediately.
> >
> > I don't think it would be a good idea to panic unnecessarily.
> > IMO it is not proper to panic when the kernel can still run.
>
> Yes these days it's practically impossible to add a BUG_ON() for more
> serious conditions than this.
>
> Please don't post new versions addressing specific implementation details
> until this fundamental issue is addressed.
>
> Thanks,
> Vlastimil
>
> > Any thoughts?
> >
> > Thanks,
> > Hyeonggon
>

Re: [PATCH 0/2] Introduce panic function when slub leaks

[Hyeonggon Yoo]

On Fri, Sep 27, 2024 at 4:28 PM zhang fangzheng
<fangzheng.zhang1003@gmail.com> wrote:
>
> On Thu, Sep 26, 2024 at 8:30 PM Vlastimil Babka <vbabka@suse.cz> wrote:
> >
> > On 9/25/24 15:18, Hyeonggon Yoo wrote:
> > > On Wed, Sep 25, 2024 at 12:23 PM Fangzheng Zhang
> > > <fangzheng.zhang@unisoc.com> wrote:
> > >>
> > >> Hi all,
> > >
> > > Hi Fangzheng,
> > >
> > >> A method to detect slub leaks by monitoring its usage in real time
> > >> on the page allocation path of the slub. When the slub occupancy
> > >> exceeds the user-set value, it is considered that the slub is leaking
> > >> at this time
> > >
> > > I'm not sure why this should be a kernel feature. Why not write a user
> > > script that parses
> > > MemTotal: and Slab: part of /proc/meminfo file and generates a log
> > > entry or an alarm?
> >
> > Yes very much agreed. It seems rather arbitrary. Why slab, why not any other
> > kernel-specific counter in /proc/meminfo? Why include NR_SLAB_RECLAIMABLE_B
> > when that's used by caches with shrinkers?
>
> Ok, this is because the current consideration is to specifically
> track the memory usage of the slab module.
> In the stability test, ie, monkey test,
> the anr or reboot problem occurs, there is a high probability
> that the slab occupancy is high when it comes to memory analysis.
> In addition to directly monitoring leaks in the allocation path, it is
> also convenient to record the allocation stack information
> when an exception occurs.

[+Cc Memory Allocation Profiling maintainers]

For recording allocation information, I think CONFIG_MEM_ALLOC_PROFILING [1] [2]
may be used to track allocation sites that contribute to memory leaks,
instead of making the kernel panic or printing WARNING?

.....Or with higher overhead, slub_debug=U [3] if it is not meant to
be run on production.

[1] https://docs.kernel.org/mm/allocation-profiling.html
[2] https://lwn.net/Articles/974380
[3] https://docs.kernel.org/mm/slub.html#debugfs-files-for-slub

Best,
Hyeonggon

> > A userspace solution should be straightforward and universal - easily
> > configurable for different scenarios.
> >
> > >> and a panic operation will be triggered immediately.
> > >
> > > I don't think it would be a good idea to panic unnecessarily.
> > > IMO it is not proper to panic when the kernel can still run.
> >
> > Yes these days it's practically impossible to add a BUG_ON() for more
> > serious conditions than this.
> >
> > Please don't post new versions addressing specific implementation details
> > until this fundamental issue is addressed.
> >
> > Thanks,
> > Vlastimil
> >
> > > Any thoughts?
> > >
> > > Thanks,
> > > Hyeonggon
> >

答复: [PATCH 0/2] Introduce panic function when slub leaks

[韩玉明 (Yuming Han)]

[-- Attachment #1: Type: text/plain, Size: 4814 bytes --]

?loop  shuo.tian

________________________________
发件人: Hyeonggon Yoo <42.hyeyoo@gmail.com>
发送时间: 2024年9月27日 16:01
收件人: zhang fangzheng
抄送: Vlastimil Babka; 张方正 (Fangzheng Zhang); Christoph Lameter; Pekka Enberg; David Rientjes; Joonsoo Kim; Andrew Morton; Roman Gushchin; Greg KH; linux-mm@kvack.org; linux-kernel@vger.kernel.org; tkjos@google.com; 韩玉明 (Yuming Han); Suren Baghdasaryan; Kent Overstreet
主题: Re: [PATCH 0/2] Introduce panic function when slub leaks


注意: 这封邮件来自于外部。除非你确定邮件内容安全，否则不要点击任何链接和附件。
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.



On Fri, Sep 27, 2024 at 4:28?PM zhang fangzheng
<fangzheng.zhang1003@gmail.com> wrote:
>
> On Thu, Sep 26, 2024 at 8:30?PM Vlastimil Babka <vbabka@suse.cz> wrote:
> >
> > On 9/25/24 15:18, Hyeonggon Yoo wrote:
> > > On Wed, Sep 25, 2024 at 12:23?PM Fangzheng Zhang
> > > <fangzheng.zhang@unisoc.com> wrote:
> > >>
> > >> Hi all,
> > >
> > > Hi Fangzheng,
> > >
> > >> A method to detect slub leaks by monitoring its usage in real time
> > >> on the page allocation path of the slub. When the slub occupancy
> > >> exceeds the user-set value, it is considered that the slub is leaking
> > >> at this time
> > >
> > > I'm not sure why this should be a kernel feature. Why not write a user
> > > script that parses
> > > MemTotal: and Slab: part of /proc/meminfo file and generates a log
> > > entry or an alarm?
> >
> > Yes very much agreed. It seems rather arbitrary. Why slab, why not any other
> > kernel-specific counter in /proc/meminfo? Why include NR_SLAB_RECLAIMABLE_B
> > when that's used by caches with shrinkers?
>
> Ok, this is because the current consideration is to specifically
> track the memory usage of the slab module.
> In the stability test, ie, monkey test,
> the anr or reboot problem occurs, there is a high probability
> that the slab occupancy is high when it comes to memory analysis.
> In addition to directly monitoring leaks in the allocation path, it is
> also convenient to record the allocation stack information
> when an exception occurs.

[+Cc Memory Allocation Profiling maintainers]

For recording allocation information, I think CONFIG_MEM_ALLOC_PROFILING [1] [2]
may be used to track allocation sites that contribute to memory leaks,
instead of making the kernel panic or printing WARNING?

.....Or with higher overhead, slub_debug=U [3] if it is not meant to
be run on production.

[1] https://docs.kernel.org/mm/allocation-profiling.html
[2] https://lwn.net/Articles/974380
[3] https://docs.kernel.org/mm/slub.html#debugfs-files-for-slub

Best,
Hyeonggon

> > A userspace solution should be straightforward and universal - easily
> > configurable for different scenarios.
> >
> > >> and a panic operation will be triggered immediately.
> > >
> > > I don't think it would be a good idea to panic unnecessarily.
> > > IMO it is not proper to panic when the kernel can still run.
> >
> > Yes these days it's practically impossible to add a BUG_ON() for more
> > serious conditions than this.
> >
> > Please don't post new versions addressing specific implementation details
> > until this fundamental issue is addressed.
> >
> > Thanks,
> > Vlastimil
> >
> > > Any thoughts?
> > >
> > > Thanks,
> > > Hyeonggon
> >
________________________________
This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。

[-- Attachment #2: Type: text/html, Size: 6671 bytes --]