Messages in this thread Patch in this message | | | Date | Sat, 05 Feb 2005 15:35:35 +1100 | From | Nick Piggin <> | Subject | Re: Race condition in ptrace |
| |
Nick Piggin wrote: > Andrew Morton wrote: > >> Nick Piggin <nickpiggin@yahoo.com.au> wrote: >>
>>> Andrew, IMO this is another bug to hold 2.6.11 for. >> >> >> >> Sure. I wouldn't consider Bodo's patch to be the one to use though.. > > > No. Something similar could be done that works on all architectures > and all wait_task_inactive callers (and is confined to sched.c). That > would still be more or less a hack to work around smtnice's unfortunate > locking though. >
Something like the following (untested) extension of Bodo's work could be the minimal fix for 2.6.11. As I've said though, I'd consider it a hack and prefer to do something about the locking. That could be done after 2.6.11 though. Depends how you feel.
Bodo, I wonder if this looks like a suitable fix for your problem?
---
linux-2.6-npiggin/include/linux/init_task.h | 1 + linux-2.6-npiggin/include/linux/sched.h | 1 + linux-2.6-npiggin/kernel/sched.c | 12 ++++++++++-- 3 files changed, 12 insertions(+), 2 deletions(-)
diff -puN include/linux/sched.h~sched-fixup-locking include/linux/sched.h --- linux-2.6/include/linux/sched.h~sched-fixup-locking 2005-02-05 15:24:00.000000000 +1100 +++ linux-2.6-npiggin/include/linux/sched.h 2005-02-05 15:24:39.000000000 +1100 @@ -533,6 +533,7 @@ struct task_struct { unsigned long ptrace; int lock_depth; /* Lock depth */ + int on_cpu; /* Is the task on the CPU, or in a ctxsw */ int prio, static_prio; struct list_head run_list; diff -puN kernel/sched.c~sched-fixup-locking kernel/sched.c --- linux-2.6/kernel/sched.c~sched-fixup-locking 2005-02-05 15:24:02.000000000 +1100 +++ linux-2.6-npiggin/kernel/sched.c 2005-02-05 15:34:37.000000000 +1100 @@ -294,7 +294,7 @@ static DEFINE_PER_CPU(struct runqueue, r #ifndef prepare_arch_switch # define prepare_arch_switch(rq, next) do { } while (0) # define finish_arch_switch(rq, next) spin_unlock_irq(&(rq)->lock) -# define task_running(rq, p) ((rq)->curr == (p)) +# define task_running(rq, p) ((p)->on_cpu) #endif /* @@ -867,7 +867,7 @@ void wait_task_inactive(task_t * p) repeat: rq = task_rq_lock(p, &flags); /* Must be off runqueue entirely, not preempted. */ - if (unlikely(p->array)) { + if (unlikely(p->array || p->on_cpu)) { /* If it's preempted, we yield. It could be a while. */ preempted = !task_running(rq, p); task_rq_unlock(rq, &flags); @@ -2805,11 +2805,18 @@ switch_tasks: rq->curr = next; ++*switch_count; + next->on_cpu = 1; prepare_arch_switch(rq, next); prev = context_switch(rq, prev, next); barrier(); finish_task_switch(prev); + /* + * Some architectures release the runqueue lock before + * context switching. Make sure this isn't reordered. + */ + smp_wmb(); + prev->on_cpu = 0; } else spin_unlock_irq(&rq->lock); @@ -4055,6 +4062,7 @@ void __devinit init_idle(task_t *idle, i set_task_cpu(idle, cpu); spin_lock_irqsave(&rq->lock, flags); + idle->on_cpu = 1; rq->curr = rq->idle = idle; set_tsk_need_resched(idle); spin_unlock_irqrestore(&rq->lock, flags); diff -puN include/linux/init_task.h~sched-fixup-locking include/linux/init_task.h --- linux-2.6/include/linux/init_task.h~sched-fixup-locking 2005-02-05 15:24:56.000000000 +1100 +++ linux-2.6-npiggin/include/linux/init_task.h 2005-02-05 15:25:07.000000000 +1100 @@ -73,6 +73,7 @@ extern struct group_info init_groups; .usage = ATOMIC_INIT(2), \ .flags = 0, \ .lock_depth = -1, \ + .on_cpu = 0, \ .prio = MAX_PRIO-20, \ .static_prio = MAX_PRIO-20, \ .policy = SCHED_NORMAL, \ _
| |