[PATCH 0/6] refine and rename slub sysfs

[PATCH 0/6] refine and rename slub sysfs

[Wei Yang]

This patch serial could be divided into two parts.

First three patches refine and adds slab sysfs.
Second three patches rename slab sysfs.

1. Refine slab sysfs

There are four level slabs:

    CPU
    CPU_PARTIAL
    PARTIAL
    FULL

And in sysfs, it use show_slab_objects() and cpu_partial_slabs_show() to
reflect the statistics.

In patch 2, it splits some function in show_slab_objects() which makes sure
only cpu_partial_slabs_show() covers statistics for CPU_PARTIAL slabs.

After doing so, it would be more clear that show_slab_objects() has totally 9
statistic combinations for three level of slabs. Each slab has three cases
statistic.

    slabs
    objects
    total_objects

And when we look at current implementation, some of them are missing. So patch
2 & 3 add them up.

2. Rename sysfs

The slab statistics in sysfs are

    slabs
    objects
    total_objects
    cpu_slabs
    partial
    partial_objects
    cpu_partial_slabs

which is a little bit hard for users to understand. The second three patches
rename sysfs file in this pattern.

    xxx_slabs[[_total]_objects]

Finally it looks Like

    slabs
    slabs_objects
    slabs_total_objects
    cpu_slabs
    cpu_slabs_objects
    cpu_slabs_total_objects
    partial_slabs
    partial_slabs_objects
    partial_slabs_total_objects
    cpu_partial_slabs

Wei Yang (6):
  mm/slub: add total_objects_partial sysfs
  mm/slub: not include cpu_partial data in cpu_slabs sysfs
  mm/slub: add cpu_slabs_[total_]objects sysfs
  mm/slub: rename ALL slabs sysfs
  mm/slub: rename partial_slabs sysfs
  mm/slub: rename cpu_partial_slab sysfs

 mm/slub.c | 64 +++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

-- 
2.11.0

[PATCH 1/6] mm/slub: add total_objects_partial sysfs

[Wei Yang]

For partial slabs, show_slab_objects could display its total objects.

This patch just adds an entry to display it.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index a7a109247730..1100d2e75870 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4983,6 +4983,12 @@ static ssize_t objects_partial_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(objects_partial);
 
+static ssize_t total_objects_partial_show(struct kmem_cache *s, char *buf)
+{
+	return show_slab_objects(s, buf, SO_PARTIAL|SO_TOTAL);
+}
+SLAB_ATTR_RO(total_objects_partial);
+
 static ssize_t slabs_cpu_partial_show(struct kmem_cache *s, char *buf)
 {
 	int objects = 0;
@@ -5359,6 +5365,7 @@ static struct attribute *slab_attrs[] = {
 	&cpu_partial_attr.attr,
 	&objects_attr.attr,
 	&objects_partial_attr.attr,
+	&total_objects_partial_attr.attr,
 	&partial_attr.attr,
 	&cpu_slabs_attr.attr,
 	&ctor_attr.attr,
-- 
2.11.0

Re: [PATCH 1/6] mm/slub: add total_objects_partial sysfs

[Christoph Lameter]

On Wed, 17 May 2017, Wei Yang wrote:

> For partial slabs, show_slab_objects could display its total objects.
>
> This patch just adds an entry to display it.

Acked-by: Christoph Lameter <cl@linux.com>

[PATCH 2/6] mm/slub: not include cpu_partial data in cpu_slabs sysfs

[Wei Yang]

There are four level slabs:

    CPU
    CPU_PARTIAL
    PARTIAL
    FULL

In current implementation, cpu_slabs sysfs would give statistics including
the first two levels. While there is another sysfs entry cpu_partial_slabs
gives details on the second level slab statistics. Since each cpu has one
slab for the first level, the current cpu_slabs output is easy to be
calculated from cpu_partial_slabs.

This patch removes the cpu_partial data in cpu_slabs for more specific slab
statistics and leave room to retrieve objects and total objects on CPU
level in the future.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 1100d2e75870..c7dddf22829d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4762,19 +4762,6 @@ static ssize_t show_slab_objects(struct kmem_cache *s,
 
 			total += x;
 			nodes[node] += x;
-
-			page = slub_percpu_partial_read_once(c);
-			if (page) {
-				node = page_to_nid(page);
-				if (flags & SO_TOTAL)
-					WARN_ON_ONCE(1);
-				else if (flags & SO_OBJECTS)
-					WARN_ON_ONCE(1);
-				else
-					x = page->pages;
-				total += x;
-				nodes[node] += x;
-			}
 		}
 	}
 
-- 
2.11.0

[PATCH 3/6] mm/slub: add cpu_slabs_[total_]objects sysfs

[Wei Yang]

For cpu slabs, show_slab_objects could display statistics for objects.

This patch just adds an entry to reflect it.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index c7dddf22829d..f2f751e6cb96 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4958,6 +4958,18 @@ static ssize_t cpu_slabs_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(cpu_slabs);
 
+static ssize_t cpu_slabs_objects_show(struct kmem_cache *s, char *buf)
+{
+	return show_slab_objects(s, buf, SO_CPU|SO_OBJECTS);
+}
+SLAB_ATTR_RO(cpu_slabs_objects);
+
+static ssize_t cpu_slabs_total_objects_show(struct kmem_cache *s, char *buf)
+{
+	return show_slab_objects(s, buf, SO_CPU|SO_TOTAL);
+}
+SLAB_ATTR_RO(cpu_slabs_total_objects);
+
 static ssize_t objects_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_ALL|SO_OBJECTS);
@@ -5354,6 +5366,8 @@ static struct attribute *slab_attrs[] = {
 	&objects_partial_attr.attr,
 	&total_objects_partial_attr.attr,
 	&partial_attr.attr,
+	&cpu_slabs_objects_attr.attr,
+	&cpu_slabs_total_objects_attr.attr,
 	&cpu_slabs_attr.attr,
 	&ctor_attr.attr,
 	&aliases_attr.attr,
-- 
2.11.0

[PATCH 4/6] mm/slub: rename ALL slabs sysfs

[Wei Yang]

Apply the sysfs pattern

    xxx_slabs[[_total]_objects]

to ALL slabs.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index f2f751e6cb96..443dacbf214e 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4970,11 +4970,11 @@ static ssize_t cpu_slabs_total_objects_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(cpu_slabs_total_objects);
 
-static ssize_t objects_show(struct kmem_cache *s, char *buf)
+static ssize_t slabs_objects_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_ALL|SO_OBJECTS);
 }
-SLAB_ATTR_RO(objects);
+SLAB_ATTR_RO(slabs_objects);
 
 static ssize_t objects_partial_show(struct kmem_cache *s, char *buf)
 {
@@ -5069,11 +5069,11 @@ static ssize_t slabs_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(slabs);
 
-static ssize_t total_objects_show(struct kmem_cache *s, char *buf)
+static ssize_t slabs_total_objects_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_ALL|SO_TOTAL);
 }
-SLAB_ATTR_RO(total_objects);
+SLAB_ATTR_RO(slabs_total_objects);
 
 static ssize_t sanity_checks_show(struct kmem_cache *s, char *buf)
 {
@@ -5362,7 +5362,7 @@ static struct attribute *slab_attrs[] = {
 	&order_attr.attr,
 	&min_partial_attr.attr,
 	&cpu_partial_attr.attr,
-	&objects_attr.attr,
+	&slabs_objects_attr.attr,
 	&objects_partial_attr.attr,
 	&total_objects_partial_attr.attr,
 	&partial_attr.attr,
@@ -5379,7 +5379,7 @@ static struct attribute *slab_attrs[] = {
 	&reserved_attr.attr,
 	&slabs_cpu_partial_attr.attr,
 #ifdef CONFIG_SLUB_DEBUG
-	&total_objects_attr.attr,
+	&slabs_total_objects_attr.attr,
 	&slabs_attr.attr,
 	&sanity_checks_attr.attr,
 	&trace_attr.attr,
-- 
2.11.0

[PATCH 5/6] mm/slub: rename partial_slabs sysfs

[Wei Yang]

Apply the sysfs pattern

    xxx_slabs[[_total]_objects]

to PARTIAL slabs.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 443dacbf214e..eb0eaa0239fd 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4946,11 +4946,11 @@ static ssize_t aliases_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(aliases);
 
-static ssize_t partial_show(struct kmem_cache *s, char *buf)
+static ssize_t partial_slabs_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_PARTIAL);
 }
-SLAB_ATTR_RO(partial);
+SLAB_ATTR_RO(partial_slabs);
 
 static ssize_t cpu_slabs_show(struct kmem_cache *s, char *buf)
 {
@@ -4976,17 +4976,17 @@ static ssize_t slabs_objects_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(slabs_objects);
 
-static ssize_t objects_partial_show(struct kmem_cache *s, char *buf)
+static ssize_t partial_slabs_objects_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_PARTIAL|SO_OBJECTS);
 }
-SLAB_ATTR_RO(objects_partial);
+SLAB_ATTR_RO(partial_slabs_objects);
 
-static ssize_t total_objects_partial_show(struct kmem_cache *s, char *buf)
+static ssize_t partial_slabs_total_objects_show(struct kmem_cache *s, char *buf)
 {
 	return show_slab_objects(s, buf, SO_PARTIAL|SO_TOTAL);
 }
-SLAB_ATTR_RO(total_objects_partial);
+SLAB_ATTR_RO(partial_slabs_total_objects);
 
 static ssize_t slabs_cpu_partial_show(struct kmem_cache *s, char *buf)
 {
@@ -5363,9 +5363,9 @@ static struct attribute *slab_attrs[] = {
 	&min_partial_attr.attr,
 	&cpu_partial_attr.attr,
 	&slabs_objects_attr.attr,
-	&objects_partial_attr.attr,
-	&total_objects_partial_attr.attr,
-	&partial_attr.attr,
+	&partial_slabs_objects_attr.attr,
+	&partial_slabs_total_objects_attr.attr,
+	&partial_slabs_attr.attr,
 	&cpu_slabs_objects_attr.attr,
 	&cpu_slabs_total_objects_attr.attr,
 	&cpu_slabs_attr.attr,
-- 
2.11.0

[PATCH 6/6] mm/slub: rename cpu_partial_slab sysfs

[Wei Yang]

Apply the sysfs pattern

    xxx_slabs

to CPU_PARTIAL slabs.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/slub.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index eb0eaa0239fd..93ff334b725e 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4988,7 +4988,7 @@ static ssize_t partial_slabs_total_objects_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(partial_slabs_total_objects);
 
-static ssize_t slabs_cpu_partial_show(struct kmem_cache *s, char *buf)
+static ssize_t cpu_partial_slabs_show(struct kmem_cache *s, char *buf)
 {
 	int objects = 0;
 	int pages = 0;
@@ -5019,7 +5019,7 @@ static ssize_t slabs_cpu_partial_show(struct kmem_cache *s, char *buf)
 #endif
 	return len + sprintf(buf + len, "\n");
 }
-SLAB_ATTR_RO(slabs_cpu_partial);
+SLAB_ATTR_RO(cpu_partial_slabs);
 
 static ssize_t reclaim_account_show(struct kmem_cache *s, char *buf)
 {
@@ -5377,7 +5377,7 @@ static struct attribute *slab_attrs[] = {
 	&destroy_by_rcu_attr.attr,
 	&shrink_attr.attr,
 	&reserved_attr.attr,
-	&slabs_cpu_partial_attr.attr,
+	&cpu_partial_slabs_attr.attr,
 #ifdef CONFIG_SLUB_DEBUG
 	&slabs_total_objects_attr.attr,
 	&slabs_attr.attr,
-- 
2.11.0

Re: [PATCH 0/6] refine and rename slub sysfs

[Christoph Lameter]

On Wed, 17 May 2017, Wei Yang wrote:

> This patch serial could be divided into two parts.
>
> First three patches refine and adds slab sysfs.
> Second three patches rename slab sysfs.

These changes will break the slabinfo tool in linux/tools/vm/slabinfo.c.
Please update it as well.

> 1. Refine slab sysfs
>
> There are four level slabs:

levels? Maybe types of slabs?

>     CPU
>     CPU_PARTIAL
>     PARTIAL
>     FULL
>
> And in sysfs, it use show_slab_objects() and cpu_partial_slabs_show() to
> reflect the statistics.
>
> In patch 2, it splits some function in show_slab_objects() which makes sure
> only cpu_partial_slabs_show() covers statistics for CPU_PARTIAL slabs.
>
> After doing so, it would be more clear that show_slab_objects() has totally 9
> statistic combinations for three level of slabs. Each slab has three cases
> statistic.
>
>     slabs
>     objects
>     total_objects

That sounds good.

> which is a little bit hard for users to understand. The second three patches
> rename sysfs file in this pattern.
>
>     xxx_slabs[[_total]_objects]
>
> Finally it looks Like
>
>     slabs
>     slabs_objects
>     slabs_total_objects
>     cpu_slabs
>     cpu_slabs_objects
>     cpu_slabs_total_objects
>     partial_slabs
>     partial_slabs_objects
>     partial_slabs_total_objects
>     cpu_partial_slabs

Arent we missing:

cpu_partial_slabs_objects
cpu_partial_slabs_total_objects

And the partial slabs exclude the cpu slabs as well as the cpu_partial
slabs?

Could you add some documentation as well to explain the exact semantics?

Re: [PATCH 0/6] refine and rename slub sysfs

[Michal Hocko]

On Wed 17-05-17 22:11:40, Wei Yang wrote:
> This patch serial could be divided into two parts.
> 
> First three patches refine and adds slab sysfs.
> Second three patches rename slab sysfs.
> 
> 1. Refine slab sysfs
> 
> There are four level slabs:
> 
>     CPU
>     CPU_PARTIAL
>     PARTIAL
>     FULL
> 
> And in sysfs, it use show_slab_objects() and cpu_partial_slabs_show() to
> reflect the statistics.
> 
> In patch 2, it splits some function in show_slab_objects() which makes sure
> only cpu_partial_slabs_show() covers statistics for CPU_PARTIAL slabs.
> 
> After doing so, it would be more clear that show_slab_objects() has totally 9
> statistic combinations for three level of slabs. Each slab has three cases
> statistic.
> 
>     slabs
>     objects
>     total_objects
> 
> And when we look at current implementation, some of them are missing. So patch
> 2 & 3 add them up.
> 
> 2. Rename sysfs
> 
> The slab statistics in sysfs are
> 
>     slabs
>     objects
>     total_objects
>     cpu_slabs
>     partial
>     partial_objects
>     cpu_partial_slabs
> 
> which is a little bit hard for users to understand. The second three patches
> rename sysfs file in this pattern.
> 
>     xxx_slabs[[_total]_objects]
> 
> Finally it looks Like
> 
>     slabs
>     slabs_objects
>     slabs_total_objects
>     cpu_slabs
>     cpu_slabs_objects
>     cpu_slabs_total_objects
>     partial_slabs
>     partial_slabs_objects
>     partial_slabs_total_objects
>     cpu_partial_slabs

_Why_ do we need all this?
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/6] refine and rename slub sysfs

[Wei Yang]

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

On Thu, May 18, 2017 at 11:06:37AM +0200, Michal Hocko wrote:
>On Wed 17-05-17 22:11:40, Wei Yang wrote:
>> This patch serial could be divided into two parts.
>> 
>> First three patches refine and adds slab sysfs.
>> Second three patches rename slab sysfs.
>> 
>> 1. Refine slab sysfs
>> 
>> There are four level slabs:
>> 
>>     CPU
>>     CPU_PARTIAL
>>     PARTIAL
>>     FULL
>> 
>> And in sysfs, it use show_slab_objects() and cpu_partial_slabs_show() to
>> reflect the statistics.
>> 
>> In patch 2, it splits some function in show_slab_objects() which makes sure
>> only cpu_partial_slabs_show() covers statistics for CPU_PARTIAL slabs.
>> 
>> After doing so, it would be more clear that show_slab_objects() has totally 9
>> statistic combinations for three level of slabs. Each slab has three cases
>> statistic.
>> 
>>     slabs
>>     objects
>>     total_objects
>> 
>> And when we look at current implementation, some of them are missing. So patch
>> 2 & 3 add them up.
>> 
>> 2. Rename sysfs
>> 
>> The slab statistics in sysfs are
>> 
>>     slabs
>>     objects
>>     total_objects
>>     cpu_slabs
>>     partial
>>     partial_objects
>>     cpu_partial_slabs
>> 
>> which is a little bit hard for users to understand. The second three patches
>> rename sysfs file in this pattern.
>> 
>>     xxx_slabs[[_total]_objects]
>> 
>> Finally it looks Like
>> 
>>     slabs
>>     slabs_objects
>>     slabs_total_objects
>>     cpu_slabs
>>     cpu_slabs_objects
>>     cpu_slabs_total_objects
>>     partial_slabs
>>     partial_slabs_objects
>>     partial_slabs_total_objects
>>     cpu_partial_slabs
>
>_Why_ do we need all this?

To have a clear statistics for each slab level.

>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/6] refine and rename slub sysfs

[Michal Hocko]

On Tue 23-05-17 11:27:05, Wei Yang wrote:
> On Thu, May 18, 2017 at 11:06:37AM +0200, Michal Hocko wrote:
> >On Wed 17-05-17 22:11:40, Wei Yang wrote:
> >> This patch serial could be divided into two parts.
> >> 
> >> First three patches refine and adds slab sysfs.
> >> Second three patches rename slab sysfs.
> >> 
> >> 1. Refine slab sysfs
> >> 
> >> There are four level slabs:
> >> 
> >>     CPU
> >>     CPU_PARTIAL
> >>     PARTIAL
> >>     FULL
> >> 
> >> And in sysfs, it use show_slab_objects() and cpu_partial_slabs_show() to
> >> reflect the statistics.
> >> 
> >> In patch 2, it splits some function in show_slab_objects() which makes sure
> >> only cpu_partial_slabs_show() covers statistics for CPU_PARTIAL slabs.
> >> 
> >> After doing so, it would be more clear that show_slab_objects() has totally 9
> >> statistic combinations for three level of slabs. Each slab has three cases
> >> statistic.
> >> 
> >>     slabs
> >>     objects
> >>     total_objects
> >> 
> >> And when we look at current implementation, some of them are missing. So patch
> >> 2 & 3 add them up.
> >> 
> >> 2. Rename sysfs
> >> 
> >> The slab statistics in sysfs are
> >> 
> >>     slabs
> >>     objects
> >>     total_objects
> >>     cpu_slabs
> >>     partial
> >>     partial_objects
> >>     cpu_partial_slabs
> >> 
> >> which is a little bit hard for users to understand. The second three patches
> >> rename sysfs file in this pattern.
> >> 
> >>     xxx_slabs[[_total]_objects]
> >> 
> >> Finally it looks Like
> >> 
> >>     slabs
> >>     slabs_objects
> >>     slabs_total_objects
> >>     cpu_slabs
> >>     cpu_slabs_objects
> >>     cpu_slabs_total_objects
> >>     partial_slabs
> >>     partial_slabs_objects
> >>     partial_slabs_total_objects
> >>     cpu_partial_slabs
> >
> >_Why_ do we need all this?
> 
> To have a clear statistics for each slab level.

Is this worth risking breakage of the userspace which consume this data
now? Do you have any user space code which will greatly benefit from the
new data and which couldn't do the same with the current format/output?

If yes this all should be in the changelog.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/6] refine and rename slub sysfs

[Christoph Lameter]

On Tue, 23 May 2017, Michal Hocko wrote:

> > >_Why_ do we need all this?
> >
> > To have a clear statistics for each slab level.
>
> Is this worth risking breakage of the userspace which consume this data
> now? Do you have any user space code which will greatly benefit from the
> new data and which couldn't do the same with the current format/output?
>
> If yes this all should be in the changelog.

And the patchset would also need to update the user space tool that is in
the kernel tree...

Again Wei please do not use "level". Type?

Re: [PATCH 0/6] refine and rename slub sysfs

[Wei Yang]

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

On Tue, May 23, 2017 at 08:39:11AM +0200, Michal Hocko wrote:
>On Tue 23-05-17 11:27:05, Wei Yang wrote:
>> On Thu, May 18, 2017 at 11:06:37AM +0200, Michal Hocko wrote:
>> >On Wed 17-05-17 22:11:40, Wei Yang wrote:
>> >> This patch serial could be divided into two parts.
>> >> 
>> >> First three patches refine and adds slab sysfs.
>> >> Second three patches rename slab sysfs.
>> >> 
>> >> 1. Refine slab sysfs
>> >> 
>> >> There are four level slabs:
>> >> 
>> >>     CPU
>> >>     CPU_PARTIAL
>> >>     PARTIAL
>> >>     FULL
>> >> 
>> >> And in sysfs, it use show_slab_objects() and cpu_partial_slabs_show() to
>> >> reflect the statistics.
>> >> 
>> >> In patch 2, it splits some function in show_slab_objects() which makes sure
>> >> only cpu_partial_slabs_show() covers statistics for CPU_PARTIAL slabs.
>> >> 
>> >> After doing so, it would be more clear that show_slab_objects() has totally 9
>> >> statistic combinations for three level of slabs. Each slab has three cases
>> >> statistic.
>> >> 
>> >>     slabs
>> >>     objects
>> >>     total_objects
>> >> 
>> >> And when we look at current implementation, some of them are missing. So patch
>> >> 2 & 3 add them up.
>> >> 
>> >> 2. Rename sysfs
>> >> 
>> >> The slab statistics in sysfs are
>> >> 
>> >>     slabs
>> >>     objects
>> >>     total_objects
>> >>     cpu_slabs
>> >>     partial
>> >>     partial_objects
>> >>     cpu_partial_slabs
>> >> 
>> >> which is a little bit hard for users to understand. The second three patches
>> >> rename sysfs file in this pattern.
>> >> 
>> >>     xxx_slabs[[_total]_objects]
>> >> 
>> >> Finally it looks Like
>> >> 
>> >>     slabs
>> >>     slabs_objects
>> >>     slabs_total_objects
>> >>     cpu_slabs
>> >>     cpu_slabs_objects
>> >>     cpu_slabs_total_objects
>> >>     partial_slabs
>> >>     partial_slabs_objects
>> >>     partial_slabs_total_objects
>> >>     cpu_partial_slabs
>> >
>> >_Why_ do we need all this?
>> 
>> To have a clear statistics for each slab level.
>
>Is this worth risking breakage of the userspace which consume this data
>now? Do you have any user space code which will greatly benefit from the
>new data and which couldn't do the same with the current format/output?
>
>If yes this all should be in the changelog.

The answer is no.

I have the same concern as yours. So this patch set could be divided into two
parts: 1. add some new entry with current name convention, 2. change the name
convention.

If there are many userspace tools use these entries, the changing is really
risky, I agree. Hmm, I still send this out, since current name convention is a
little difficult for users to understand, especially after we have several
levels slabs. Is it possible to use the name convention I proposed and add
link to them to keep the userspace interface?

And the second part is to fully utilize current functions. In function
show_slab_objects(), we have 9 combinations of slab statistics. 3 for each
slab level. And currently code just enable 6 of them. So the first three tries
to enable the missing 3 to make it a more complete statistics.

BTW, I found we don't have any entry for full slabs statistics. Not sure this
is omitted intendedly or not. If the community agrees, I still have a path to
enable the statistics for full slabs.

Thanks for your comments~ Michal

>
>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/6] refine and rename slub sysfs

[Michal Hocko]

On Wed 24-05-17 17:54:50, Wei Yang wrote:
> On Tue, May 23, 2017 at 08:39:11AM +0200, Michal Hocko wrote:
[...]
> >Is this worth risking breakage of the userspace which consume this data
> >now? Do you have any user space code which will greatly benefit from the
> >new data and which couldn't do the same with the current format/output?
> >
> >If yes this all should be in the changelog.
> 
> The answer is no.
> 
> I have the same concern as yours. So this patch set could be divided into two
> parts: 1. add some new entry with current name convention, 2. change the name
> convention.

Who is going to use those new entries and for what purpose? Why do we
want to expose even more details of the slab allocator to the userspace.
Is the missing information something fundamental that some user space
cannot work without it? Seriously these are essential questions you
should have answer for _before_ posting the patch and mention all those
reasons in the changelog.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 0/6] refine and rename slub sysfs

[Wei Yang]

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

On Wed, May 24, 2017 at 02:03:18PM +0200, Michal Hocko wrote:
>On Wed 24-05-17 17:54:50, Wei Yang wrote:
>> On Tue, May 23, 2017 at 08:39:11AM +0200, Michal Hocko wrote:
>[...]
>> >Is this worth risking breakage of the userspace which consume this data
>> >now? Do you have any user space code which will greatly benefit from the
>> >new data and which couldn't do the same with the current format/output?
>> >
>> >If yes this all should be in the changelog.
>> 
>> The answer is no.
>> 
>> I have the same concern as yours. So this patch set could be divided into two
>> parts: 1. add some new entry with current name convention, 2. change the name
>> convention.
>
>Who is going to use those new entries and for what purpose? Why do we
>want to expose even more details of the slab allocator to the userspace.
>Is the missing information something fundamental that some user space
>cannot work without it? Seriously these are essential questions you
>should have answer for _before_ posting the patch and mention all those
>reasons in the changelog.

It is me who wants to get more details of the slub behavior.  
AFAIK, no one else is expecting this.

Hmm, if we really don't want to export these entries, why not remove related
code? Looks we are sure they will not be touched.

>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/6] refine and rename slub sysfs

[Christoph Lameter]

On Wed, 24 May 2017, Wei Yang wrote:

> >
> >Who is going to use those new entries and for what purpose? Why do we
> >want to expose even more details of the slab allocator to the userspace.
> >Is the missing information something fundamental that some user space
> >cannot work without it? Seriously these are essential questions you
> >should have answer for _before_ posting the patch and mention all those
> >reasons in the changelog.
>
> It is me who wants to get more details of the slub behavior.
> AFAIK, no one else is expecting this.

I would appreciate some clearer structured statistics. These are important
for diagnostics and for debugging. Do not go overboard with this. Respin
it and provide also a cleanup of the slabinfo tool? I would appreciate it.

> Hmm, if we really don't want to export these entries, why not remove related
> code? Looks we are sure they will not be touched.

Please have a look at the slabinfo code which depends on those fields in
order to display slab information. I have patchsets here that will add
more functionality to slab and those will also add additional fields to
sysfs.

Re: [PATCH 0/6] refine and rename slub sysfs

[Michal Hocko]

On Wed 24-05-17 23:21:24, Wei Yang wrote:
> On Wed, May 24, 2017 at 02:03:18PM +0200, Michal Hocko wrote:
> >On Wed 24-05-17 17:54:50, Wei Yang wrote:
> >> On Tue, May 23, 2017 at 08:39:11AM +0200, Michal Hocko wrote:
> >[...]
> >> >Is this worth risking breakage of the userspace which consume this data
> >> >now? Do you have any user space code which will greatly benefit from the
> >> >new data and which couldn't do the same with the current format/output?
> >> >
> >> >If yes this all should be in the changelog.
> >> 
> >> The answer is no.
> >> 
> >> I have the same concern as yours. So this patch set could be divided into two
> >> parts: 1. add some new entry with current name convention, 2. change the name
> >> convention.
> >
> >Who is going to use those new entries and for what purpose? Why do we
> >want to expose even more details of the slab allocator to the userspace.
> >Is the missing information something fundamental that some user space
> >cannot work without it? Seriously these are essential questions you
> >should have answer for _before_ posting the patch and mention all those
> >reasons in the changelog.
> 
> It is me who wants to get more details of the slub behavior.  
> AFAIK, no one else is expecting this.

My point is that whatever the reason is, it _should_ be described
properly. This is a user visible change and we will have hard time to
change in future once there is userspace depending on it. So ask
yourself, is this so useful that the future maintenance will be still
reasonable? Also doesn't this export too much of the internal
implementation details that would make future development harder?
Also make sure to CC linux-api mailing list for future posts which
involve user API visible changes.

Thanks!
-- 
Michal Hocko
SUSE Labs