1016212 - [Tarako] No memory pressure event if LMK was triggered by swap low

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Danny Liang [:dliang]]

Attachment: facebook_slog.tar.gz

There exist a case which memory pressure event won't be triggered before LMK killed foreground process due to free swap is low.

How to reproduce:
1. Install facebook.
2. Playing facebook
3. Facebook will be killed by LMK

Actual results:
There is no memory pressure event before LMK killed foreground process.

Expected results:
Memory pressure event will be triggered and the life time of facebook should be longer.

Comment 1

[ying.xu]

Can you do some tests to support your theory?

"Memory pressure event will be triggered and the life time of facebook should be longer."


Honestly , I really don't know if there was any side effects.

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Danny Liang [:dliang] from comment #0)
> There exist a case which memory pressure event won't be triggered before LMK
> killed foreground process due to free swap is low.

Is this a kernel problem? AFAIK the low memory trigger does not take swap space into account before firing the notification but I could be wrong.

Comment 3

[ying.xu]

(In reply to Gabriele Svelto [:gsvelto] from comment #2)
> AFAIK the low memory trigger does not take swap
> space into account before firing the notification but I could be wrong.

yes,you're right.

we're discussing adding this way to kernel or not.

Comment 4

[ying.xu]

this fix will be helpful.

pref("image.mem.max_decoded_image_kb", 25000);
https://bugzilla.mozilla.org/show_bug.cgi?id=1007019

Comment 5

[Danny Liang [:dliang]]

We have another case can reproduce this issue, it's playing CutTheRope.
When playing CutTheRope, CutTheRope will be killed suddently due to free swap is low, but cache and free memory are enough.

Comment 6

[Danny Liang [:dliang]]

Attachment: dont_kill_fg_process_when_free_swap_is_low.patch

The attached patch is don't kill foreground process when free swap is low, there is longer playing time and user experience should be better.

Comment 7

[James Zhang (Spreadtrum)]

Danny, can you attach slog?

Comment 8

[Danny Liang [:dliang]]

Attachment: slog_cuttherope.tar.gz

Comment 9

[ying.xu]

when free of swap partition == 0KB,  oom-killer will be involved.

If we don't kill foreground app here in LMK, when free of swap partition == 0KB, oom-killer still kill.

The current value of killing foreground app for LMK is 512KB. 

Even we leave it th oom-killer to kill, that won't help much here.

Comment 10

[Danny Liang [:dliang]]

(In reply to ying.xu from comment #9)
> when free of swap partition == 0KB,  oom-killer will be involved.
> 
> If we don't kill foreground app here in LMK, when free of swap partition ==
> 0KB, oom-killer still kill.
> 
> The current value of killing foreground app for LMK is 512KB. 
> 
> Even we leave it th oom-killer to kill, that won't help much here.

In the case we met, fg was killed by LMK when free swap is lower than 2MB, not 512KB.
By our test, don't killed fg according to free swap really helps, it have chance to trigger memory pressure event but system became slow.
As a user to use this low end smart-phone, I can accept system is slow rather than foreground was killed suddently.

Comment 11

[Danny Liang [:dliang]]

I did some tests on CutTheRope w/ and w/o patch. W/ patch, I can play to level 2-8, it can be continue but I am tired; w/o the patch, the game usually is closed before level 1-3. It showed the patch helps much.

Comment 12

[ying.xu]

(In reply to Danny Liang [:dliang] from comment #11)
> I did some tests on CutTheRope w/ and w/o patch. W/ patch, I can play to
> level 2-8, it can be continue but I am tired; w/o the patch, the game
> usually is closed before level 1-3. It showed the patch helps much.

OK.
I will merge this today.

Comment 13

[ying.xu]

(In reply to Danny Liang [:dliang] from comment #10)

> In the case we met, fg was killed by LMK when free swap is lower than 2MB,
> not 512KB.

yes, 512KB is the value of killing b2g

Comment 14

[ying.xu]

(In reply to Danny Liang [:dliang] from comment #11)
> I did some tests on CutTheRope w/ and w/o patch. W/ patch, I can play to
> level 2-8, it can be continue but I am tired; w/o the patch, the game
> usually is closed before level 1-3. It showed the patch helps much.

Can you upload the slog of cuttherope being killed, with your patch applied?

Comment 15

[Danny Liang [:dliang]]

(In reply to ying.xu from comment #14)
> (In reply to Danny Liang [:dliang] from comment #11)
> > I did some tests on CutTheRope w/ and w/o patch. W/ patch, I can play to
> > level 2-8, it can be continue but I am tired; w/o the patch, the game
> > usually is closed before level 1-3. It showed the patch helps much.
> 
> Can you upload the slog of cuttherope being killed, with your patch applied?

What log do you need? Attached slog_cuttherope is cuttherope be killed w/o patch.
I didn't save the log of playing cuttherope w/ patch.

Comment 16

[ying.xu]

(In reply to Danny Liang [:dliang] from comment #15)

> I didn't save the log of playing cuttherope w/ patch.

Can you capture one w/ patch?

Comment 17

[Danny Liang [:dliang]]

(In reply to ying.xu from comment #16)
> (In reply to Danny Liang [:dliang] from comment #15)
> 
> > I didn't save the log of playing cuttherope w/ patch.
> 
> Can you capture one w/ patch?

What case do you want? w/ patch and play it to until game is closed? or play it for a while?
My previous test is play it to level 2-8 and exit the game by myself.

Comment 18

[ying.xu]

(In reply to Danny Liang [:dliang] from comment #17)
> What case do you want? w/ patch and play it to until game is closed? or play
> it for a while?
> My previous test is play it to level 2-8 and exit the game by myself.

oh, my badness.
I want a log that w/ patch, the game was killed by system. To check no other side effects.
If that can't happen, I'll be very happy.

Comment 19

[James Zhang (Spreadtrum)]

If this patch has been verified with 24 hours monkey test, please land it.

Comment 20

[James Zhang (Spreadtrum)]

And please let Horace review it.

Comment 21

[Horace Hsieh]

Just for the patch in comment#6?
OK, for this fragment of code, it's ok to me.
In android platform, we allow to set min_adj under the foreground value if both "other_free" and "other_file" are all less than lowmem_minfree[FOREGOURND_LEVEL], but only prevent adjusting it due to low free swap space.

However, it's up to you.

Comment 22

[ying.xu]

we meet a problem once

the music app was playing at background but get killed ,due to low free memory of swap partition

http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=316704

01-01 03:16:11.481 <4>0[ 9241.841547] Free swap  = 2504kB
01-01 03:16:11.481 <4>0[ 9241.841555] Total swap = 65532kB

Seems we can disable the kill action for background perceivable app when free memory of swap partition was low.

Comment 23

[Danny Liang [:dliang]]

(In reply to ying.xu from comment #22)
> we meet a problem once
> 
> the music app was playing at background but get killed ,due to low free
> memory of swap partition
> 
> http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=316704
> 
> 01-01 03:16:11.481 <4>0[ 9241.841547] Free swap  = 2504kB
> 01-01 03:16:11.481 <4>0[ 9241.841555] Total swap = 65532kB
> 
> Seems we can disable the kill action for background perceivable app when
> free memory of swap partition was low.

Do we have STR to reproduce this case? Is there any memory leak or other problem to cause this issue? In normal case of playing music, the free swap is higher than 30MB.

Comment 24

[ying.xu]

(In reply to Danny Liang [:dliang] from comment #23)
> (In reply to ying.xu from comment #22)
> > we meet a problem once
> > 
> > the music app was playing at background but get killed ,due to low free
> > memory of swap partition
> > 
> > http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=316704
> > 
> > 01-01 03:16:11.481 <4>0[ 9241.841547] Free swap  = 2504kB
> > 01-01 03:16:11.481 <4>0[ 9241.841555] Total swap = 65532kB
> > 
> > Seems we can disable the kill action for background perceivable app when
> > free memory of swap partition was low.
> 
> Do we have STR to reproduce this case? Is there any memory leak or other
> problem to cause this issue? In normal case of playing music, the free swap
> is higher than 30MB.

no, we meet this once.
I don't know Because get-about-memory run fault, I can't give you a memory report.

Comment 25

[ying.xu]

Attachment: slog_20140527131221_sp6821a_userdebug.zip

log for Comment 24  ying.xu  2014-05-30 05:03:36 PDT

Comment 26

[Danny Liang [:dliang]]

(In reply to ying.xu from comment #25)
> Created attachment 8431527 [details]
> slog_20140527131221_sp6821a_userdebug.zip
> 
> log for Comment 24  ying.xu  2014-05-30 05:03:36 PDT

Could we know the distribution of swap memory when music was killed?

Comment 27

[ying.xu]

(In reply to Danny Liang [:dliang] from comment #26)
> > log for Comment 24  ying.xu  2014-05-30 05:03:36 PDT
> 
> Could we know the distribution of swap memory when music was killed?

we can't. all I had is the log.

Comment 28

[Horace Hsieh]

(In reply to Danny Liang [:dliang] from comment #26)
> (In reply to ying.xu from comment #25)
> > Created attachment 8431527 [details]
> > slog_20140527131221_sp6821a_userdebug.zip
> > 
> > log for Comment 24  ying.xu  2014-05-30 05:03:36 PDT
> 
> Could we know the distribution of swap memory when music was killed?

Yes, we have some patches to check the swap info of each process now,
please stay tuned.

Comment 29

[Danny Liang [:dliang]]

(In reply to Horace Hsieh from comment #28)
> (In reply to Danny Liang [:dliang] from comment #26)
> > (In reply to ying.xu from comment #25)
> > > Created attachment 8431527 [details]
> > > slog_20140527131221_sp6821a_userdebug.zip
> > > 
> > > log for Comment 24  ying.xu  2014-05-30 05:03:36 PDT
> > 
> > Could we know the distribution of swap memory when music was killed?
> 
> Yes, we have some patches to check the swap info of each process now,
> please stay tuned.
Do you mean the patch that print user_proc_info but might cause deadlock?
Have we fix the bug of patch?

Comment 30

[Horace Hsieh]

(In reply to Danny Liang [:dliang] from comment #29)
> (In reply to Horace Hsieh from comment #28)
> > (In reply to Danny Liang [:dliang] from comment #26)
> > > (In reply to ying.xu from comment #25)
> > > > Created attachment 8431527 [details]
> > > > slog_20140527131221_sp6821a_userdebug.zip
> > > > 
> > > > log for Comment 24  ying.xu  2014-05-30 05:03:36 PDT
> > > 
> > > Could we know the distribution of swap memory when music was killed?
> > 
> > Yes, we have some patches to check the swap info of each process now,
> > please stay tuned.
> Do you mean the patch that print user_proc_info but might cause deadlock?
> Have we fix the bug of patch?

No, I answered "Could we know the distribution of swap memory when music was killed".
Currently, we are developing some patches to show the swap info of each process.
I think it should be useful to you.

Comment 31

[Horace Hsieh]

(In reply to Danny Liang [:dliang] from comment #29)
> (In reply to Horace Hsieh from comment #28)
> > (In reply to Danny Liang [:dliang] from comment #26)
> > > (In reply to ying.xu from comment #25)
> > > > Created attachment 8431527 [details]
> > > > slog_20140527131221_sp6821a_userdebug.zip
> > > > 
> > > > log for Comment 24  ying.xu  2014-05-30 05:03:36 PDT
> > > 
> > > Could we know the distribution of swap memory when music was killed?
> > 
> > Yes, we have some patches to check the swap info of each process now,
> > please stay tuned.
> Do you mean the patch that print user_proc_info but might cause deadlock?
> Have we fix the bug of patch?

http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=318972

Comment 32

[Danny Liang [:dliang]]

(In reply to Horace Hsieh from comment #31)
> (In reply to Danny Liang [:dliang] from comment #29)
> > (In reply to Horace Hsieh from comment #28)
> > > (In reply to Danny Liang [:dliang] from comment #26)
> > > > (In reply to ying.xu from comment #25)
> > > > > Created attachment 8431527 [details]
> > > > > slog_20140527131221_sp6821a_userdebug.zip
> > > > > 
> > > > > log for Comment 24  ying.xu  2014-05-30 05:03:36 PDT
> > > > 
> > > > Could we know the distribution of swap memory when music was killed?
> > > 
> > > Yes, we have some patches to check the swap info of each process now,
> > > please stay tuned.
> > Do you mean the patch that print user_proc_info but might cause deadlock?
> > Have we fix the bug of patch?
> 
> http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=318972

I cannot access the bug, the following message was showed: You are not authorized to access bug #318972.

Comment 33

[ying.xu]

we can use get_mm_counter(mm, MM_SWAPENTS) to dump swap information 
add this function call in dump_tasks
that should be enough for us right now.

Comment 34

[Horace Hsieh]

> (In reply to Horace Hsieh from comment #31)
...
> I cannot access the bug, the following message was showed: You are not
> authorized to access bug #318972.

It's newly created to fix some bug in procrank to show swap info and also add the support of proportional swap info to procrank.

Comment 35

[Danny Liang [:dliang]]

(In reply to James Zhang from comment #19)
> If this patch has been verified with 24 hours monkey test, please land it.

Hi James, how is the result? Could we land it?

Comment 36

[James Zhang (Spreadtrum)]

We will land it today.

Comment 37

[ying.xu]

commit 4c82f1be5aafc5a102a3c3d98a641e0724683abe
Author: ying.xu <[e-mail]>
Date:   Tue Jun 3 15:56:55 2014 +0800

    Bug#303502  don't kill foreground apps when free of swap disk low