* [PATCH] mm/slub.c: remove 'per_cpu' which is useless variable
@ 2013-07-08 8:07 Chen Gang
2013-07-11 6:45 ` Pekka Enberg
0 siblings, 1 reply; 24+ messages in thread
From: Chen Gang @ 2013-07-08 8:07 UTC (permalink / raw)
To: cl, penberg, mpm; +Cc: linux-mm, Andrew Morton
Remove 'per_cpu', since it is useless now after the patch: "205ab99
slub: Update statistics handling for variable order slabs".
Also beautify code with tab alignment.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
mm/slub.c | 17 ++++++-----------
1 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 2caaa67..aa847eb 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4271,12 +4271,10 @@ static ssize_t show_slab_objects(struct kmem_cache *s,
int node;
int x;
unsigned long *nodes;
- unsigned long *per_cpu;
- nodes = kzalloc(2 * sizeof(unsigned long) * nr_node_ids, GFP_KERNEL);
+ nodes = kzalloc(sizeof(unsigned long) * nr_node_ids, GFP_KERNEL);
if (!nodes)
return -ENOMEM;
- per_cpu = nodes + nr_node_ids;
if (flags & SO_CPU) {
int cpu;
@@ -4307,8 +4305,6 @@ static ssize_t show_slab_objects(struct kmem_cache *s,
total += x;
nodes[node] += x;
}
-
- per_cpu[node]++;
}
}
@@ -4318,12 +4314,11 @@ static ssize_t show_slab_objects(struct kmem_cache *s,
for_each_node_state(node, N_NORMAL_MEMORY) {
struct kmem_cache_node *n = get_node(s, node);
- if (flags & SO_TOTAL)
- x = atomic_long_read(&n->total_objects);
- else if (flags & SO_OBJECTS)
- x = atomic_long_read(&n->total_objects) -
- count_partial(n, count_free);
-
+ if (flags & SO_TOTAL)
+ x = atomic_long_read(&n->total_objects);
+ else if (flags & SO_OBJECTS)
+ x = atomic_long_read(&n->total_objects) -
+ count_partial(n, count_free);
else
x = atomic_long_read(&n->nr_slabs);
total += x;
--
1.7.7.6
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: remove 'per_cpu' which is useless variable
2013-07-08 8:07 [PATCH] mm/slub.c: remove 'per_cpu' which is useless variable Chen Gang
@ 2013-07-11 6:45 ` Pekka Enberg
2013-07-11 6:50 ` Chen Gang
0 siblings, 1 reply; 24+ messages in thread
From: Pekka Enberg @ 2013-07-11 6:45 UTC (permalink / raw)
To: Chen Gang; +Cc: cl, mpm, linux-mm, Andrew Morton
Hi,
On 07/08/2013 11:07 AM, Chen Gang wrote:
> Remove 'per_cpu', since it is useless now after the patch: "205ab99
> slub: Update statistics handling for variable order slabs".
Whoa, that's a really old commit. Christoph?
> Also beautify code with tab alignment.
That needs to be a separate patch.
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
> mm/slub.c | 17 ++++++-----------
> 1 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 2caaa67..aa847eb 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4271,12 +4271,10 @@ static ssize_t show_slab_objects(struct kmem_cache *s,
> int node;
> int x;
> unsigned long *nodes;
> - unsigned long *per_cpu;
>
> - nodes = kzalloc(2 * sizeof(unsigned long) * nr_node_ids, GFP_KERNEL);
> + nodes = kzalloc(sizeof(unsigned long) * nr_node_ids, GFP_KERNEL);
> if (!nodes)
> return -ENOMEM;
> - per_cpu = nodes + nr_node_ids;
>
> if (flags & SO_CPU) {
> int cpu;
> @@ -4307,8 +4305,6 @@ static ssize_t show_slab_objects(struct kmem_cache *s,
> total += x;
> nodes[node] += x;
> }
> -
> - per_cpu[node]++;
> }
> }
>
> @@ -4318,12 +4314,11 @@ static ssize_t show_slab_objects(struct kmem_cache *s,
> for_each_node_state(node, N_NORMAL_MEMORY) {
> struct kmem_cache_node *n = get_node(s, node);
>
> - if (flags & SO_TOTAL)
> - x = atomic_long_read(&n->total_objects);
> - else if (flags & SO_OBJECTS)
> - x = atomic_long_read(&n->total_objects) -
> - count_partial(n, count_free);
> -
> + if (flags & SO_TOTAL)
> + x = atomic_long_read(&n->total_objects);
> + else if (flags & SO_OBJECTS)
> + x = atomic_long_read(&n->total_objects) -
> + count_partial(n, count_free);
> else
> x = atomic_long_read(&n->nr_slabs);
> total += x;
>
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: remove 'per_cpu' which is useless variable
2013-07-11 6:45 ` Pekka Enberg
@ 2013-07-11 6:50 ` Chen Gang
2013-07-11 16:45 ` Christoph Lameter
0 siblings, 1 reply; 24+ messages in thread
From: Chen Gang @ 2013-07-11 6:50 UTC (permalink / raw)
To: Pekka Enberg; +Cc: cl, mpm, linux-mm, Andrew Morton
On 07/11/2013 02:45 PM, Pekka Enberg wrote:
> Hi,
>
> On 07/08/2013 11:07 AM, Chen Gang wrote:
>> Remove 'per_cpu', since it is useless now after the patch: "205ab99
>> slub: Update statistics handling for variable order slabs".
>
> Whoa, that's a really old commit. Christoph?
>
Hmm..., waiting discussing result... :-)
>> Also beautify code with tab alignment.
>
> That needs to be a separate patch.
OK, I should send it after get result from above discussion.
Thanks.
--
Chen Gang
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: remove 'per_cpu' which is useless variable
2013-07-11 6:50 ` Chen Gang
@ 2013-07-11 16:45 ` Christoph Lameter
2013-07-11 18:28 ` Pekka Enberg
0 siblings, 1 reply; 24+ messages in thread
From: Christoph Lameter @ 2013-07-11 16:45 UTC (permalink / raw)
To: Chen Gang; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On Thu, 11 Jul 2013, Chen Gang wrote:
> On 07/11/2013 02:45 PM, Pekka Enberg wrote:
> > Hi,
> >
> > On 07/08/2013 11:07 AM, Chen Gang wrote:
> >> Remove 'per_cpu', since it is useless now after the patch: "205ab99
> >> slub: Update statistics handling for variable order slabs".
> >
> > Whoa, that's a really old commit. Christoph?
Gosh we have some code duplication there. Patch mismerged?
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: remove 'per_cpu' which is useless variable
2013-07-11 16:45 ` Christoph Lameter
@ 2013-07-11 18:28 ` Pekka Enberg
2013-07-11 20:16 ` Christoph Lameter
0 siblings, 1 reply; 24+ messages in thread
From: Pekka Enberg @ 2013-07-11 18:28 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Chen Gang, mpm, linux-mm, Andrew Morton
On 7/11/13 7:45 PM, Christoph Lameter wrote:
> On Thu, 11 Jul 2013, Chen Gang wrote:
>
>> On 07/11/2013 02:45 PM, Pekka Enberg wrote:
>>> Hi,
>>>
>>> On 07/08/2013 11:07 AM, Chen Gang wrote:
>>>> Remove 'per_cpu', since it is useless now after the patch: "205ab99
>>>> slub: Update statistics handling for variable order slabs".
>>>
>>> Whoa, that's a really old commit. Christoph?
>
> Gosh we have some code duplication there. Patch mismerged?
What duplication are you referring to?
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: remove 'per_cpu' which is useless variable
2013-07-11 18:28 ` Pekka Enberg
@ 2013-07-11 20:16 ` Christoph Lameter
2013-07-11 23:52 ` Chen Gang
0 siblings, 1 reply; 24+ messages in thread
From: Christoph Lameter @ 2013-07-11 20:16 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Chen Gang, mpm, linux-mm, Andrew Morton
On Thu, 11 Jul 2013, Pekka Enberg wrote:
> > Gosh we have some code duplication there. Patch mismerged?
>
> What duplication are you referring to?
Sorry, no duplicate. The partial list is handled in the same way as the
per cpu slab. Yep its ok to remove. Chen: Clean up your patches and then
we can merge them.
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: remove 'per_cpu' which is useless variable
2013-07-11 20:16 ` Christoph Lameter
@ 2013-07-11 23:52 ` Chen Gang
2013-07-12 0:23 ` [PATCH v2] " Chen Gang
0 siblings, 1 reply; 24+ messages in thread
From: Chen Gang @ 2013-07-11 23:52 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On 07/12/2013 04:16 AM, Christoph Lameter wrote:
> On Thu, 11 Jul 2013, Pekka Enberg wrote:
>
>>> Gosh we have some code duplication there. Patch mismerged?
>>
>> What duplication are you referring to?
>
> Sorry, no duplicate. The partial list is handled in the same way as the
> per cpu slab. Yep its ok to remove. Chen: Clean up your patches and then
> we can merge them.
>
OK, thanks. I should send patch v2 for it.
Thanks.
--
Chen Gang
--
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] 24+ messages in thread
* [PATCH v2] mm/slub.c: remove 'per_cpu' which is useless variable
2013-07-11 23:52 ` Chen Gang
@ 2013-07-12 0:23 ` Chen Gang
2013-07-12 0:55 ` [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track() Chen Gang
2013-07-12 13:45 ` [PATCH v2] mm/slub.c: remove 'per_cpu' which is useless variable Christoph Lameter
0 siblings, 2 replies; 24+ messages in thread
From: Chen Gang @ 2013-07-12 0:23 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
Remove 'per_cpu', since it is useless now after the patch: "205ab99
slub: Update statistics handling for variable order slabs". And the
partial list is handled in the same way as the per cpu slab.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
mm/slub.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 2b02d66..c94a03f 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4271,12 +4271,10 @@ static ssize_t show_slab_objects(struct kmem_cache *s,
int node;
int x;
unsigned long *nodes;
- unsigned long *per_cpu;
- nodes = kzalloc(2 * sizeof(unsigned long) * nr_node_ids, GFP_KERNEL);
+ nodes = kzalloc(sizeof(unsigned long) * nr_node_ids, GFP_KERNEL);
if (!nodes)
return -ENOMEM;
- per_cpu = nodes + nr_node_ids;
if (flags & SO_CPU) {
int cpu;
@@ -4307,8 +4305,6 @@ static ssize_t show_slab_objects(struct kmem_cache *s,
total += x;
nodes[node] += x;
}
-
- per_cpu[node]++;
}
}
--
1.7.7.6
--
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] 24+ messages in thread
* [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-12 0:23 ` [PATCH v2] " Chen Gang
@ 2013-07-12 0:55 ` Chen Gang
2013-07-12 13:49 ` Christoph Lameter
2013-07-12 13:45 ` [PATCH v2] mm/slub.c: remove 'per_cpu' which is useless variable Christoph Lameter
1 sibling, 1 reply; 24+ messages in thread
From: Chen Gang @ 2013-07-12 0:55 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
Since alloc_loc_track() will alloc additional space, and already knows
about 'max', so need be sure of 'max' must be larger than 't->count'.
The caller may not notice about it, e.g. call from add_location() in
"mm/slub.c", which only let "max = 2 * max" when "t->count >= t->max"
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
mm/slub.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 2b02d66..36f606d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3993,6 +3993,9 @@ static int alloc_loc_track(struct loc_track *t, unsigned long max, gfp_t flags)
struct location *l;
int order;
+ if (t->count >= max)
+ return 0;
+
order = get_order(sizeof(struct location) * max);
l = (void *)__get_free_pages(flags, order);
--
1.7.7.6
--
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] 24+ messages in thread
* Re: [PATCH v2] mm/slub.c: remove 'per_cpu' which is useless variable
2013-07-12 0:23 ` [PATCH v2] " Chen Gang
2013-07-12 0:55 ` [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track() Chen Gang
@ 2013-07-12 13:45 ` Christoph Lameter
2013-07-15 0:08 ` Chen Gang
1 sibling, 1 reply; 24+ messages in thread
From: Christoph Lameter @ 2013-07-12 13:45 UTC (permalink / raw)
To: Chen Gang; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On Fri, 12 Jul 2013, Chen Gang wrote:
> Remove 'per_cpu', since it is useless now after the patch: "205ab99
> slub: Update statistics handling for variable order slabs". And the
> partial list is handled in the same way as the per cpu slab.
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-12 0:55 ` [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track() Chen Gang
@ 2013-07-12 13:49 ` Christoph Lameter
2013-07-15 0:17 ` Chen Gang
0 siblings, 1 reply; 24+ messages in thread
From: Christoph Lameter @ 2013-07-12 13:49 UTC (permalink / raw)
To: Chen Gang; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On Fri, 12 Jul 2013, Chen Gang wrote:
> Since alloc_loc_track() will alloc additional space, and already knows
> about 'max', so need be sure of 'max' must be larger than 't->count'.
alloc_loc_track is only called if t->count > max from add_location:
/*
* Not found. Insert new tracking element.
*/
if (t->count >= t->max && !alloc_loc_track(t, 2 * t->max, GFP_ATOMIC))
return 0;
> The caller may not notice about it, e.g. call from add_location() in
> "mm/slub.c", which only let "max = 2 * max" when "t->count >= t->max"
That call already has the condition checked before the call. The only
other caller is list_locations which calls alloc_loc_track when t->count == 0
--
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] 24+ messages in thread
* Re: [PATCH v2] mm/slub.c: remove 'per_cpu' which is useless variable
2013-07-12 13:45 ` [PATCH v2] mm/slub.c: remove 'per_cpu' which is useless variable Christoph Lameter
@ 2013-07-15 0:08 ` Chen Gang
0 siblings, 0 replies; 24+ messages in thread
From: Chen Gang @ 2013-07-15 0:08 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On 07/12/2013 09:45 PM, Christoph Lameter wrote:
> On Fri, 12 Jul 2013, Chen Gang wrote:
>
>> Remove 'per_cpu', since it is useless now after the patch: "205ab99
>> slub: Update statistics handling for variable order slabs". And the
>> partial list is handled in the same way as the per cpu slab.
>
> Acked-by: Christoph Lameter <cl@linux.com>
>
>
Thanks.
--
Chen Gang
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-12 13:49 ` Christoph Lameter
@ 2013-07-15 0:17 ` Chen Gang
2013-07-15 15:16 ` Christoph Lameter
0 siblings, 1 reply; 24+ messages in thread
From: Chen Gang @ 2013-07-15 0:17 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On 07/12/2013 09:49 PM, Christoph Lameter wrote:
> On Fri, 12 Jul 2013, Chen Gang wrote:
>
>> Since alloc_loc_track() will alloc additional space, and already knows
>> about 'max', so need be sure of 'max' must be larger than 't->count'.
>
> alloc_loc_track is only called if t->count > max from add_location:
>
For add_location(), if "t->count > t->max", it calls alloc_loc_track()
with "max == 2 * t->max".
In this case we need be sure that "t->count < 2 * t->max".
> /*
> * Not found. Insert new tracking element.
> */
> if (t->count >= t->max && !alloc_loc_track(t, 2 * t->max, GFP_ATOMIC))
> return 0;
>
Thanks.
--
Chen Gang
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-15 0:17 ` Chen Gang
@ 2013-07-15 15:16 ` Christoph Lameter
2013-07-16 1:03 ` Chen Gang
0 siblings, 1 reply; 24+ messages in thread
From: Christoph Lameter @ 2013-07-15 15:16 UTC (permalink / raw)
To: Chen Gang; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On Mon, 15 Jul 2013, Chen Gang wrote:
> On 07/12/2013 09:49 PM, Christoph Lameter wrote:
> > On Fri, 12 Jul 2013, Chen Gang wrote:
> >
> >> Since alloc_loc_track() will alloc additional space, and already knows
> >> about 'max', so need be sure of 'max' must be larger than 't->count'.
> >
> > alloc_loc_track is only called if t->count > max from add_location:
> >
>
> For add_location(), if "t->count > t->max", it calls alloc_loc_track()
> with "max == 2 * t->max".
>
> In this case we need be sure that "t->count < 2 * t->max".
We are sure about that since t->count is always incremented by one and
then checked against t->max. The location database is build up from a
single hardware thread without any concurrency.
So we do not really need this 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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-15 15:16 ` Christoph Lameter
@ 2013-07-16 1:03 ` Chen Gang
2013-07-17 15:03 ` Christoph Lameter
0 siblings, 1 reply; 24+ messages in thread
From: Chen Gang @ 2013-07-16 1:03 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On 07/15/2013 11:16 PM, Christoph Lameter wrote:
> On Mon, 15 Jul 2013, Chen Gang wrote:
>
>> > On 07/12/2013 09:49 PM, Christoph Lameter wrote:
>>> > > On Fri, 12 Jul 2013, Chen Gang wrote:
>>> > >
>>>> > >> Since alloc_loc_track() will alloc additional space, and already knows
>>>> > >> about 'max', so need be sure of 'max' must be larger than 't->count'.
>>> > >
>>> > > alloc_loc_track is only called if t->count > max from add_location:
>>> > >
>> >
>> > For add_location(), if "t->count > t->max", it calls alloc_loc_track()
>> > with "max == 2 * t->max".
>> >
>> > In this case we need be sure that "t->count < 2 * t->max".
> We are sure about that since t->count is always incremented by one and
> then checked against t->max. The location database is build up from a
> single hardware thread without any concurrency.
>
> So we do not really need this patch.
>
>
>
Hmm... what you says above is reasonable.
In this case, since alloc_loc_track() is a static function, it will
depend on the related maintainers' willing and opinions to decide
whether add the related check or not (just like add 'BUG_ON' or not).
I need respect the original related maintainers' willing and opinions.
Thanks.
--
Chen Gang
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-16 1:03 ` Chen Gang
@ 2013-07-17 15:03 ` Christoph Lameter
2013-07-18 0:43 ` Chen Gang
0 siblings, 1 reply; 24+ messages in thread
From: Christoph Lameter @ 2013-07-17 15:03 UTC (permalink / raw)
To: Chen Gang; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On Tue, 16 Jul 2013, Chen Gang wrote:
> Hmm... what you says above is reasonable.
>
> In this case, since alloc_loc_track() is a static function, it will
> depend on the related maintainers' willing and opinions to decide
> whether add the related check or not (just like add 'BUG_ON' or not).
>
> I need respect the original related maintainers' willing and opinions.
You are talking to the author of the code.
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-17 15:03 ` Christoph Lameter
@ 2013-07-18 0:43 ` Chen Gang
2013-07-18 13:45 ` Christoph Lameter
0 siblings, 1 reply; 24+ messages in thread
From: Chen Gang @ 2013-07-18 0:43 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On 07/17/2013 11:03 PM, Christoph Lameter wrote:
> On Tue, 16 Jul 2013, Chen Gang wrote:
>
>> Hmm... what you says above is reasonable.
>>
>> In this case, since alloc_loc_track() is a static function, it will
>> depend on the related maintainers' willing and opinions to decide
>> whether add the related check or not (just like add 'BUG_ON' or not).
>>
>> I need respect the original related maintainers' willing and opinions.
>
> You are talking to the author of the code.
>
Firstly, at least, my this patch is really useless.
Hmm... when anybody says "need respect original authors' willing and
opinions", I think it often means we have found the direct issue, but
none of us find the root issue.
e.g. for our this case:
the direct issue is:
"whether need check the length with 'max' parameter".
but maybe the root issue is:
"whether use 'size' as related parameter name instead of 'max'".
in alloc_loc_track(), 'max' just plays the 'size' role.
Thanks.
--
Chen Gang
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-18 0:43 ` Chen Gang
@ 2013-07-18 13:45 ` Christoph Lameter
2013-07-19 0:05 ` Chen Gang F T
0 siblings, 1 reply; 24+ messages in thread
From: Christoph Lameter @ 2013-07-18 13:45 UTC (permalink / raw)
To: Chen Gang; +Cc: Pekka Enberg, mpm, linux-mm, Andrew Morton
On Thu, 18 Jul 2013, Chen Gang wrote:
> Hmm... when anybody says "need respect original authors' willing and
> opinions", I think it often means we have found the direct issue, but
> none of us find the root issue.
Is there an actual problem / failure being addressed by this patch?
> e.g. for our this case:
> the direct issue is:
> "whether need check the length with 'max' parameter".
> but maybe the root issue is:
> "whether use 'size' as related parameter name instead of 'max'".
> in alloc_loc_track(), 'max' just plays the 'size' role.
"max" determines the size of the loc_track structure. So these can
roughly mean the same thing.
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-18 13:45 ` Christoph Lameter
@ 2013-07-19 0:05 ` Chen Gang F T
2013-07-19 13:57 ` Christoph Lameter
0 siblings, 1 reply; 24+ messages in thread
From: Chen Gang F T @ 2013-07-19 0:05 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Chen Gang, Pekka Enberg, mpm, linux-mm, Andrew Morton
On 07/18/2013 09:45 PM, Christoph Lameter wrote:
> On Thu, 18 Jul 2013, Chen Gang wrote:
>
>> > Hmm... when anybody says "need respect original authors' willing and
>> > opinions", I think it often means we have found the direct issue, but
>> > none of us find the root issue.
> Is there an actual problem / failure being addressed by this patch?
>
No, at least, this patch (add parameter length checking) is useless.
>> > e.g. for our this case:
>> > the direct issue is:
>> > "whether need check the length with 'max' parameter".
>> > but maybe the root issue is:
>> > "whether use 'size' as related parameter name instead of 'max'".
>> > in alloc_loc_track(), 'max' just plays the 'size' role.
> "max" determines the size of the loc_track structure. So these can
> roughly mean the same thing.
Yes, "'max' can roughly mean the same thing", but they are still a
little different.
'max' also means: "the caller tells callee: I have told you the
maximize buffer length, so I need not check the buffer length to be
sure of no memory overflow, you need be sure of it".
'size' means: "the caller tells callee: you should use the size which I
give you, I am sure it is OK, do not care about whether it can cause
memory overflow or not".
The diff may like this:
--------------------------------diff begin------------------------------
diff --git a/mm/slub.c b/mm/slub.c
index 2b02d66..8564677 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3988,12 +3988,12 @@ static void free_loc_track(struct loc_track *t)
get_order(sizeof(struct location) * t->max));
}
-static int alloc_loc_track(struct loc_track *t, unsigned long max, gfp_t flags)
+static int alloc_loc_track(struct loc_track *t, unsigned long size, gfp_t flags)
{
struct location *l;
int order;
- order = get_order(sizeof(struct location) * max);
+ order = get_order(sizeof(struct location) * size);
l = (void *)__get_free_pages(flags, order);
if (!l)
@@ -4003,7 +4003,7 @@ static int alloc_loc_track(struct loc_track *t, unsigned long max, gfp_t flags)
memcpy(l, t->loc, sizeof(struct location) * t->count);
free_loc_track(t);
}
- t->max = max;
+ t->max = size;
t->loc = l;
return 1;
}
--------------------------------diff end--------------------------------
Thanks
--
Chen Gang
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-19 0:05 ` Chen Gang F T
@ 2013-07-19 13:57 ` Christoph Lameter
2013-07-22 0:27 ` Chen Gang
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Christoph Lameter @ 2013-07-19 13:57 UTC (permalink / raw)
To: Chen Gang F T; +Cc: Chen Gang, Pekka Enberg, mpm, linux-mm, Andrew Morton
On Fri, 19 Jul 2013, Chen Gang F T wrote:
> Yes, "'max' can roughly mean the same thing", but they are still a
> little different.
>
> 'max' also means: "the caller tells callee: I have told you the
> maximize buffer length, so I need not check the buffer length to be
> sure of no memory overflow, you need be sure of it".
>
> 'size' means: "the caller tells callee: you should use the size which I
> give you, I am sure it is OK, do not care about whether it can cause
> memory overflow or not".
Ok that makes sense.
> The diff may like this:
I am fine with such a patch.
Ultimately I would like the tracking and debugging technology to be
abstracted from the slub allocator and made generally useful by putting it
into mm/slab_common.c. SLAB has similar things but does not have all the
features.
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-19 13:57 ` Christoph Lameter
@ 2013-07-22 0:27 ` Chen Gang
2013-07-22 0:42 ` Wanpeng Li
2013-07-22 0:42 ` Wanpeng Li
2 siblings, 0 replies; 24+ messages in thread
From: Chen Gang @ 2013-07-22 0:27 UTC (permalink / raw)
To: Christoph Lameter
Cc: Chen Gang F T, Pekka Enberg, mpm, linux-mm, Andrew Morton
On 07/19/2013 09:57 PM, Christoph Lameter wrote:
> On Fri, 19 Jul 2013, Chen Gang F T wrote:
>
>> > Yes, "'max' can roughly mean the same thing", but they are still a
>> > little different.
>> >
>> > 'max' also means: "the caller tells callee: I have told you the
>> > maximize buffer length, so I need not check the buffer length to be
>> > sure of no memory overflow, you need be sure of it".
>> >
>> > 'size' means: "the caller tells callee: you should use the size which I
>> > give you, I am sure it is OK, do not care about whether it can cause
>> > memory overflow or not".
> Ok that makes sense.
>
Thanks.
>> > The diff may like this:
> I am fine with such a patch.
>
If suitable, I should send the related patch for it.
Is it necessary to send the related patch for it ?
> Ultimately I would like the tracking and debugging technology to be
> abstracted from the slub allocator and made generally useful by putting it
> into mm/slab_common.c. SLAB has similar things but does not have all the
> features.
>
At least for me, it is reasonable and necessary.
If possible, I'd like to do with it.
Excuse me, I have to do another things within this month. If really may
let me do, I should finish within next month (2013-08-31), is it OK ?
Thanks.
--
Chen Gang
--
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-19 13:57 ` Christoph Lameter
2013-07-22 0:27 ` Chen Gang
2013-07-22 0:42 ` Wanpeng Li
@ 2013-07-22 0:42 ` Wanpeng Li
2013-07-22 1:30 ` Chen Gang
2 siblings, 1 reply; 24+ messages in thread
From: Wanpeng Li @ 2013-07-22 0:42 UTC (permalink / raw)
To: Christoph Lameter
Cc: Chen Gang F T, Chen Gang, Pekka Enberg, mpm, linux-mm,
Andrew Morton
On Fri, Jul 19, 2013 at 01:57:28PM +0000, Christoph Lameter wrote:
>On Fri, 19 Jul 2013, Chen Gang F T wrote:
>
>> Yes, "'max' can roughly mean the same thing", but they are still a
>> little different.
>>
>> 'max' also means: "the caller tells callee: I have told you the
>> maximize buffer length, so I need not check the buffer length to be
>> sure of no memory overflow, you need be sure of it".
>>
>> 'size' means: "the caller tells callee: you should use the size which I
>> give you, I am sure it is OK, do not care about whether it can cause
>> memory overflow or not".
>
>Ok that makes sense.
>
>> The diff may like this:
>
>I am fine with such a patch.
>
>Ultimately I would like the tracking and debugging technology to be
>abstracted from the slub allocator and made generally useful by putting it
>into mm/slab_common.c. SLAB has similar things but does not have all the
>features.
Coincidence, I am doing this work recently and will post patches soon.
;-)
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-19 13:57 ` Christoph Lameter
2013-07-22 0:27 ` Chen Gang
@ 2013-07-22 0:42 ` Wanpeng Li
2013-07-22 0:42 ` Wanpeng Li
2 siblings, 0 replies; 24+ messages in thread
From: Wanpeng Li @ 2013-07-22 0:42 UTC (permalink / raw)
To: Christoph Lameter
Cc: Chen Gang F T, Chen Gang, Pekka Enberg, mpm, linux-mm,
Andrew Morton
On Fri, Jul 19, 2013 at 01:57:28PM +0000, Christoph Lameter wrote:
>On Fri, 19 Jul 2013, Chen Gang F T wrote:
>
>> Yes, "'max' can roughly mean the same thing", but they are still a
>> little different.
>>
>> 'max' also means: "the caller tells callee: I have told you the
>> maximize buffer length, so I need not check the buffer length to be
>> sure of no memory overflow, you need be sure of it".
>>
>> 'size' means: "the caller tells callee: you should use the size which I
>> give you, I am sure it is OK, do not care about whether it can cause
>> memory overflow or not".
>
>Ok that makes sense.
>
>> The diff may like this:
>
>I am fine with such a patch.
>
>Ultimately I would like the tracking and debugging technology to be
>abstracted from the slub allocator and made generally useful by putting it
>into mm/slab_common.c. SLAB has similar things but does not have all the
>features.
Coincidence, I am doing this work recently and will post patches soon.
;-)
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] 24+ messages in thread
* Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()
2013-07-22 0:42 ` Wanpeng Li
@ 2013-07-22 1:30 ` Chen Gang
0 siblings, 0 replies; 24+ messages in thread
From: Chen Gang @ 2013-07-22 1:30 UTC (permalink / raw)
To: Wanpeng Li
Cc: Christoph Lameter, Chen Gang F T, Pekka Enberg, mpm, linux-mm,
Andrew Morton
On 07/22/2013 08:42 AM, Wanpeng Li wrote:
> On Fri, Jul 19, 2013 at 01:57:28PM +0000, Christoph Lameter wrote:
>> On Fri, 19 Jul 2013, Chen Gang F T wrote:
>>
>>> Yes, "'max' can roughly mean the same thing", but they are still a
>>> little different.
>>>
>>> 'max' also means: "the caller tells callee: I have told you the
>>> maximize buffer length, so I need not check the buffer length to be
>>> sure of no memory overflow, you need be sure of it".
>>>
>>> 'size' means: "the caller tells callee: you should use the size which I
>>> give you, I am sure it is OK, do not care about whether it can cause
>>> memory overflow or not".
>>
>> Ok that makes sense.
>>
>>> The diff may like this:
>>
>> I am fine with such a patch.
>>
>> Ultimately I would like the tracking and debugging technology to be
>> abstracted from the slub allocator and made generally useful by putting it
>> into mm/slab_common.c. SLAB has similar things but does not have all the
>> features.
>
> Coincidence, I am doing this work recently and will post patches soon.
> ;-)
>
Thanks. :-)
> 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>
>
>
>
--
Chen Gang
--
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] 24+ messages in thread
end of thread, other threads:[~2013-07-22 1:31 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-08 8:07 [PATCH] mm/slub.c: remove 'per_cpu' which is useless variable Chen Gang
2013-07-11 6:45 ` Pekka Enberg
2013-07-11 6:50 ` Chen Gang
2013-07-11 16:45 ` Christoph Lameter
2013-07-11 18:28 ` Pekka Enberg
2013-07-11 20:16 ` Christoph Lameter
2013-07-11 23:52 ` Chen Gang
2013-07-12 0:23 ` [PATCH v2] " Chen Gang
2013-07-12 0:55 ` [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track() Chen Gang
2013-07-12 13:49 ` Christoph Lameter
2013-07-15 0:17 ` Chen Gang
2013-07-15 15:16 ` Christoph Lameter
2013-07-16 1:03 ` Chen Gang
2013-07-17 15:03 ` Christoph Lameter
2013-07-18 0:43 ` Chen Gang
2013-07-18 13:45 ` Christoph Lameter
2013-07-19 0:05 ` Chen Gang F T
2013-07-19 13:57 ` Christoph Lameter
2013-07-22 0:27 ` Chen Gang
2013-07-22 0:42 ` Wanpeng Li
2013-07-22 0:42 ` Wanpeng Li
2013-07-22 1:30 ` Chen Gang
2013-07-12 13:45 ` [PATCH v2] mm/slub.c: remove 'per_cpu' which is useless variable Christoph Lameter
2013-07-15 0:08 ` Chen Gang
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).