lowmemorykiller: Avoid excessive/redundant calling of LMK

The global shrinker will invoke lowmem_shrink in a loop. The loop will be run (total_scan_pages/batch_size) times. The default batch_size will be 128 which will make shrinker invoking 100s of times. LMK does meaningful work only during first 2-3 times and then rest of the invocations are just CPU cycle waste. Fix that by returning to the shrinker with SHRINK_STOP when LMK doesn't find any more work to do. The deciding factor here is, no process found in the selected LMK bucket or memory conditions are sane.

Signed-off-by Chintan Pandya <[email protected]>

---
 drivers/staging/android/lowmemorykiller.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/staging/android/lowmemorykiller.c +4 -1

index b545d3d..5bf483f 100644
@@ -110,7 +110,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	if (min_score_adj == OOM_SCORE_ADJ_MAX + 1) {
 		lowmem_print(5, "lowmem_scan %lu, %x, return 0\n",
 			     sc->nr_to_scan, sc->gfp_mask);
-		return 0;
+		return SHRINK_STOP;
 	}
 
 	selected_oom_score_adj = min_score_adj;

@@ -163,6 +163,9 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 		set_tsk_thread_flag(selected, TIF_MEMDIE);
 		send_sig(SIGKILL, selected, 0);
 		rem += selected_tasksize;
+	} else {
+		rcu_read_unlock();
+		return SHRINK_STOP;
 	}
 
 	lowmem_print(4, "lowmem_scan %lu, %x, return %lu\n",

Signature · 10 lines

Chintan Pandya

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation

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

Quoted 10 lines · Chintan Pandya

The global shrinker will invoke lowmem_shrink in a loop. The loop will be run (total_scan_pages/batch_size) times. The default batch_size will be 128 which will make shrinker invoking 100s of times. LMK does meaningful work only during first 2-3 times and then rest of the invocations are just CPU cycle waste. Fix that by returning to the shrinker with SHRINK_STOP when LMK doesn't find any more work to do. The deciding factor here is, no process found in the selected LMK bucket or memory conditions are sane.

lowmemory killer is broken by design and this one of the examples which shows why. It simply doesn't fit into shrinkers concept.

The count_object callback simply lies and tells the core that all the reclaimable LRU pages are scanable and gives it this as a number which the core uses for total_scan. scan_objects callback then happily ignore nr_to_reclaim and does its one time job where it iterates over _all_ tasks and picks up the victim and returns its rss as a return value. This is just a subset of LRU pages of course so it continues looping until total_scan goes down to 0 finally.

If this really has to be a shrinker then, shouldn't it evaluate the OOM situation in the count callback and return non zero only if OOM and then the scan callback would kill and return nr_to_reclaim.

Or even better wouldn't it be much better to use vmpressure to wake up a kernel module which would simply check the situation and kill something?

Please do not put only cosmetic changes on top of broken concept and try to think about a proper solution that is what staging is for AFAIU.

The code is in this state for quite some time and I would really hate if it got merged just because it is in staging for too long and it is used out there.

Quoted 31 lines · Michal Hocko

Quoted 10 lines · Chintan Pandya

The global shrinker will invoke lowmem_shrink in a loop. The loop will be run (total_scan_pages/batch_size) times. The default batch_size will be 128 which will make shrinker invoking 100s of times. LMK does meaningful work only during first 2-3 times and then rest of the invocations are just CPU cycle waste. Fix that by returning to the shrinker with SHRINK_STOP when LMK doesn't find any more work to do. The deciding factor here is, no process found in the selected LMK bucket or memory conditions are sane.

lowmemory killer is broken by design and this one of the examples which shows why. It simply doesn't fit into shrinkers concept.

The count_object callback simply lies and tells the core that all the reclaimable LRU pages are scanable and gives it this as a number which the core uses for total_scan. scan_objects callback then happily ignore nr_to_reclaim and does its one time job where it iterates over _all_ tasks and picks up the victim and returns its rss as a return value. This is just a subset of LRU pages of course so it continues looping until total_scan goes down to 0 finally.

If this really has to be a shrinker then, shouldn't it evaluate the OOM situation in the count callback and return non zero only if OOM and then the scan callback would kill and return nr_to_reclaim.

Or even better wouldn't it be much better to use vmpressure to wake up a kernel module which would simply check the situation and kill something?

Quoted 6 lines

Please do not put only cosmetic changes on top of broken concept and try to think about a proper solution that is what staging is for AFAIU.

The code is in this state for quite some time and I would really hate if it got merged just because it is in staging for too long and it is used out there.

So the in-kernel low-memory-killer is hopefully on its way out.

With Lollipop on some devices, Android is using the mempressure notifiers to kill processes from userland. However, not all devices have moved to this new model (and possibly some resulting performance issues are being worked out? Its not clear). So hopefully we can drop it soon, but I'd like to make sure we don't get only a half-working solution upstream before we do remove it.

thanks
-john

Quoted 31 lines · Michal Hocko

Quoted 10 lines · Chintan Pandya

The global shrinker will invoke lowmem_shrink in a loop. The loop will be run (total_scan_pages/batch_size) times. The default batch_size will be 128 which will make shrinker invoking 100s of times. LMK does meaningful work only during first 2-3 times and then rest of the invocations are just CPU cycle waste. Fix that by returning to the shrinker with SHRINK_STOP when LMK doesn't find any more work to do. The deciding factor here is, no process found in the selected LMK bucket or memory conditions are sane.

lowmemory killer is broken by design and this one of the examples which shows why. It simply doesn't fit into shrinkers concept.

The count_object callback simply lies and tells the core that all the reclaimable LRU pages are scanable and gives it this as a number which the core uses for total_scan. scan_objects callback then happily ignore nr_to_reclaim and does its one time job where it iterates over _all_ tasks and picks up the victim and returns its rss as a return value. This is just a subset of LRU pages of course so it continues looping until total_scan goes down to 0 finally.

If this really has to be a shrinker then, shouldn't it evaluate the OOM situation in the count callback and return non zero only if OOM and then the scan callback would kill and return nr_to_reclaim.

Or even better wouldn't it be much better to use vmpressure to wake up a kernel module which would simply check the situation and kill something?

Quoted 6 lines

Please do not put only cosmetic changes on top of broken concept and try to think about a proper solution that is what staging is for AFAIU.

The code is in this state for quite some time and I would really hate if it got merged just because it is in staging for too long and it is used out there.

So the in-kernel low-memory-killer is hopefully on its way out.

With Lollipop on some devices, Android is using the mempressure notifiers to kill processes from userland. However, not all devices have moved to this new model (and possibly some resulting performance issues are being worked out? Its not clear). So hopefully we can drop it soon, but I'd like to make sure we don't get only a half-working solution upstream before we do remove it.

thanks
-john