linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] mm/slab: Fix drain freelist excessively
@ 2013-06-24 10:23 Wanpeng Li
  2013-06-24 10:23 ` [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub Wanpeng Li
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Wanpeng Li @ 2013-06-24 10:23 UTC (permalink / raw)
  To: Pekka Enberg, Christoph Lameter, Matt Mackall
  Cc: Glauber Costa, Andrew Morton, Joonsoo Kim, David Rientjes,
	linux-mm, linux-kernel, 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.

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 be12f68..18628da 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.7.10.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-06-24 10:23 [PATCH 1/3] mm/slab: Fix drain freelist excessively Wanpeng Li
@ 2013-06-24 10:23 ` Wanpeng Li
  2013-06-24 21:23   ` David Rientjes
  2013-06-24 10:23 ` [PATCH 3/3] mm/slab: Fix /proc/slabinfo unwriteable for slab Wanpeng Li
  2013-07-01 15:46 ` [PATCH 1/3] mm/slab: Fix drain freelist excessively Christoph Lameter
  2 siblings, 1 reply; 19+ messages in thread
From: Wanpeng Li @ 2013-06-24 10:23 UTC (permalink / raw)
  To: Pekka Enberg, Christoph Lameter, Matt Mackall
  Cc: Glauber Costa, Andrew Morton, Joonsoo Kim, David Rientjes,
	linux-mm, linux-kernel, Wanpeng Li

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 |    4 ++--
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 18628da..fe68e60 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -4430,16 +4430,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..95c8860 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -271,3 +271,6 @@ struct kmem_cache_node {
 #endif
 
 };
+
+void *s_next(struct seq_file *m, void *p, loff_t *pos);
+void s_stop(struct seq_file *m, void *p);
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 2d41450..d161b81 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 *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)
+void s_stop(struct seq_file *m, void *p)
 {
 	mutex_unlock(&slab_mutex);
 }
-- 
1.7.10.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/3] mm/slab: Fix /proc/slabinfo unwriteable for slab
  2013-06-24 10:23 [PATCH 1/3] mm/slab: Fix drain freelist excessively Wanpeng Li
  2013-06-24 10:23 ` [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub Wanpeng Li
@ 2013-06-24 10:23 ` Wanpeng Li
  2013-07-01 15:49   ` Christoph Lameter
  2013-07-01 15:46 ` [PATCH 1/3] mm/slab: Fix drain freelist excessively Christoph Lameter
  2 siblings, 1 reply; 19+ messages in thread
From: Wanpeng Li @ 2013-06-24 10:23 UTC (permalink / raw)
  To: Pekka Enberg, Christoph Lameter, Matt Mackall
  Cc: Glauber Costa, Andrew Morton, Joonsoo Kim, David Rientjes,
	linux-mm, linux-kernel, 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 d161b81..7fdde79 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.7.10.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-06-24 10:23 ` [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub Wanpeng Li
@ 2013-06-24 21:23   ` David Rientjes
  2013-06-27  0:21     ` Wanpeng Li
                       ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: David Rientjes @ 2013-06-24 21:23 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: Pekka Enberg, Christoph Lameter, Matt Mackall, Glauber Costa,
	Andrew Morton, Joonsoo Kim, linux-mm, linux-kernel

On Mon, 24 Jun 2013, Wanpeng Li wrote:

> This patch shares s_next and s_stop between slab and slub.
> 

Just about the entire kernel includes slab.h, so I think you'll need to 
give these slab-specific names instead of exporting "s_next" and "s_stop" 
to everybody.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-06-24 21:23   ` David Rientjes
  2013-06-27  0:21     ` Wanpeng Li
@ 2013-06-27  0:21     ` Wanpeng Li
  2013-07-01 15:48     ` Christoph Lameter
  2 siblings, 0 replies; 19+ messages in thread
From: Wanpeng Li @ 2013-06-27  0:21 UTC (permalink / raw)
  To: David Rientjes
  Cc: Pekka Enberg, Christoph Lameter, Matt Mackall, Glauber Costa,
	Andrew Morton, Joonsoo Kim, linux-mm, linux-kernel

On Mon, Jun 24, 2013 at 02:23:11PM -0700, David Rientjes wrote:
>On Mon, 24 Jun 2013, Wanpeng Li wrote:
>
>> This patch shares s_next and s_stop between slab and slub.
>> 
>
>Just about the entire kernel includes slab.h, so I think you'll need to 
>give these slab-specific names instead of exporting "s_next" and "s_stop" 
>to everybody.

Ok, I will update them in next version, thanks for your review. ;-)

Regards,
Wanpeng Li 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-06-24 21:23   ` David Rientjes
@ 2013-06-27  0:21     ` Wanpeng Li
  2013-06-27  0:21     ` Wanpeng Li
  2013-07-01 15:48     ` Christoph Lameter
  2 siblings, 0 replies; 19+ messages in thread
From: Wanpeng Li @ 2013-06-27  0:21 UTC (permalink / raw)
  To: David Rientjes
  Cc: Pekka Enberg, Christoph Lameter, Matt Mackall, Glauber Costa,
	Andrew Morton, Joonsoo Kim, linux-mm, linux-kernel

On Mon, Jun 24, 2013 at 02:23:11PM -0700, David Rientjes wrote:
>On Mon, 24 Jun 2013, Wanpeng Li wrote:
>
>> This patch shares s_next and s_stop between slab and slub.
>> 
>
>Just about the entire kernel includes slab.h, so I think you'll need to 
>give these slab-specific names instead of exporting "s_next" and "s_stop" 
>to everybody.

Ok, I will update them in next version, thanks for your review. ;-)

Regards,
Wanpeng Li 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] mm/slab: Fix drain freelist excessively
  2013-06-24 10:23 [PATCH 1/3] mm/slab: Fix drain freelist excessively Wanpeng Li
  2013-06-24 10:23 ` [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub Wanpeng Li
  2013-06-24 10:23 ` [PATCH 3/3] mm/slab: Fix /proc/slabinfo unwriteable for slab Wanpeng Li
@ 2013-07-01 15:46 ` Christoph Lameter
  2013-07-01 23:45   ` Wanpeng Li
  2013-07-01 23:45   ` Wanpeng Li
  2 siblings, 2 replies; 19+ messages in thread
From: Christoph Lameter @ 2013-07-01 15:46 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: Pekka Enberg, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, David Rientjes, linux-mm, linux-kernel

On Mon, 24 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.

Well its intended to be the number of slabs to free. The patch does not
fix the callers that pass the number of slabs.

I think the best approach would be to fix the callers that pass # of
objects. Make sure they pass # of slabs.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-06-24 21:23   ` David Rientjes
  2013-06-27  0:21     ` Wanpeng Li
  2013-06-27  0:21     ` Wanpeng Li
@ 2013-07-01 15:48     ` Christoph Lameter
  2013-07-01 23:49       ` Wanpeng Li
                         ` (2 more replies)
  2 siblings, 3 replies; 19+ messages in thread
From: Christoph Lameter @ 2013-07-01 15:48 UTC (permalink / raw)
  To: David Rientjes
  Cc: Wanpeng Li, Pekka Enberg, Matt Mackall, Glauber Costa,
	Andrew Morton, Joonsoo Kim, linux-mm, linux-kernel

On Mon, 24 Jun 2013, David Rientjes wrote:

> On Mon, 24 Jun 2013, Wanpeng Li wrote:
>
> > This patch shares s_next and s_stop between slab and slub.
> >
>
> Just about the entire kernel includes slab.h, so I think you'll need to
> give these slab-specific names instead of exporting "s_next" and "s_stop"
> to everybody.

He put the export into mm/slab.h. The headerfile is only included by
mm/sl?b.c .

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/3] mm/slab: Fix /proc/slabinfo unwriteable for slab
  2013-06-24 10:23 ` [PATCH 3/3] mm/slab: Fix /proc/slabinfo unwriteable for slab Wanpeng Li
@ 2013-07-01 15:49   ` Christoph Lameter
  2013-07-01 23:43     ` Wanpeng Li
  2013-07-01 23:43     ` Wanpeng Li
  0 siblings, 2 replies; 19+ messages in thread
From: Christoph Lameter @ 2013-07-01 15:49 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: Pekka Enberg, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, David Rientjes, linux-mm, linux-kernel

On Mon, 24 Jun 2013, Wanpeng Li wrote:

>  1 file changed, 10 insertions(+)
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index d161b81..7fdde79 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;
>  }

It may be easier to define a macro SLABINFO_RIGHTS and use #ifdefs to
assign the correct one. That way we have only one slab_proc_init().

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/3] mm/slab: Fix /proc/slabinfo unwriteable for slab
  2013-07-01 15:49   ` Christoph Lameter
  2013-07-01 23:43     ` Wanpeng Li
@ 2013-07-01 23:43     ` Wanpeng Li
  1 sibling, 0 replies; 19+ messages in thread
From: Wanpeng Li @ 2013-07-01 23:43 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, David Rientjes, linux-mm, linux-kernel

On Mon, Jul 01, 2013 at 03:49:54PM +0000, Christoph Lameter wrote:
>On Mon, 24 Jun 2013, Wanpeng Li wrote:
>
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index d161b81..7fdde79 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;
>>  }
>
>It may be easier to define a macro SLABINFO_RIGHTS and use #ifdefs to
>assign the correct one. That way we have only one slab_proc_init().
>

Greate point! I	will do it in next version. ;-)

Regards,
Wanpeng Li 

>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/3] mm/slab: Fix /proc/slabinfo unwriteable for slab
  2013-07-01 15:49   ` Christoph Lameter
@ 2013-07-01 23:43     ` Wanpeng Li
  2013-07-01 23:43     ` Wanpeng Li
  1 sibling, 0 replies; 19+ messages in thread
From: Wanpeng Li @ 2013-07-01 23:43 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, David Rientjes, linux-mm, linux-kernel

On Mon, Jul 01, 2013 at 03:49:54PM +0000, Christoph Lameter wrote:
>On Mon, 24 Jun 2013, Wanpeng Li wrote:
>
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index d161b81..7fdde79 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;
>>  }
>
>It may be easier to define a macro SLABINFO_RIGHTS and use #ifdefs to
>assign the correct one. That way we have only one slab_proc_init().
>

Greate point! I	will do it in next version. ;-)

Regards,
Wanpeng Li 

>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] mm/slab: Fix drain freelist excessively
  2013-07-01 15:46 ` [PATCH 1/3] mm/slab: Fix drain freelist excessively Christoph Lameter
@ 2013-07-01 23:45   ` Wanpeng Li
  2013-07-01 23:45   ` Wanpeng Li
  1 sibling, 0 replies; 19+ messages in thread
From: Wanpeng Li @ 2013-07-01 23:45 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, David Rientjes, linux-mm, linux-kernel

On Mon, Jul 01, 2013 at 03:46:52PM +0000, Christoph Lameter wrote:
>On Mon, 24 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.
>
>Well its intended to be the number of slabs to free. The patch does not
>fix the callers that pass the number of slabs.
>
>I think the best approach would be to fix the callers that pass # of
>objects. Make sure they pass # of slabs.
>

Good point, I will fix it in next version.

Regards,
Wanpeng Li 

>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] mm/slab: Fix drain freelist excessively
  2013-07-01 15:46 ` [PATCH 1/3] mm/slab: Fix drain freelist excessively Christoph Lameter
  2013-07-01 23:45   ` Wanpeng Li
@ 2013-07-01 23:45   ` Wanpeng Li
  1 sibling, 0 replies; 19+ messages in thread
From: Wanpeng Li @ 2013-07-01 23:45 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, David Rientjes, linux-mm, linux-kernel

On Mon, Jul 01, 2013 at 03:46:52PM +0000, Christoph Lameter wrote:
>On Mon, 24 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.
>
>Well its intended to be the number of slabs to free. The patch does not
>fix the callers that pass the number of slabs.
>
>I think the best approach would be to fix the callers that pass # of
>objects. Make sure they pass # of slabs.
>

Good point, I will fix it in next version.

Regards,
Wanpeng Li 

>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-07-01 15:48     ` Christoph Lameter
  2013-07-01 23:49       ` Wanpeng Li
@ 2013-07-01 23:49       ` Wanpeng Li
  2013-07-07 16:41       ` Pekka Enberg
  2 siblings, 0 replies; 19+ messages in thread
From: Wanpeng Li @ 2013-07-01 23:49 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, linux-mm, linux-kernel

On Mon, Jul 01, 2013 at 03:48:09PM +0000, Christoph Lameter wrote:
>On Mon, 24 Jun 2013, David Rientjes wrote:
>
>> On Mon, 24 Jun 2013, Wanpeng Li wrote:
>>
>> > This patch shares s_next and s_stop between slab and slub.
>> >
>>
>> Just about the entire kernel includes slab.h, so I think you'll need to
>> give these slab-specific names instead of exporting "s_next" and "s_stop"
>> to everybody.
>
>He put the export into mm/slab.h. The headerfile is only included by
>mm/sl?b.c .

So I think this patch is ok, David? 

Regards,
Wanpeng Li 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-07-01 15:48     ` Christoph Lameter
@ 2013-07-01 23:49       ` Wanpeng Li
  2013-07-01 23:49       ` Wanpeng Li
  2013-07-07 16:41       ` Pekka Enberg
  2 siblings, 0 replies; 19+ messages in thread
From: Wanpeng Li @ 2013-07-01 23:49 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, linux-mm, linux-kernel

On Mon, Jul 01, 2013 at 03:48:09PM +0000, Christoph Lameter wrote:
>On Mon, 24 Jun 2013, David Rientjes wrote:
>
>> On Mon, 24 Jun 2013, Wanpeng Li wrote:
>>
>> > This patch shares s_next and s_stop between slab and slub.
>> >
>>
>> Just about the entire kernel includes slab.h, so I think you'll need to
>> give these slab-specific names instead of exporting "s_next" and "s_stop"
>> to everybody.
>
>He put the export into mm/slab.h. The headerfile is only included by
>mm/sl?b.c .

So I think this patch is ok, David? 

Regards,
Wanpeng Li 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-07-01 15:48     ` Christoph Lameter
  2013-07-01 23:49       ` Wanpeng Li
  2013-07-01 23:49       ` Wanpeng Li
@ 2013-07-07 16:41       ` Pekka Enberg
  2013-07-08  0:16         ` Wanpeng Li
  2013-07-08  0:16         ` Wanpeng Li
  2 siblings, 2 replies; 19+ messages in thread
From: Pekka Enberg @ 2013-07-07 16:41 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: David Rientjes, Wanpeng Li, Matt Mackall, Glauber Costa,
	Andrew Morton, Joonsoo Kim, linux-mm@kvack.org, LKML

On Mon, Jul 1, 2013 at 6:48 PM, Christoph Lameter <cl@linux.com> wrote:
> On Mon, 24 Jun 2013, David Rientjes wrote:
>
>> On Mon, 24 Jun 2013, Wanpeng Li wrote:
>>
>> > This patch shares s_next and s_stop between slab and slub.
>> >
>>
>> Just about the entire kernel includes slab.h, so I think you'll need to
>> give these slab-specific names instead of exporting "s_next" and "s_stop"
>> to everybody.
>
> He put the export into mm/slab.h. The headerfile is only included by
> mm/sl?b.c .

But he then went on to add globally visible symbols "s_next" and
"s_stop" which is bad...

Please send me an incremental patch on top of slab/next to fix this
up. Otherwise I'll revert it before sending a pull request to Linus.

                      Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-07-07 16:41       ` Pekka Enberg
  2013-07-08  0:16         ` Wanpeng Li
@ 2013-07-08  0:16         ` Wanpeng Li
  2013-07-08  8:03           ` Pekka Enberg
  1 sibling, 1 reply; 19+ messages in thread
From: Wanpeng Li @ 2013-07-08  0:16 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Christoph Lameter, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, linux-mm, linux-kernel

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

On Sun, Jul 07, 2013 at 07:41:54PM +0300, Pekka Enberg wrote:
>On Mon, Jul 1, 2013 at 6:48 PM, Christoph Lameter <cl@linux.com> wrote:
>> On Mon, 24 Jun 2013, David Rientjes wrote:
>>
>>> On Mon, 24 Jun 2013, Wanpeng Li wrote:
>>>
>>> > This patch shares s_next and s_stop between slab and slub.
>>> >
>>>
>>> Just about the entire kernel includes slab.h, so I think you'll need to
>>> give these slab-specific names instead of exporting "s_next" and "s_stop"
>>> to everybody.
>>
>> He put the export into mm/slab.h. The headerfile is only included by
>> mm/sl?b.c .
>
>But he then went on to add globally visible symbols "s_next" and
>"s_stop" which is bad...
>
>Please send me an incremental patch on top of slab/next to fix this
>up. Otherwise I'll revert it before sending a pull request to Linus.
>
>                      Pekka

Hi Pekka,

I attach the incremental patch in attachment. ;-)

Regards,
Wanpeng Li 


[-- Attachment #2: 0001-slab.patch --]
[-- Type: text/x-diff, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-07-07 16:41       ` Pekka Enberg
@ 2013-07-08  0:16         ` Wanpeng Li
  2013-07-08  0:16         ` Wanpeng Li
  1 sibling, 0 replies; 19+ messages in thread
From: Wanpeng Li @ 2013-07-08  0:16 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Christoph Lameter, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, linux-mm, linux-kernel

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

On Sun, Jul 07, 2013 at 07:41:54PM +0300, Pekka Enberg wrote:
>On Mon, Jul 1, 2013 at 6:48 PM, Christoph Lameter <cl@linux.com> wrote:
>> On Mon, 24 Jun 2013, David Rientjes wrote:
>>
>>> On Mon, 24 Jun 2013, Wanpeng Li wrote:
>>>
>>> > This patch shares s_next and s_stop between slab and slub.
>>> >
>>>
>>> Just about the entire kernel includes slab.h, so I think you'll need to
>>> give these slab-specific names instead of exporting "s_next" and "s_stop"
>>> to everybody.
>>
>> He put the export into mm/slab.h. The headerfile is only included by
>> mm/sl?b.c .
>
>But he then went on to add globally visible symbols "s_next" and
>"s_stop" which is bad...
>
>Please send me an incremental patch on top of slab/next to fix this
>up. Otherwise I'll revert it before sending a pull request to Linus.
>
>                      Pekka

Hi Pekka,

I attach the incremental patch in attachment. ;-)

Regards,
Wanpeng Li 


[-- Attachment #2: 0001-slab.patch --]
[-- Type: text/x-diff, Size: 2019 bytes --]

>From 02f008d7321a4a10babb4f7398b9eb8ebe13af24 Mon Sep 17 00:00:00 2001
From: Wanpeng Li <liwanp@linux.vnet.ibm.com>
Date: Mon, 8 Jul 2013 08:08:28 +0800
Subject: [PATCH] mm/slab: Give s_next and s_stop slab-specific names

Give s_next and s_stop slab-specific names instead of exporting 
"s_next" and "s_stop".

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

diff --git a/mm/slab.c b/mm/slab.c
index 9bf2251..57ab422 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -4440,8 +4440,8 @@ static int leaks_show(struct seq_file *m, void *p)
 
 static const struct seq_operations slabstats_op = {
 	.start = leaks_start,
-	.next = s_next,
-	.stop = s_stop,
+	.next = slab_next,
+	.stop = slab_stop,
 	.show = leaks_show,
 };
 
diff --git a/mm/slab.h b/mm/slab.h
index 95c8860..620ceed 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -272,5 +272,5 @@ struct kmem_cache_node {
 
 };
 
-void *s_next(struct seq_file *m, void *p, loff_t *pos);
-void s_stop(struct seq_file *m, void *p);
+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 13ae037..eacdffa 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -536,12 +536,12 @@ static void *s_start(struct seq_file *m, loff_t *pos)
 	return seq_list_start(&slab_caches, *pos);
 }
 
-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);
 }
 
-void s_stop(struct seq_file *m, void *p)
+void slab_stop(struct seq_file *m, void *p)
 {
 	mutex_unlock(&slab_mutex);
 }
@@ -618,8 +618,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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub
  2013-07-08  0:16         ` Wanpeng Li
@ 2013-07-08  8:03           ` Pekka Enberg
  0 siblings, 0 replies; 19+ messages in thread
From: Pekka Enberg @ 2013-07-08  8:03 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: Christoph Lameter, Matt Mackall, Glauber Costa, Andrew Morton,
	Joonsoo Kim, linux-mm, linux-kernel

On 07/08/2013 03:16 AM, Wanpeng Li wrote:
> On Sun, Jul 07, 2013 at 07:41:54PM +0300, Pekka Enberg wrote:
>> On Mon, Jul 1, 2013 at 6:48 PM, Christoph Lameter <cl@linux.com> wrote:
>>> On Mon, 24 Jun 2013, David Rientjes wrote:
>>>
>>>> On Mon, 24 Jun 2013, Wanpeng Li wrote:
>>>>
>>>>> This patch shares s_next and s_stop between slab and slub.
>>>>>
>>>>
>>>> Just about the entire kernel includes slab.h, so I think you'll need to
>>>> give these slab-specific names instead of exporting "s_next" and "s_stop"
>>>> to everybody.
>>>
>>> He put the export into mm/slab.h. The headerfile is only included by
>>> mm/sl?b.c .
>>
>> But he then went on to add globally visible symbols "s_next" and
>> "s_stop" which is bad...
>>
>> Please send me an incremental patch on top of slab/next to fix this
>> up. Otherwise I'll revert it before sending a pull request to Linus.
>
> I attach the incremental patch in attachment. ;-)

Applied, thanks!

			Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2013-07-08  8:03 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-24 10:23 [PATCH 1/3] mm/slab: Fix drain freelist excessively Wanpeng Li
2013-06-24 10:23 ` [PATCH 2/3] mm/slab: Sharing s_next and s_stop between slab and slub Wanpeng Li
2013-06-24 21:23   ` David Rientjes
2013-06-27  0:21     ` Wanpeng Li
2013-06-27  0:21     ` Wanpeng Li
2013-07-01 15:48     ` Christoph Lameter
2013-07-01 23:49       ` Wanpeng Li
2013-07-01 23:49       ` Wanpeng Li
2013-07-07 16:41       ` Pekka Enberg
2013-07-08  0:16         ` Wanpeng Li
2013-07-08  0:16         ` Wanpeng Li
2013-07-08  8:03           ` Pekka Enberg
2013-06-24 10:23 ` [PATCH 3/3] mm/slab: Fix /proc/slabinfo unwriteable for slab Wanpeng Li
2013-07-01 15:49   ` Christoph Lameter
2013-07-01 23:43     ` Wanpeng Li
2013-07-01 23:43     ` Wanpeng Li
2013-07-01 15:46 ` [PATCH 1/3] mm/slab: Fix drain freelist excessively Christoph Lameter
2013-07-01 23:45   ` Wanpeng Li
2013-07-01 23:45   ` Wanpeng Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).