Messages in this thread | | | Date | Wed, 21 Jan 2009 12:13:14 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH] softlockup: remove hung_task_check_count |
| |
* Mandeep Singh Baines <msb@google.com> wrote:
> read_lock(&tasklist_lock); > do_each_thread(g, t) { > - if (!--max_count) > - goto unlock; > + if (!--max_count) { > + /* > + * Drop the lock every once in a while and resched if > + * necessary. Don't want to hold the lock too long. > + */ > + get_task_struct(t); > + read_unlock(&tasklist_lock); > + max_count = HUNG_TASK_CHECK_COUNT; > + if (need_resched()) > + schedule(); > + read_lock(&tasklist_lock); > + put_task_struct(t); > + /* > + * t was unlinked from tasklist. Can't continue in this > + * case. Exit and try again next time. > + */ > + if (t->state == TASK_DEAD) > + goto unlock; > + }
firstly, this bit should move into a helper function. Also, why dont you do the need_resched() check first (it's very lighweight) - and thus only do the heavy ops (get-task-struct & tasklist_lock unlock) if that is set?
But most importantly, isnt the logic above confused? --max_count will reach zero exactly once, and then we'll loop for a long time.
Ingo
| |