lkml.org 
[lkml]   [2009]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/2] softlockup: check all tasks in hung_task
From
Date
On Fri, 2009-01-30 at 12:49 -0800, Mandeep Singh Baines wrote:
> Instead of checking only hung_task_check_count tasks, all tasks are checked.
> hung_task_check_count is still used to put an upper bound on the critical
> section. Every hung_task_check_count checks, the critical section is
> refreshed. Keeping the critical section small minimizes time preemption is
> disabled and keeps rcu grace periods small.
>
> To prevent following a stale pointer, get_task_struct is called on g and t.
> To verify that g and t have not been unhashed while outside the critical
> section, the task states are checked.
>
> The design was proposed by Frédéric Weisbecker.
>
> Frédéric Weisbecker (fweisbec@gmail.com) wrote:
> >
> > Instead of having this arbitrary limit of tasks, why not just
> > lurk the need_resched() and then schedule if it needs too.
> >
> > I know that sounds a bit racy, because you will have to release the
> > tasklist_lock and
> > a lot of things can happen in the task list until you become resched.
> > But you can do a get_task_struct() on g and t before your thread is
> > going to sleep and then put them
> > when it is awaken.
> > Perhaps some tasks will disappear or be appended in the list before g
> > and t, but that doesn't really matter:
> > if they disappear, they didn't lockup, and if they were appended, they
> > are not enough cold to be analyzed :-)
> >
> > This way you can drop the arbitrary limit of task number given by the user....
> >
> > Frederic.
> >
>
> Signed-off-by: Mandeep Singh Baines <msb@google.com>
> ---
> kernel/hung_task.c | 28 ++++++++++++++++++++++++++--
> 1 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index a841db3..1c8c9f9 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -109,6 +109,25 @@ static void check_hung_task(struct task_struct *t, unsigned long now,
> panic("hung_task: blocked tasks");
> }
>
> + /*
> + * To avoid extending the RCU grace period for an unbounded amount of time,
> + * periodically exit the critical section and enter a new one.
> + *
> + * For preemptible RCU it is sufficient to call rcu_read_unlock in order
> + * exit the grace period. For classic RCU, a reschedule is required.
> + */
> +static void check_hung_rcu_refresh(struct task_struct *g, struct task_struct *t)
> +{
> + get_task_struct(g);
> + get_task_struct(t);
> + rcu_read_unlock();
> + if (need_resched())
> + schedule();

won't a simple cond_resched(), do?

> + rcu_read_lock();
> + put_task_struct(t);
> + put_task_struct(g);
> +}
> +
> /*
> * Check whether a TASK_UNINTERRUPTIBLE does not get woken up for
> * a really long time (120 seconds). If that happens, print out
> @@ -129,8 +148,13 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
>
> rcu_read_lock();
> do_each_thread(g, t) {
> - if (!--max_count)
> - goto unlock;
> + if (sysctl_hung_task_check_count && !(max_count--)) {
> + max_count = sysctl_hung_task_check_count;
> + check_hung_rcu_refresh(g, t);
> + /* Exit if t or g was unhashed during refresh. */
> + if (t->state == TASK_DEAD || g->state == TASK_DEAD)
> + goto unlock;
> + }

Its all a bit ugly, but I suppose there's no way around that.

> /* use "==" to skip the TASK_KILLABLE tasks waiting on NFS */
> if (t->state == TASK_UNINTERRUPTIBLE)
> check_hung_task(t, now, timeout);

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

\
 
 \ /
  Last update: 2009-01-31 20:25    [W:0.096 / U:0.512 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site