2010-07-13 17:52:12

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] slob_free:free objects to their own list

Hi Bob,

[ Please CC me on SLOB patches. You can use the 'scripts/get_maintainer.pl'
script to figure out automatically who to CC on your patches. ]

On Sat, Jul 10, 2010 at 1:05 PM, Bob Liu <[email protected]> wrote:
> slob has alloced smaller objects from their own list in reduce
> overall external fragmentation and increase repeatability,
> free to their own list also.
>
> Signed-off-by: Bob Liu <[email protected]>

The patch looks sane to me. Matt, does it look OK to you as well?

It would be nice to have some fragmentation numbers for this. One
really simple test case is to grep for MemTotal and MemFree in
/proc/meminfo. I'd expect to see some small improvement with your
patch applied. Quantifying long term fragmentation would be even
better but I don't have a good test case for that so I'm CC'ing Mel.

> ---
> ?mm/slob.c | ? ?9 ++++++++-
> ?1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/mm/slob.c b/mm/slob.c
> index 3f19a34..d582171 100644
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -396,6 +396,7 @@ static void slob_free(void *block, int size)
> ? ? ? ?slob_t *prev, *next, *b = (slob_t *)block;
> ? ? ? ?slobidx_t units;
> ? ? ? ?unsigned long flags;
> + ? ? ? struct list_head *slob_list;
>
> ? ? ? ?if (unlikely(ZERO_OR_NULL_PTR(block)))
> ? ? ? ? ? ? ? ?return;
> @@ -424,7 +425,13 @@ static void slob_free(void *block, int size)
> ? ? ? ? ? ? ? ?set_slob(b, units,
> ? ? ? ? ? ? ? ? ? ? ? ?(void *)((unsigned long)(b +
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SLOB_UNITS(PAGE_SIZE)) & PAGE_MASK));
> - ? ? ? ? ? ? ? set_slob_page_free(sp, &free_slob_small);
> + ? ? ? ? ? ? ? if (size < SLOB_BREAK1)
> + ? ? ? ? ? ? ? ? ? ? ? slob_list = &free_slob_small;
> + ? ? ? ? ? ? ? else if (size < SLOB_BREAK2)
> + ? ? ? ? ? ? ? ? ? ? ? slob_list = &free_slob_medium;
> + ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? slob_list = &free_slob_large;
> + ? ? ? ? ? ? ? set_slob_page_free(sp, slob_list);
> ? ? ? ? ? ? ? ?goto out;
> ? ? ? ?}
>
> --
> 1.5.6.3
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. ?For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>


2010-07-13 17:53:15

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] slob_free:free objects to their own list

On Tue, Jul 13, 2010 at 8:52 PM, Pekka Enberg <[email protected]> wrote:
> It would be nice to have some fragmentation numbers for this. One
> really simple test case is to grep for MemTotal and MemFree in
> /proc/meminfo. I'd expect to see some small improvement with your
> patch applied.

Small correction: grep them immediately after you've booted up the
kernel. If you run applications, the numbers are not comparable.

2010-07-13 21:53:52

by Matt Mackall

[permalink] [raw]
Subject: Re: [PATCH] slob_free:free objects to their own list

On Tue, 2010-07-13 at 20:52 +0300, Pekka Enberg wrote:
> Hi Bob,
>
> [ Please CC me on SLOB patches. You can use the 'scripts/get_maintainer.pl'
> script to figure out automatically who to CC on your patches. ]
>
> On Sat, Jul 10, 2010 at 1:05 PM, Bob Liu <[email protected]> wrote:
> > slob has alloced smaller objects from their own list in reduce
> > overall external fragmentation and increase repeatability,
> > free to their own list also.
> >
> > Signed-off-by: Bob Liu <[email protected]>
>
> The patch looks sane to me. Matt, does it look OK to you as well?

Yep, this should be a marginal improvement.

Acked-by: Matt Mackall <[email protected]>

--
Mathematics is the supreme nostalgia of our time.

2010-07-14 09:07:47

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] slob_free:free objects to their own list

On Wed, Jul 14, 2010 at 12:53 AM, Matt Mackall <[email protected]> wrote:
> On Tue, 2010-07-13 at 20:52 +0300, Pekka Enberg wrote:
>> Hi Bob,
>>
>> [ Please CC me on SLOB patches. You can use the 'scripts/get_maintainer.pl'
>> ? script to figure out automatically who to CC on your patches. ]
>>
>> On Sat, Jul 10, 2010 at 1:05 PM, Bob Liu <[email protected]> wrote:
>> > slob has alloced smaller objects from their own list in reduce
>> > overall external fragmentation and increase repeatability,
>> > free to their own list also.
>> >
>> > Signed-off-by: Bob Liu <[email protected]>
>>
>> The patch looks sane to me. Matt, does it look OK to you as well?
>
> Yep, this should be a marginal improvement.
>
> Acked-by: Matt Mackall <[email protected]>

Great! Bob, if you could provide the /proc/meminfo numbers for the
patch description, I'd be more than happy to merge this.

Pekka

2010-07-16 05:46:30

by Bob Liu

[permalink] [raw]
Subject: Re: [PATCH] slob_free:free objects to their own list

On Wed, Jul 14, 2010 at 5:07 PM, Pekka Enberg <[email protected]> wrote:
> On Wed, Jul 14, 2010 at 12:53 AM, Matt Mackall <[email protected]> wrote:
>> On Tue, 2010-07-13 at 20:52 +0300, Pekka Enberg wrote:
>>> Hi Bob,
>>>
>>> [ Please CC me on SLOB patches. You can use the 'scripts/get_maintainer.pl'
>>>   script to figure out automatically who to CC on your patches. ]
>>>
>>> On Sat, Jul 10, 2010 at 1:05 PM, Bob Liu <[email protected]> wrote:
>>> > slob has alloced smaller objects from their own list in reduce
>>> > overall external fragmentation and increase repeatability,
>>> > free to their own list also.
>>> >
>>> > Signed-off-by: Bob Liu <[email protected]>
>>>
>>> The patch looks sane to me. Matt, does it look OK to you as well?
>>
>> Yep, this should be a marginal improvement.
>>
>> Acked-by: Matt Mackall <[email protected]>
>
> Great! Bob, if you could provide the /proc/meminfo numbers for the
> patch description, I'd be more than happy to merge this.
>
Hi, Pekka

Sorry for the wrong cc and later reply.
This is /proc/meminfo result in my test machine:
without this patch:
===
MemTotal: 1030720 kB
MemFree: 750012 kB
Buffers: 15496 kB
Cached: 160396 kB
SwapCached: 0 kB
Active: 105024 kB
Inactive: 145604 kB
Active(anon): 74816 kB
Inactive(anon): 2180 kB
Active(file): 30208 kB
Inactive(file): 143424 kB
Unevictable: 16 kB
....

with this patch:
===
MemTotal: 1030720 kB
MemFree: 751908 kB
Buffers: 15492 kB
Cached: 160280 kB
SwapCached: 0 kB
Active: 102720 kB
Inactive: 146140 kB
Active(anon): 73168 kB
Inactive(anon): 2180 kB
Active(file): 29552 kB
Inactive(file): 143960 kB
Unevictable: 16 kB
...

The result show only very small improverment!
And when i tested it on a embeded system with 64MB, I found this path
is never called while kernel booting.

Thanks for the kindly review.

--
Regards,
--Bob

2010-07-16 08:17:29

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] slob_free:free objects to their own list

Bob Liu wrote:
> This is /proc/meminfo result in my test machine:
> without this patch:
> ===
> MemTotal: 1030720 kB
> MemFree: 750012 kB
> Buffers: 15496 kB
> Cached: 160396 kB
> SwapCached: 0 kB
> Active: 105024 kB
> Inactive: 145604 kB
> Active(anon): 74816 kB
> Inactive(anon): 2180 kB
> Active(file): 30208 kB
> Inactive(file): 143424 kB
> Unevictable: 16 kB
> ....
>
> with this patch:
> ===
> MemTotal: 1030720 kB
> MemFree: 751908 kB
> Buffers: 15492 kB
> Cached: 160280 kB
> SwapCached: 0 kB
> Active: 102720 kB
> Inactive: 146140 kB
> Active(anon): 73168 kB
> Inactive(anon): 2180 kB
> Active(file): 29552 kB
> Inactive(file): 143960 kB
> Unevictable: 16 kB
> ...
>
> The result show only very small improverment!
> And when i tested it on a embeded system with 64MB, I found this path
> is never called while kernel booting.

That's 1 MB improvement which is by no means small! I applied the patch.
Thanks Bob!