| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 2 Feb 2009 16:05:05 -0800
From: Mandeep Singh Baines <msb@...gle.com>
To: Ingo Molnar <mingo@...e.hu>,
Frédéric Weisbecker <fweisbec@...il.com>,
linux-kernel@...r.kernel.org
Cc: rientjes@...gle.com, mbligh@...gle.com, thockin@...gle.com,
Andrew Morton <akpm@...ux-foundation.org>
Subject: [PATCH 2/2 v2] softlockup: check all tasks in hung_task
On Sat, Jan 31, 2009 at 11:22 AM, Peter Zijlstra <peterz@...radead.org> wro=
te:
> On Fri, 2009-01-30 at 12:49 -0800, Mandeep Singh Baines wrote:
<snip>
>> +static void check_hung_rcu_refresh(struct task_struct *g, struct task_s=
truct *t)
>> +{
>> + get_task_struct(g);
>> + get_task_struct(t);
>> + rcu_read_unlock();
>> + if (need_resched())
>> + schedule();
>
> won't a simple cond_resched(), do?
>
Yes, it will. Thanks Peter!
>> + 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(unsign=
ed 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 =3D sysctl_hung_task_check_count;
>> + check_hung_rcu_refresh(g, t);
>> + /* Exit if t or g was unhashed during refresh. */
>> + if (t->state =3D=3D TASK_DEAD || g->state =3D=3D T=
ASK_DEAD)
>> + goto unlock;
>> + }
>
> Its all a bit ugly, but I suppose there's no way around that.
Yeah, need to guarantee that next of t and g isn't pointing to a freed
task_struct. If the t or g has not been unlinked, then its guaranteed that
the next fields are valid.
If the state is != TASK_DEAD then its guaranteed that the task hasn't been
unlinked. I guess that's the shakiest assumption in the chain. If the code were
ever changed to set the state to TASK_DEAD after unlinking ...
<snip>
Here's v2 of this patch. The only thing that's changed is the cond_resched().
---
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@...il.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@...gle.com>
---
kernel/hung_task.c | 27 +++++++++++++++++++++++++--
1 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index a841db3..464c96e 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -109,6 +109,24 @@ 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();
+ cond_resched();
+ 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 +147,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;
+ }
/* use "==" to skip the TASK_KILLABLE tasks waiting on NFS */
if (t->state == TASK_UNINTERRUPTIBLE)
check_hung_task(t, now, timeout);
--
1.5.4.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists