On Mon, May 31, 2010 at 10:52 PM, Luis Claudio R. Goncalves <lclaudio@xxxxxxxx> wrote: > On Mon, May 31, 2010 at 03:51:02PM +0900, KAMEZAWA Hiroyuki wrote: > | On Mon, 31 May 2010 15:09:41 +0900 > | Minchan Kim <minchan.kim@xxxxxxxxx> wrote: > | > On Mon, May 31, 2010 at 2:54 PM, KAMEZAWA Hiroyuki > | > <kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote: > ... > | > >> > IIUC, the purpose of rising priority is to accerate dying thread to exit() > | > >> > for freeing memory AFAP. But to free memory, exit, all threads which share > | > >> > mm_struct should exit, too. I'm sorry if I miss something. > | > >> > | > >> How do we kill only some thread and what's the benefit of it? > | > >> I think when if some thread receives KILL signal, the process include > | > >> the thread will be killed. > | > >> > | > > yes, so, if you want a _process_ die quickly, you have to acceralte the whole > | > > threads on a process. Acceralating a thread in a process is not big help. > | > > | > Yes. > | > > | > I see the code. > | > oom_kill_process is called by > | > > | > 1. mem_cgroup_out_of_memory > | > 2. __out_of_memory > | > 3. out_of_memory > | > > | > > | > (1,2) calls select_bad_process which select victim task in processes > | > by do_each_process. > | > But 3 isn't In case of CONSTRAINT_MEMORY_POLICY, it kills current. > | > In only the case, couldn't we pass task of process, not one of thread? > | > > | > | Hmm, my point is that priority-acceralation is against a thread, not against a process. > | So, most of threads in memory-eater will not gain high priority even with this patch > | and works slowly. > > This is a good point... > > | I have no objections to this patch. I just want to confirm the purpose. If this patch > | is for accelating exiting process by SIGKILL, it seems not enough. > > I understand (from the comments in the code) the badness calculation gives more > points to the siblings in a thread that have their own mm. I wonder if what you > are describing is not a corner case. > > Again, your idea sounds like an interesting refinement to the patch. I am > just not sure this change should implemented now or in a second round of > changes. First of all, I think your patch is first. That's because I am not sure this logic is effective. /* * We give our sacrificial lamb high priority and access to * all the memory it needs. That way it should be able to * exit() and clear out its resources quickly... */ p->rt.time_slice = HZ; Peter changed it in fa717060f1ab. Now if we change rt.time_slice as HZ, it means the task have high priority? I am not a scheduler expert. but as I looked through scheduler code, rt.time_slice is only related to RT scheduler. so if we uses CFS, it doesn't make task high priority. Perter, Right? If it is right, I think Luis patch will fix it. Secondly, as Kame pointed out, we have to raise whole thread's priority to kill victim process for reclaiming pages. But I think it has deadlock problem. If we raise whole threads's priority and some thread has dependency of other thread which is blocked, it makes system deadlock. So I think it's not easy part. If this part is really big problem, we should consider it more carefully. > > | If an explanation as "acceralating all thread's priority in a process seems overkill" > | is given in changelog or comment, it's ok to me. > > If my understanding of badness() is right, I wouldn't be ashamed of saying > that it seems to be _a bit_ overkill. But I may be wrong in my > interpretation. > > While re-reading the code I noticed that in select_bad_process() we can > eventually bump on an already dying task, case in which we just wait for > the task to die and avoid killing other tasks. Maybe we could boost the > priority of the dying task here too. Yes. It is good where we boost priority of task, I think. > > Luis > -- > [ Luis Claudio R. Goncalves Bass - Gospel - RT ] > [ Fingerprint: 4FDD B8C4 3C59 34BD 8BE9 2696 7203 D980 A448 C8F8 ] > > -- Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href