Re: [PATCH] softlockup: remove hung_task_check_count

[Ingo Molnar]

* Mandeep Singh Baines <msb@xxxxxxxxxx> 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
--
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/