linux-kernel - Re: [patch 02/11 -mmotm] lowmemorykiller: Don't count free space unless it meets the specified limit by itself

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 12 May 2009 17:27:18 -0700
From:	Arve Hjønnevåg <arve@...roid.com>
To:	Mel Gorman <mel@....ul.ie>
Cc:	David Rientjes <rientjes@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Nick Piggin <npiggin@...e.de>,
	Peter Ziljstra <a.p.ziljstra@...llo.nl>,
	Christoph Lameter <cl@...ux-foundation.org>,
	Dave Hansen <dave@...ux.vnet.ibm.com>,
	San Mehat <san@...roid.com>, linux-kernel@...r.kernel.org
Subject: Re: [patch 02/11 -mmotm] lowmemorykiller: Don't count free space 
	unless it meets the specified limit by itself

On Tue, May 12, 2009 at 2:23 AM, Mel Gorman <mel@....ul.ie> wrote:
> On Sun, May 10, 2009 at 03:07:10PM -0700, David Rientjes wrote:
>> From: Arve Hjønnevåg <arve@...roid.com>
>>
>> This allows processes to be killed when the kernel evict cache pages in
>> an attempt to get more contiguous free memory.
>>
>
> I'm not seeing what this patch has to do with contiguous free memory. I
> see what the patch is doing - lowering the threshold allowing a
> particular min_adj value to be used, but not why.
>

If the memory is fragmented, contiguous allocations can cause all
caches to be freed. The low memory killer would not trigger in this
case.

>
>> Signed-off-by: Arve Hjønnevåg <arve@...roid.com>
>> ---
>>  drivers/staging/android/lowmemorykiller.c |   13 +++++++++----
>>  1 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c
>> --- a/drivers/staging/android/lowmemorykiller.c
>> +++ b/drivers/staging/android/lowmemorykiller.c
>> @@ -58,20 +58,25 @@ static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)
>>       int min_adj = OOM_ADJUST_MAX + 1;
>>       int selected_tasksize = 0;
>>       int array_size = ARRAY_SIZE(lowmem_adj);
>> -     int other_free = global_page_state(NR_FREE_PAGES) + global_page_state(NR_FILE_PAGES);
>> +     int other_free = global_page_state(NR_FREE_PAGES);
>> +     int other_file = global_page_state(NR_FILE_PAGES);
>>       if(lowmem_adj_size < array_size)
>>               array_size = lowmem_adj_size;
>>       if(lowmem_minfree_size < array_size)
>>               array_size = lowmem_minfree_size;
>>       for(i = 0; i < array_size; i++) {
>
> It would appear that lowmem_adj_size is making assumptions on
> the number of zones that exist in the system. It's not clear why
> sysctl_lowmem_reserve_ratio[] is not being used or how they differ.

lowmem_adj_size is the number of elements in the array. Why would the
number of zones in the system affect it? The goal of the
lowmemorykiller is to make more memory available for caches. I would
expect increasing sysctl_lowmem_reserve_ratio to have the opposite
effect, but on our system with only one zone it has no effect.

>
>> -             if(other_free < lowmem_minfree[i]) {
>> +             if (other_free < lowmem_minfree[i] &&
>> +                 other_file < lowmem_minfree[i]) {
>>                       min_adj = lowmem_adj[i];
>>                       break;
>>               }
>>       }
>>       if(nr_to_scan > 0)
>> -             lowmem_print(3, "lowmem_shrink %d, %x, ofree %d, ma %d\n", nr_to_scan, gfp_mask, other_free, min_adj);
>> -     rem = global_page_state(NR_ACTIVE) + global_page_state(NR_INACTIVE);
>> +             lowmem_print(3, "lowmem_shrink %d, %x, ofree %d %d, ma %d\n", nr_to_scan, gfp_mask, other_free, other_file, min_adj);
>> +     rem = global_page_state(NR_ACTIVE_ANON) +
>> +             global_page_state(NR_ACTIVE_FILE) +
>> +             global_page_state(NR_INACTIVE_ANON) +
>> +             global_page_state(NR_INACTIVE_FILE);
>
> This looks like it's a compile fix since changes made to 4f98a2fe. This
> should have been in a separate patch and prioritised.

You are right. These patches were originally on 2.6.27 and I did not
notice that the first patch was also broken when I fixed this patch
for 2.6.29.

>
>>       if (nr_to_scan <= 0 || min_adj == OOM_ADJUST_MAX + 1) {
>>               lowmem_print(5, "lowmem_shrink %d, %x, return %d\n", nr_to_scan, gfp_mask, rem);
>>               return rem;
>
> --
> Mel Gorman
> Part-time Phd Student                          Linux Technology Center
> University of Limerick                         IBM Dublin Software Lab
>



-- 
Arve Hjønnevåg
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/