[PATCH v2 1/3] mm/slab: Fix drain freelist excessively

[PATCH v2 1/3] mm/slab: Fix drain freelist excessively

[Wanpeng Li]

The drain_freelist is called to drain slabs_free lists for cache reap,
cache shrink, memory hotplug callback etc. The tofree parameter is the
number of slab objects to free instead of the number of slabs to free.
The parameter transfered from callers is n->free_objects or n->freelimit
+ 5 * (searchp->num - 1) / (5 * searchp->num), and both of them mean
the number of slabs objects. I add printk to dump drain information:

[  122.864255] tofree is 2, actually free is 52, cache size is 26

The number of objects which caller prefer to drain is 2, however, actually
52 objects are drained, this destroy the cache locality. This patch fix it
by compare the number of slabs objects which already drained instead of
compare the number of slabs to the number of slab objects prefer to drain.
After this patch applied, there is no excessive drain as dump information 
mentioned above.

Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
---
 mm/slab.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 8ccd296..f58eb0b 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2479,7 +2479,7 @@ static void drain_cpu_caches(struct kmem_cache *cachep)
 
 /*
  * Remove slabs from the list of free slabs.
- * Specify the number of slabs to drain in tofree.
+ * Specify the number of slab objects to drain in tofree.
  *
  * Returns the actual number of slabs released.
  */
@@ -2491,7 +2491,7 @@ static int drain_freelist(struct kmem_cache *cache,
 	struct slab *slabp;
 
 	nr_freed = 0;
-	while (nr_freed < tofree && !list_empty(&n->slabs_free)) {
+	while (nr_freed * cache->num < tofree && !list_empty(&n->slabs_free)) {
 
 		spin_lock_irq(&n->list_lock);
 		p = n->slabs_free.prev;
-- 
1.8.1.2

[PATCH v2 2/3] mm/slab: Sharing s_next and s_stop between slab and slub

[Wanpeng Li]

Changelog:
 v1 -> v2:
  * give these slab-specific names instead of exporting "s_next" and "s_stop"

This patch shares s_next and s_stop between slab and slub.

Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
---
 mm/slab.c        | 10 ----------
 mm/slab.h        |  3 +++
 mm/slab_common.c |  8 ++++----
 3 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index f58eb0b..f3f454ad 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -4431,16 +4431,6 @@ static int leaks_show(struct seq_file *m, void *p)
 	return 0;
 }
 
-static void *s_next(struct seq_file *m, void *p, loff_t *pos)
-{
-	return seq_list_next(p, &slab_caches, pos);
-}
-
-static void s_stop(struct seq_file *m, void *p)
-{
-	mutex_unlock(&slab_mutex);
-}
-
 static const struct seq_operations slabstats_op = {
 	.start = leaks_start,
 	.next = s_next,
diff --git a/mm/slab.h b/mm/slab.h
index f96b49e..620ceed 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -271,3 +271,6 @@ struct kmem_cache_node {
 #endif
 
 };
+
+void *slab_next(struct seq_file *m, void *p, loff_t *pos);
+void slab_stop(struct seq_file *m, void *p);
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 2d41450..4ee4325 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -531,12 +531,12 @@ static void *s_start(struct seq_file *m, loff_t *pos)
 	return seq_list_start(&slab_caches, *pos);
 }
 
-static void *s_next(struct seq_file *m, void *p, loff_t *pos)
+void *slab_next(struct seq_file *m, void *p, loff_t *pos)
 {
 	return seq_list_next(p, &slab_caches, pos);
 }
 
-static void s_stop(struct seq_file *m, void *p)
+void slab_stop(struct seq_file *m, void *p)
 {
 	mutex_unlock(&slab_mutex);
 }
@@ -613,8 +613,8 @@ static int s_show(struct seq_file *m, void *p)
  */
 static const struct seq_operations slabinfo_op = {
 	.start = s_start,
-	.next = s_next,
-	.stop = s_stop,
+	.next = slab_next,
+	.stop = slab_stop,
 	.show = s_show,
 };
 
-- 
1.8.1.2

[PATCH v2 3/3] mm/slab: Fix /proc/slabinfo unwriteable for slab

[Wanpeng Li]

Slab have some tunables like limit, batchcount, and sharedfactor can be
tuned through function slabinfo_write. Commit (b7454ad3: mm/sl[au]b: Move
slabinfo processing to slab_common.c) uncorrectly change /proc/slabinfo
unwriteable for slab, this patch fix it by revert to original mode.

Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
---
 mm/slab_common.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 4ee4325..d41ff76 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -631,10 +631,20 @@ static const struct file_operations proc_slabinfo_operations = {
 	.release	= seq_release,
 };
 
+#ifdef CONFIG_SLAB
+static int __init slab_proc_init(void)
+{
+	proc_create("slabinfo", S_IWUSR | S_IRUSR, NULL, &proc_slabinfo_operations);
+	return 0;
+}
+#endif
+#ifdef CONFIG_SLUB
 static int __init slab_proc_init(void)
 {
 	proc_create("slabinfo", S_IRUSR, NULL, &proc_slabinfo_operations);
 	return 0;
 }
+#endif
+
 module_init(slab_proc_init);
 #endif /* CONFIG_SLABINFO */
-- 
1.8.1.2

Re: [PATCH v2 1/3] mm/slab: Fix drain freelist excessively

[Christoph Lameter]

On Thu, 27 Jun 2013, Wanpeng Li wrote:

> The drain_freelist is called to drain slabs_free lists for cache reap,
> cache shrink, memory hotplug callback etc. The tofree parameter is the
> number of slab objects to free instead of the number of slabs to free.
> The parameter transfered from callers is n->free_objects or n->freelimit
> + 5 * (searchp->num - 1) / (5 * searchp->num), and both of them mean
> the number of slabs objects. I add printk to dump drain information:

See my reply to V1.

Re: [PATCH v2 3/3] mm/slab: Fix /proc/slabinfo unwriteable for slab

[Christoph Lameter]

On Thu, 27 Jun 2013, Wanpeng Li wrote:

> Slab have some tunables like limit, batchcount, and sharedfactor can be
> tuned through function slabinfo_write. Commit (b7454ad3: mm/sl[au]b: Move
> slabinfo processing to slab_common.c) uncorrectly change /proc/slabinfo
> unwriteable for slab, this patch fix it by revert to original mode.

See comment on V1.