From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751648AbcFUUWK (ORCPT ); Tue, 21 Jun 2016 16:22:10 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:35961 "EHLO mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751347AbcFUUWH (ORCPT ); Tue, 21 Jun 2016 16:22:07 -0400 Date: Tue, 21 Jun 2016 13:22:00 -0700 (PDT) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: Ganesh Mahendran cc: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org, arve@android.com, riandrews@android.com Subject: Re: [PATCH 2/3] staging: lowmemorykiller: count anon pages only when we have swap devices In-Reply-To: <1466487018-5410-2-git-send-email-opensource.ganesh@gmail.com> Message-ID: References: <1466487018-5410-1-git-send-email-opensource.ganesh@gmail.com> <1466487018-5410-2-git-send-email-opensource.ganesh@gmail.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 21 Jun 2016, Ganesh Mahendran wrote: > lowmem_count() should only count anon pages when we have swap device. > Why? > Signed-off-by: Ganesh Mahendran > --- > drivers/staging/android/lowmemorykiller.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c > index 6da9260..1d8de47 100644 > --- a/drivers/staging/android/lowmemorykiller.c > +++ b/drivers/staging/android/lowmemorykiller.c > @@ -73,10 +73,14 @@ static unsigned long lowmem_deathpending_timeout; > static unsigned long lowmem_count(struct shrinker *s, > struct shrink_control *sc) > { > - return global_page_state(NR_ACTIVE_ANON) + > - global_page_state(NR_ACTIVE_FILE) + > - global_page_state(NR_INACTIVE_ANON) + > - global_page_state(NR_INACTIVE_FILE); > + unsigned long freeable = global_page_state(NR_ACTIVE_FILE) + > + global_page_state(NR_INACTIVE_FILE); > + > + if (get_nr_swap_pages() > 0) > + freeable += global_page_state(NR_ACTIVE_ANON) + > + global_page_state(NR_INACTIVE_ANON); > + > + return freeable; > } > > static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc) Shouldn't this be advertising the amount of memory that is freeable by killing the process with the highest priority oom_score_adj? It's not legitimate to say it can free all anon and file memory if nothing is oom killable, so this function is wrong both originally and with your patched version.