* [patch 2/2]slub: add a type for slab partial list position
@ 2011-08-23 0:37 ` Shaohua Li
0 siblings, 0 replies; 14+ messages in thread
From: Shaohua Li @ 2011-08-23 0:37 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-mm, lkml, cl, penberg, Shi, Alex, Chen, Tim C
Adding slab to partial list head/tail is sensentive to performance.
So adding a type to document it to avoid we get it wrong.
Signed-off-by: Shaohua Li <shli@kernel.org>
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
mm/slub.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
Index: linux/mm/slub.c
===================================================================
--- linux.orig/mm/slub.c 2011-08-23 08:20:24.000000000 +0800
+++ linux/mm/slub.c 2011-08-23 08:34:29.000000000 +0800
@@ -1525,16 +1525,21 @@ static void discard_slab(struct kmem_cac
free_slab(s, page);
}
+enum partial_list_position {
+ PARTIAL_LIST_HEAD,
+ PARTIAL_LIST_TAIL,
+};
+
/*
* Management of partially allocated slabs.
*
* list_lock must be held.
*/
static inline void add_partial(struct kmem_cache_node *n,
- struct page *page, int tail)
+ struct page *page, enum partial_list_position tail)
{
n->nr_partial++;
- if (tail)
+ if (tail == PARTIAL_LIST_TAIL)
list_add_tail(&page->lru, &n->partial);
else
list_add(&page->lru, &n->partial);
@@ -1781,13 +1786,13 @@ static void deactivate_slab(struct kmem_
enum slab_modes l = M_NONE, m = M_NONE;
void *freelist;
void *nextfree;
- int tail = 0;
+ enum partial_list_position tail = PARTIAL_LIST_HEAD;
struct page new;
struct page old;
if (page->freelist) {
stat(s, DEACTIVATE_REMOTE_FREES);
- tail = 1;
+ tail = PARTIAL_LIST_TAIL;
}
c->tid = next_tid(c->tid);
@@ -1893,7 +1898,8 @@ redo:
if (m == M_PARTIAL) {
add_partial(n, page, tail);
- stat(s, tail ? DEACTIVATE_TO_TAIL : DEACTIVATE_TO_HEAD);
+ stat(s, tail == PARTIAL_LIST_TAIL ? DEACTIVATE_TO_TAIL
+ : DEACTIVATE_TO_HEAD);
} else if (m == M_FULL) {
@@ -2377,7 +2383,7 @@ static void __slab_free(struct kmem_cach
*/
if (unlikely(!prior)) {
remove_full(s, page);
- add_partial(n, page, 1);
+ add_partial(n, page, PARTIAL_LIST_TAIL);
stat(s, FREE_ADD_PARTIAL);
}
}
@@ -2695,7 +2701,7 @@ static void early_kmem_cache_node_alloc(
init_kmem_cache_node(n, kmem_cache_node);
inc_slabs_node(kmem_cache_node, node, page->objects);
- add_partial(n, page, 0);
+ add_partial(n, page, PARTIAL_LIST_HEAD);
}
static void free_kmem_cache_nodes(struct kmem_cache *s)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [patch 2/2]slub: add a type for slab partial list position
@ 2011-08-23 0:37 ` Shaohua Li
0 siblings, 0 replies; 14+ messages in thread
From: Shaohua Li @ 2011-08-23 0:37 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-mm, lkml, cl, penberg, Shi, Alex, Chen, Tim C
Adding slab to partial list head/tail is sensentive to performance.
So adding a type to document it to avoid we get it wrong.
Signed-off-by: Shaohua Li <shli@kernel.org>
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
mm/slub.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
Index: linux/mm/slub.c
===================================================================
--- linux.orig/mm/slub.c 2011-08-23 08:20:24.000000000 +0800
+++ linux/mm/slub.c 2011-08-23 08:34:29.000000000 +0800
@@ -1525,16 +1525,21 @@ static void discard_slab(struct kmem_cac
free_slab(s, page);
}
+enum partial_list_position {
+ PARTIAL_LIST_HEAD,
+ PARTIAL_LIST_TAIL,
+};
+
/*
* Management of partially allocated slabs.
*
* list_lock must be held.
*/
static inline void add_partial(struct kmem_cache_node *n,
- struct page *page, int tail)
+ struct page *page, enum partial_list_position tail)
{
n->nr_partial++;
- if (tail)
+ if (tail == PARTIAL_LIST_TAIL)
list_add_tail(&page->lru, &n->partial);
else
list_add(&page->lru, &n->partial);
@@ -1781,13 +1786,13 @@ static void deactivate_slab(struct kmem_
enum slab_modes l = M_NONE, m = M_NONE;
void *freelist;
void *nextfree;
- int tail = 0;
+ enum partial_list_position tail = PARTIAL_LIST_HEAD;
struct page new;
struct page old;
if (page->freelist) {
stat(s, DEACTIVATE_REMOTE_FREES);
- tail = 1;
+ tail = PARTIAL_LIST_TAIL;
}
c->tid = next_tid(c->tid);
@@ -1893,7 +1898,8 @@ redo:
if (m == M_PARTIAL) {
add_partial(n, page, tail);
- stat(s, tail ? DEACTIVATE_TO_TAIL : DEACTIVATE_TO_HEAD);
+ stat(s, tail == PARTIAL_LIST_TAIL ? DEACTIVATE_TO_TAIL
+ : DEACTIVATE_TO_HEAD);
} else if (m == M_FULL) {
@@ -2377,7 +2383,7 @@ static void __slab_free(struct kmem_cach
*/
if (unlikely(!prior)) {
remove_full(s, page);
- add_partial(n, page, 1);
+ add_partial(n, page, PARTIAL_LIST_TAIL);
stat(s, FREE_ADD_PARTIAL);
}
}
@@ -2695,7 +2701,7 @@ static void early_kmem_cache_node_alloc(
init_kmem_cache_node(n, kmem_cache_node);
inc_slabs_node(kmem_cache_node, node, page->objects);
- add_partial(n, page, 0);
+ add_partial(n, page, PARTIAL_LIST_HEAD);
}
static void free_kmem_cache_nodes(struct kmem_cache *s)
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
2011-08-23 0:37 ` Shaohua Li
@ 2011-08-23 15:25 ` Christoph Lameter
-1 siblings, 0 replies; 14+ messages in thread
From: Christoph Lameter @ 2011-08-23 15:25 UTC (permalink / raw)
To: Shaohua Li; +Cc: Andrew Morton, linux-mm, lkml, penberg, Shi, Alex, Chen, Tim C
On Tue, 23 Aug 2011, Shaohua Li wrote:
> Adding slab to partial list head/tail is sensentive to performance.
> So adding a type to document it to avoid we get it wrong.
I think that if you want to make it more descriptive then using the stats
values (DEACTIVATE_TO_TAIL/HEAD) would avoid having to introduce an
additional enum and it would also avoid the if statement in the stat call.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
@ 2011-08-23 15:25 ` Christoph Lameter
0 siblings, 0 replies; 14+ messages in thread
From: Christoph Lameter @ 2011-08-23 15:25 UTC (permalink / raw)
To: Shaohua Li; +Cc: Andrew Morton, linux-mm, lkml, penberg, Shi, Alex, Chen, Tim C
On Tue, 23 Aug 2011, Shaohua Li wrote:
> Adding slab to partial list head/tail is sensentive to performance.
> So adding a type to document it to avoid we get it wrong.
I think that if you want to make it more descriptive then using the stats
values (DEACTIVATE_TO_TAIL/HEAD) would avoid having to introduce an
additional enum and it would also avoid the if statement in the stat call.
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
2011-08-23 15:25 ` Christoph Lameter
@ 2011-08-24 0:57 ` Shaohua Li
-1 siblings, 0 replies; 14+ messages in thread
From: Shaohua Li @ 2011-08-24 0:57 UTC (permalink / raw)
To: Christoph Lameter
Cc: Andrew Morton, linux-mm, lkml, penberg, Shi, Alex, Chen, Tim C
On Tue, 2011-08-23 at 23:25 +0800, Christoph Lameter wrote:
> On Tue, 23 Aug 2011, Shaohua Li wrote:
>
> > Adding slab to partial list head/tail is sensentive to performance.
> > So adding a type to document it to avoid we get it wrong.
>
> I think that if you want to make it more descriptive then using the stats
> values (DEACTIVATE_TO_TAIL/HEAD) would avoid having to introduce an
> additional enum and it would also avoid the if statement in the stat call.
ok, that's better.
Subject: slub: explicitly document position of inserting slab to partial list
Adding slab to partial list head/tail is sensitive to performance.
So explicitly uses DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD to document
it to avoid we get it wrong.
Signed-off-by: Shaohua Li <shli@kernel.org>
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
mm/slub.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Index: linux/mm/slub.c
===================================================================
--- linux.orig/mm/slub.c 2011-08-24 08:46:27.000000000 +0800
+++ linux/mm/slub.c 2011-08-24 08:49:41.000000000 +0800
@@ -1534,7 +1534,7 @@ static inline void add_partial(struct km
struct page *page, int tail)
{
n->nr_partial++;
- if (tail)
+ if (tail == DEACTIVATE_TO_TAIL)
list_add_tail(&page->lru, &n->partial);
else
list_add(&page->lru, &n->partial);
@@ -1781,13 +1781,13 @@ static void deactivate_slab(struct kmem_
enum slab_modes l = M_NONE, m = M_NONE;
void *freelist;
void *nextfree;
- int tail = 0;
+ int tail = DEACTIVATE_TO_HEAD;
struct page new;
struct page old;
if (page->freelist) {
stat(s, DEACTIVATE_REMOTE_FREES);
- tail = 1;
+ tail = DEACTIVATE_TO_TAIL;
}
c->tid = next_tid(c->tid);
@@ -1893,7 +1893,7 @@ redo:
if (m == M_PARTIAL) {
add_partial(n, page, tail);
- stat(s, tail ? DEACTIVATE_TO_TAIL : DEACTIVATE_TO_HEAD);
+ stat(s, tail);
} else if (m == M_FULL) {
@@ -2377,7 +2377,7 @@ static void __slab_free(struct kmem_cach
*/
if (unlikely(!prior)) {
remove_full(s, page);
- add_partial(n, page, 1);
+ add_partial(n, page, DEACTIVATE_TO_TAIL);
stat(s, FREE_ADD_PARTIAL);
}
}
@@ -2695,7 +2695,7 @@ static void early_kmem_cache_node_alloc(
init_kmem_cache_node(n, kmem_cache_node);
inc_slabs_node(kmem_cache_node, node, page->objects);
- add_partial(n, page, 0);
+ add_partial(n, page, DEACTIVATE_TO_HEAD);
}
static void free_kmem_cache_nodes(struct kmem_cache *s)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
@ 2011-08-24 0:57 ` Shaohua Li
0 siblings, 0 replies; 14+ messages in thread
From: Shaohua Li @ 2011-08-24 0:57 UTC (permalink / raw)
To: Christoph Lameter
Cc: Andrew Morton, linux-mm, lkml, penberg, Shi, Alex, Chen, Tim C
On Tue, 2011-08-23 at 23:25 +0800, Christoph Lameter wrote:
> On Tue, 23 Aug 2011, Shaohua Li wrote:
>
> > Adding slab to partial list head/tail is sensentive to performance.
> > So adding a type to document it to avoid we get it wrong.
>
> I think that if you want to make it more descriptive then using the stats
> values (DEACTIVATE_TO_TAIL/HEAD) would avoid having to introduce an
> additional enum and it would also avoid the if statement in the stat call.
ok, that's better.
Subject: slub: explicitly document position of inserting slab to partial list
Adding slab to partial list head/tail is sensitive to performance.
So explicitly uses DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD to document
it to avoid we get it wrong.
Signed-off-by: Shaohua Li <shli@kernel.org>
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
mm/slub.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Index: linux/mm/slub.c
===================================================================
--- linux.orig/mm/slub.c 2011-08-24 08:46:27.000000000 +0800
+++ linux/mm/slub.c 2011-08-24 08:49:41.000000000 +0800
@@ -1534,7 +1534,7 @@ static inline void add_partial(struct km
struct page *page, int tail)
{
n->nr_partial++;
- if (tail)
+ if (tail == DEACTIVATE_TO_TAIL)
list_add_tail(&page->lru, &n->partial);
else
list_add(&page->lru, &n->partial);
@@ -1781,13 +1781,13 @@ static void deactivate_slab(struct kmem_
enum slab_modes l = M_NONE, m = M_NONE;
void *freelist;
void *nextfree;
- int tail = 0;
+ int tail = DEACTIVATE_TO_HEAD;
struct page new;
struct page old;
if (page->freelist) {
stat(s, DEACTIVATE_REMOTE_FREES);
- tail = 1;
+ tail = DEACTIVATE_TO_TAIL;
}
c->tid = next_tid(c->tid);
@@ -1893,7 +1893,7 @@ redo:
if (m == M_PARTIAL) {
add_partial(n, page, tail);
- stat(s, tail ? DEACTIVATE_TO_TAIL : DEACTIVATE_TO_HEAD);
+ stat(s, tail);
} else if (m == M_FULL) {
@@ -2377,7 +2377,7 @@ static void __slab_free(struct kmem_cach
*/
if (unlikely(!prior)) {
remove_full(s, page);
- add_partial(n, page, 1);
+ add_partial(n, page, DEACTIVATE_TO_TAIL);
stat(s, FREE_ADD_PARTIAL);
}
}
@@ -2695,7 +2695,7 @@ static void early_kmem_cache_node_alloc(
init_kmem_cache_node(n, kmem_cache_node);
inc_slabs_node(kmem_cache_node, node, page->objects);
- add_partial(n, page, 0);
+ add_partial(n, page, DEACTIVATE_TO_HEAD);
}
static void free_kmem_cache_nodes(struct kmem_cache *s)
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
2011-08-24 0:57 ` Shaohua Li
@ 2011-08-24 13:55 ` Christoph Lameter
-1 siblings, 0 replies; 14+ messages in thread
From: Christoph Lameter @ 2011-08-24 13:55 UTC (permalink / raw)
To: Shaohua Li; +Cc: Andrew Morton, linux-mm, lkml, penberg, Shi, Alex, Chen, Tim C
On Wed, 24 Aug 2011, Shaohua Li wrote:
> Subject: slub: explicitly document position of inserting slab to partial list
Acked-by: Christoph Lameter <cl@linux.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
@ 2011-08-24 13:55 ` Christoph Lameter
0 siblings, 0 replies; 14+ messages in thread
From: Christoph Lameter @ 2011-08-24 13:55 UTC (permalink / raw)
To: Shaohua Li; +Cc: Andrew Morton, linux-mm, lkml, penberg, Shi, Alex, Chen, Tim C
On Wed, 24 Aug 2011, Shaohua Li wrote:
> Subject: slub: explicitly document position of inserting slab to partial list
Acked-by: Christoph Lameter <cl@linux.com>
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
2011-08-24 0:57 ` Shaohua Li
@ 2011-08-29 3:06 ` Alex,Shi
-1 siblings, 0 replies; 14+ messages in thread
From: Alex,Shi @ 2011-08-29 3:06 UTC (permalink / raw)
To: Li, Shaohua
Cc: Christoph Lameter, Andrew Morton, linux-mm, lkml, penberg, Chen, Tim C
On Wed, 2011-08-24 at 08:57 +0800, Li, Shaohua wrote:
> On Tue, 2011-08-23 at 23:25 +0800, Christoph Lameter wrote:
> > On Tue, 23 Aug 2011, Shaohua Li wrote:
> >
> > > Adding slab to partial list head/tail is sensentive to performance.
> > > So adding a type to document it to avoid we get it wrong.
> >
> > I think that if you want to make it more descriptive then using the stats
> > values (DEACTIVATE_TO_TAIL/HEAD) would avoid having to introduce an
> > additional enum and it would also avoid the if statement in the stat call.
> ok, that's better.
>
> Subject: slub: explicitly document position of inserting slab to partial list
>
> Adding slab to partial list head/tail is sensitive to performance.
> So explicitly uses DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD to document
> it to avoid we get it wrong.
Frankly speaking, using DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD in
slab_alloc, slab_free make code hard to understand. Just adding some
comments will be more clear and understandable. like the following:
Do you think so?
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2377,6 +2377,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
*/
if (unlikely(!prior)) {
remove_full(s, page);
+ /* only one object left in the page, so add to partial tail */
add_partial(n, page, 1);
stat(s, FREE_ADD_PARTIAL);
}
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
@ 2011-08-29 3:06 ` Alex,Shi
0 siblings, 0 replies; 14+ messages in thread
From: Alex,Shi @ 2011-08-29 3:06 UTC (permalink / raw)
To: Li, Shaohua
Cc: Christoph Lameter, Andrew Morton, linux-mm, lkml, penberg, Chen, Tim C
On Wed, 2011-08-24 at 08:57 +0800, Li, Shaohua wrote:
> On Tue, 2011-08-23 at 23:25 +0800, Christoph Lameter wrote:
> > On Tue, 23 Aug 2011, Shaohua Li wrote:
> >
> > > Adding slab to partial list head/tail is sensentive to performance.
> > > So adding a type to document it to avoid we get it wrong.
> >
> > I think that if you want to make it more descriptive then using the stats
> > values (DEACTIVATE_TO_TAIL/HEAD) would avoid having to introduce an
> > additional enum and it would also avoid the if statement in the stat call.
> ok, that's better.
>
> Subject: slub: explicitly document position of inserting slab to partial list
>
> Adding slab to partial list head/tail is sensitive to performance.
> So explicitly uses DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD to document
> it to avoid we get it wrong.
Frankly speaking, using DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD in
slab_alloc, slab_free make code hard to understand. Just adding some
comments will be more clear and understandable. like the following:
Do you think so?
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2377,6 +2377,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
*/
if (unlikely(!prior)) {
remove_full(s, page);
+ /* only one object left in the page, so add to partial tail */
add_partial(n, page, 1);
stat(s, FREE_ADD_PARTIAL);
}
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
2011-08-29 3:06 ` Alex,Shi
@ 2011-08-29 14:20 ` Christoph Lameter
-1 siblings, 0 replies; 14+ messages in thread
From: Christoph Lameter @ 2011-08-29 14:20 UTC (permalink / raw)
To: Alex,Shi; +Cc: Li, Shaohua, Andrew Morton, linux-mm, lkml, penberg, Chen, Tim C
On Mon, 29 Aug 2011, Alex,Shi wrote:
> On Wed, 2011-08-24 at 08:57 +0800, Li, Shaohua wrote:
> > On Tue, 2011-08-23 at 23:25 +0800, Christoph Lameter wrote:
> > > On Tue, 23 Aug 2011, Shaohua Li wrote:
> > >
> > > > Adding slab to partial list head/tail is sensentive to performance.
> > > > So adding a type to document it to avoid we get it wrong.
> > >
> > > I think that if you want to make it more descriptive then using the stats
> > > values (DEACTIVATE_TO_TAIL/HEAD) would avoid having to introduce an
> > > additional enum and it would also avoid the if statement in the stat call.
> > ok, that's better.
> >
> > Subject: slub: explicitly document position of inserting slab to partial list
> >
> > Adding slab to partial list head/tail is sensitive to performance.
> > So explicitly uses DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD to document
> > it to avoid we get it wrong.
>
> Frankly speaking, using DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD in
> slab_alloc, slab_free make code hard to understand. Just adding some
> comments will be more clear and understandable. like the following:
> Do you think so?
Yes, I like that more.
Acked-by: Christoph Lameter <cl@linux.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
@ 2011-08-29 14:20 ` Christoph Lameter
0 siblings, 0 replies; 14+ messages in thread
From: Christoph Lameter @ 2011-08-29 14:20 UTC (permalink / raw)
To: Alex,Shi; +Cc: Li, Shaohua, Andrew Morton, linux-mm, lkml, penberg, Chen, Tim C
On Mon, 29 Aug 2011, Alex,Shi wrote:
> On Wed, 2011-08-24 at 08:57 +0800, Li, Shaohua wrote:
> > On Tue, 2011-08-23 at 23:25 +0800, Christoph Lameter wrote:
> > > On Tue, 23 Aug 2011, Shaohua Li wrote:
> > >
> > > > Adding slab to partial list head/tail is sensentive to performance.
> > > > So adding a type to document it to avoid we get it wrong.
> > >
> > > I think that if you want to make it more descriptive then using the stats
> > > values (DEACTIVATE_TO_TAIL/HEAD) would avoid having to introduce an
> > > additional enum and it would also avoid the if statement in the stat call.
> > ok, that's better.
> >
> > Subject: slub: explicitly document position of inserting slab to partial list
> >
> > Adding slab to partial list head/tail is sensitive to performance.
> > So explicitly uses DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD to document
> > it to avoid we get it wrong.
>
> Frankly speaking, using DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD in
> slab_alloc, slab_free make code hard to understand. Just adding some
> comments will be more clear and understandable. like the following:
> Do you think so?
Yes, I like that more.
Acked-by: Christoph Lameter <cl@linux.com>
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
2011-08-29 14:20 ` Christoph Lameter
@ 2011-08-30 1:51 ` Shaohua Li
-1 siblings, 0 replies; 14+ messages in thread
From: Shaohua Li @ 2011-08-30 1:51 UTC (permalink / raw)
To: Christoph Lameter
Cc: Shi, Alex, Andrew Morton, linux-mm, lkml, penberg, Chen, Tim C
On Mon, 2011-08-29 at 22:20 +0800, Christoph Lameter wrote:
> On Mon, 29 Aug 2011, Alex,Shi wrote:
>
> > On Wed, 2011-08-24 at 08:57 +0800, Li, Shaohua wrote:
> > > On Tue, 2011-08-23 at 23:25 +0800, Christoph Lameter wrote:
> > > > On Tue, 23 Aug 2011, Shaohua Li wrote:
> > > >
> > > > > Adding slab to partial list head/tail is sensentive to performance.
> > > > > So adding a type to document it to avoid we get it wrong.
> > > >
> > > > I think that if you want to make it more descriptive then using the stats
> > > > values (DEACTIVATE_TO_TAIL/HEAD) would avoid having to introduce an
> > > > additional enum and it would also avoid the if statement in the stat call.
> > > ok, that's better.
> > >
> > > Subject: slub: explicitly document position of inserting slab to partial list
> > >
> > > Adding slab to partial list head/tail is sensitive to performance.
> > > So explicitly uses DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD to document
> > > it to avoid we get it wrong.
> >
> > Frankly speaking, using DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD in
> > slab_alloc, slab_free make code hard to understand. Just adding some
> > comments will be more clear and understandable. like the following:
> > Do you think so?
>
> Yes, I like that more.
fine, let me add it to the first patch
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch 2/2]slub: add a type for slab partial list position
@ 2011-08-30 1:51 ` Shaohua Li
0 siblings, 0 replies; 14+ messages in thread
From: Shaohua Li @ 2011-08-30 1:51 UTC (permalink / raw)
To: Christoph Lameter
Cc: Shi, Alex, Andrew Morton, linux-mm, lkml, penberg, Chen, Tim C
On Mon, 2011-08-29 at 22:20 +0800, Christoph Lameter wrote:
> On Mon, 29 Aug 2011, Alex,Shi wrote:
>
> > On Wed, 2011-08-24 at 08:57 +0800, Li, Shaohua wrote:
> > > On Tue, 2011-08-23 at 23:25 +0800, Christoph Lameter wrote:
> > > > On Tue, 23 Aug 2011, Shaohua Li wrote:
> > > >
> > > > > Adding slab to partial list head/tail is sensentive to performance.
> > > > > So adding a type to document it to avoid we get it wrong.
> > > >
> > > > I think that if you want to make it more descriptive then using the stats
> > > > values (DEACTIVATE_TO_TAIL/HEAD) would avoid having to introduce an
> > > > additional enum and it would also avoid the if statement in the stat call.
> > > ok, that's better.
> > >
> > > Subject: slub: explicitly document position of inserting slab to partial list
> > >
> > > Adding slab to partial list head/tail is sensitive to performance.
> > > So explicitly uses DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD to document
> > > it to avoid we get it wrong.
> >
> > Frankly speaking, using DEACTIVATE_TO_TAIL/DEACTIVATE_TO_HEAD in
> > slab_alloc, slab_free make code hard to understand. Just adding some
> > comments will be more clear and understandable. like the following:
> > Do you think so?
>
> Yes, I like that more.
fine, let me add it to the first patch
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-08-30 1:50 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-23 0:37 [patch 2/2]slub: add a type for slab partial list position Shaohua Li
2011-08-23 0:37 ` Shaohua Li
2011-08-23 15:25 ` Christoph Lameter
2011-08-23 15:25 ` Christoph Lameter
2011-08-24 0:57 ` Shaohua Li
2011-08-24 0:57 ` Shaohua Li
2011-08-24 13:55 ` Christoph Lameter
2011-08-24 13:55 ` Christoph Lameter
2011-08-29 3:06 ` Alex,Shi
2011-08-29 3:06 ` Alex,Shi
2011-08-29 14:20 ` Christoph Lameter
2011-08-29 14:20 ` Christoph Lameter
2011-08-30 1:51 ` Shaohua Li
2011-08-30 1:51 ` Shaohua Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.