Re: [PATCH 1/4] oom: move oom_adj to signal_struct

From: KOSAKI Motohiro
Date: Thu Aug 06 2009 - 01:16:59 EST


2009/8/6 Oleg Nesterov <oleg@xxxxxxxxxx>:
> Sorry for late reply. And sorry, I didn't read these patches carefully yet,
> probably missed something...
>
> On 08/04, KOSAKI Motohiro wrote:
>>
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -34,6 +34,31 @@ int sysctl_oom_dump_tasks;
>>  static DEFINE_SPINLOCK(zone_scan_lock);
>>  /* #define DEBUG */
>>
>> +int get_oom_adj(struct task_struct *tsk)
>
> is it used outside oom_kill.c ?

Good catch.
Will fix.


>> +{
>> +     unsigned long flags;
>> +     int oom_adj = OOM_DISABLE;
>> +
>> +     if (tsk->mm && lock_task_sighand(tsk, &flags)) {
>
> Minor nit. _Afaics_, unlike proc, oom_kill.c never needs lock_task_sighand()
> to access ->signal->oom_adj.
>
> If the task was found under tasklist_lock by for_each_process/do_each_thread
> it must have the valid ->signal != NULL and it can't go away.

Thanks good suggestion!
Will fix.



> With these patches I think mm-introduce-proc-pid-oom_adj_child.patch should
> be dropped. This is good ;)

I agree, It should be dropped.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/