linux-kernel - Re: lowmemory android driver not needed?

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:	Wed, 21 Jan 2009 22:13:14 -0800
From:	Arve Hjønnevåg <arve@...roid.com>
To:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Cc:	Greg KH <greg@...ah.com>, Alan Cox <alan@...rguk.ukuu.org.uk>,
	Pavel Machek <pavel@...e.cz>,
	Brian Swetland <swetland@...gle.com>, arve@...gle.com,
	San Mehat <san@...roid.com>, Robert Love <rlove@...gle.com>,
	linux-kernel@...r.kernel.org,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	Tony Lindgren <tony@...mide.com>,
	ext Juha Yrjölä <juha.yrjola@...idboot.com>,
	viktor.rosendahl@...ia.com, Trilok Soni <soni.trilok@...il.com>
Subject: Re: lowmemory android driver not needed?

On Fri, Jan 16, 2009 at 5:18 AM, KOSAKI Motohiro
<kosaki.motohiro@...fujitsu.com> wrote:
> also quick review to lowmemorykiller.c

>> static struct shrinker lowmem_shrinker = {
>>         .shrink = lowmem_shrink,
>>         .seeks = DEFAULT_SEEKS * 16
>> };
>
> why do you choice *16?
>

To indicate that it is more expensive to restart a killed process than
a regular cache page.

>
>> static uint32_t lowmem_debug_level = 2;
>> static int lowmem_adj[6] = {
>
> why do you choice [6]?

We use six levels.


>> static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)
>> {
>>         struct task_struct *p;
>>         struct task_struct *selected = NULL;
>>         int rem = 0;
>>         int tasksize;
>>         int i;
>>         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);
>
> I think you don't consider mlocked page (and/or other unevictable page).
> if much mlocked file page exist, other_free can become large value.
> then, this routine don't kill any process.
>

Yes, this is a problem we have seen, but I did not find counter for
pinned cache pages. I may be able to use NR_UNEVICTABLE if
CONFIG_UNEVICTABLE_LRU is enabled now though.

Another problem we run into is that allocations of contiguous pages
can cause kswapd to empty all the caches. This will not trigger this
lowmemorykiller or the regular oom killer.

>>         read_lock(&tasklist_lock);
>>         for_each_process(p) {
>
> Oops. too long locking.
> shrink_slab() is freqentlly called function. I don't like costly operation.
>
>>                 if(p->oomkilladj >= 0 && p->mm) {
>>                         tasksize = get_mm_rss(p->mm);
>>                         if(nr_to_scan > 0 && tasksize > 0 && p->oomkilladj >= min_adj) {
>>                                 if(selected == NULL ||
>>                                    p->oomkilladj > selected->oomkilladj ||
>>                                    (p->oomkilladj == selected->oomkilladj &&
>>                                     tasksize > selected_tasksize)) {
>>                                         selected = p;
>>                                         selected_tasksize = tasksize;
>>                                         lowmem_print(2, "select %d (%s), adj %d, size %d, to kill\n",
>>                                                      p->pid, p->comm, p->oomkilladj, tasksize);
>>                                 }
>>                         }
>>                         rem += tasksize;
>
> this code mean,
> shrinker->shrink(0, gfp_mask) indicate to recalculate total rss every time.
> it is too CPU and battery wasting.
>

How about this:
----
diff --git a/drivers/misc/lowmemorykiller.c b/drivers/misc/lowmemorykiller.c
index 3715d56..3f4e41a 100644
--- a/drivers/misc/lowmemorykiller.c
+++ b/drivers/misc/lowmemorykiller.c
@@ -71,6 +71,12 @@ static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)
        }
        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_ANON) +
global_page_state(NR_ACTIVE_FILE);
+       if (!nr_to_scan || min_adj == OOM_ADJUST_MAX + 1) {
+               lowmem_print(5, "lowmem_shrink %d, %x, return %d\n",
nr_to_scan, gfp_mask, rem);
+               return rem;
+       }
+
        read_lock(&tasklist_lock);
        for_each_process(p) {
                if(p->oomkilladj >= 0 && p->mm) {
@@ -86,7 +92,6 @@ static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)
                                                     p->pid, p->comm,
p->oomkilladj, tasksize);
                                }
                        }
-                       rem += tasksize;
                }
        }
        if(selected != NULL) {
----


-- 
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/