linux-kernel - Re: [PATCH] slub: avoid scanning all partial slabs in get

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: <4a8aa758-175e-471c-9c22-bb61dbb956e8@suse.cz>
Date: Fri, 23 Feb 2024 10:17:34 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: Jianfeng Wang <jianfeng.w.wang@...cle.com>,
 "Christoph Lameter (Ampere)" <cl@...ux.com>,
 Chengming Zhou <chengming.zhou@...ux.dev>
Cc: David Rientjes <rientjes@...gle.com>, penberg@...nel.org,
 iamjoonsoo.kim@....com, akpm@...ux-foundation.org, roman.gushchin@...ux.dev,
 42.hyeyoo@...il.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 Chengming Zhou <zhouchengming@...edance.com>
Subject: Re: [PATCH] slub: avoid scanning all partial slabs in get_slabinfo()

On 2/23/24 08:36, Jianfeng Wang wrote:
> 
> On 2/22/24 7:02 PM, Christoph Lameter (Ampere) wrote:
>> On Thu, 22 Feb 2024, Chengming Zhou wrote:
>> 
>>> Anyway, I put the code below for discussion...
>> 
>> Can we guestimate the free objects based on the number of partial slabs. That number is available.
>> 
> 
> Yes.
> I've thought about calculating the average number of free objects in a
> partial slab (through sampling) and then estimating the total number of
> free objects as (avg * n->nr_partial).
> 
> See the following.
> 
> ---
>  mm/slub.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 63d281dfacdb..13385761049c 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2963,6 +2963,8 @@ static inline bool free_debug_processing(struct kmem_cache *s,
>  #endif /* CONFIG_SLUB_DEBUG */
>  
>  #if defined(CONFIG_SLUB_DEBUG) || defined(SLAB_SUPPORTS_SYSFS)
> +#define MAX_PARTIAL_TO_SCAN 10000
> +
>  static unsigned long count_partial(struct kmem_cache_node *n,
>  					int (*get_count)(struct slab *))
>  {
> @@ -2971,8 +2973,22 @@ static unsigned long count_partial(struct kmem_cache_node *n,
>  	struct slab *slab;
>  
>  	spin_lock_irqsave(&n->list_lock, flags);
> -	list_for_each_entry(slab, &n->partial, slab_list)
> -		x += get_count(slab);
> +	if (n->nr_partial > MAX_PARTIAL_TO_SCAN) {
> +		/* Estimate total count of objects via sampling */
> +		unsigned long sample_rate = n->nr_partial / MAX_PARTIAL_TO_SCAN;
> +		unsigned long scanned = 0;
> +		unsigned long counted = 0;
> +		list_for_each_entry(slab, &n->partial, slab_list) {

Unfortunately this is still going through the whole list.

> +			if (++scanned % sample_rate == 0) {
> +				x += get_count(slab);

The get_count() itself is trivial, so this won't help much.
(except saving a function call which can be noticeable with all the
retpolines and other unfortunate stuff these days; we could refactor the
code to be inline, but processing the whole long list would still be an issue).

So this would have to instead count first up to MAX_PARTIAL_TO_SCAN slabs
and then terminate the loop. This can make the sampling biased, unfortunately.

> +				counted++;
> +			}
> +		}
> +		x = mult_frac(x, n->nr_partial, counted);
> +	} else {
> +		list_for_each_entry(slab, &n->partial, slab_list)
> +			x += get_count(slab);
> +	}
>  	spin_unlock_irqrestore(&n->list_lock, flags);
>  	return x;
>  }