linux-kernel - Re: [patch 1/3] slub: add per-cache slab thrash ratio

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: <alpine.DEB.2.00.0903302140250.27065@chino.kir.corp.google.com>
Date:	Mon, 30 Mar 2009 21:44:57 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Christoph Lameter <cl@...ux.com>
cc:	Pekka Enberg <penberg@...helsinki.fi>,
	Nick Piggin <nickpiggin@...oo.com.au>,
	Martin Bligh <mbligh@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [patch 1/3] slub: add per-cache slab thrash ratio

On Mon, 30 Mar 2009, David Rientjes wrote:

> The way the code is currently written, this acts as an initialization when 
> there was no previous object count (i.e. its coming from 
> kmem_cache_open()) and acts as an adjustment when there was a previous 
> count (i.e. /sys/kernel/slab/cache/order was changed).  The only way to 
> avoid adding this to calculate_sizes() would be to add logic to 
> order_store() to adjust the watermark when the order changes, but that 
> duplicates the same calculation that is required for initialization if 
> s->min_free_watermark does get a default value in kmem_cache_open() as 
> Pekka suggested.

I applied the following to the patchset so that the initialization of the 
watermark is always separate from the updating as a result of changing 
/sys/kernel/slab/cache/order.

Since the setting of a default watermark in kmem_cache_open() will require 
a calculation to find the corresponding min_free_watermark depending on 
the object size for a pre-defined default ratio, it will require the same 
calculation that is now in order_store(), but I agree it's simpler to 
understand and justifies the code duplication.
---
diff --git a/mm/slub.c b/mm/slub.c
index 76fa5a6..61ae612 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2187,7 +2187,6 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order)
 	unsigned long flags = s->flags;
 	unsigned long size = s->objsize;
 	unsigned long align = s->align;
-	u16 thrash_ratio = 0;
 	int order;
 
 	/*
@@ -2293,13 +2292,10 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order)
 	/*
 	 * Determine the number of objects per slab
 	 */
-	if (oo_objects(s->oo))
-		thrash_ratio = s->min_free_watermark * 100 / oo_objects(s->oo);
 	s->oo = oo_make(order, size);
 	s->min = oo_make(get_order(size), size);
 	if (oo_objects(s->oo) > oo_objects(s->max))
 		s->max = s->oo;
-	s->min_free_watermark = oo_objects(s->oo) * thrash_ratio / 100;
 
 	return !!oo_objects(s->oo);
 
@@ -3824,6 +3820,7 @@ static ssize_t order_store(struct kmem_cache *s,
 				const char *buf, size_t length)
 {
 	unsigned long order;
+	unsigned long thrash_ratio;
 	int err;
 
 	err = strict_strtoul(buf, 10, &order);
@@ -3833,7 +3830,9 @@ static ssize_t order_store(struct kmem_cache *s,
 	if (order > slub_max_order || order < slub_min_order)
 		return -EINVAL;
 
+	thrash_ratio = s->min_free_watermark * 100 / oo_objects(s->oo);
 	calculate_sizes(s, order);
+	s->min_free_watermark = oo_objects(s->oo) * thrash_ratio / 100;
 	return length;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/