From: Herbert Poetzl (herbert_at_13thfloor.at)
Date: Fri 19 Dec 2003 - 00:42:28 GMT
On Thu, Dec 18, 2003 at 05:09:10PM -0500, nathan_at_0x00.org wrote:
> Hey,
>
> I am testing v1.22 with 2.4.23.
> I am getting an oops in fs/proc/array.c:proc_pid_status
sounds like it is quite reproducible, do you
have some code, or a sequence of actions to
trigger it?
> I believe the offending code is:
>
> buffer += sprintf (buffer,"s_context: %d [", task->vx_id);
> for (i=0; i<NB_S_CONTEXT; i++) {
> int ctx = task->s_info->vx_id[i];
>
> if (ctx == 0)
> break;
> buffer += sprintf (buffer," %d",ctx);
> }
>
> The oops is occuring on line 3 of that code.
> I believe the problem is due to lack of adequate
> locking around the block of code.
hmm, sounds reasonable ...
> Should there be some read locking code around this block?
> I am going to wrap the block that accesses the
> s_info structure around a read_lock()/unlock of
> tasklist_lock and see if I can stabilize the machine.
I guess this won't work, let me explain what I
think that happens before you get this oops ...
A: proc_pid_status() from somewhere ...
B: reaches release_task somehow ...
... both sync on task_lock()
A: advances past if (task->s_info)
B: calls vx_release_info(p), which clears p->s_info
A: loops in ctx = task->s_info->*
so IMHO one simple solution should be to acquire
and hold the ctx_ref_lock over the entire access
in proc_pid_status ...
basically like this ...
--- linux/kernel/vcontext.c Sat Dec 13 19:01:44 2003
+++ linux/kernel/vcontext.c Fri Dec 19 01:26:19 2003
@@ -152,7 +152,7 @@ static void vx_alloc_info(void)
}
}
-static spinlock_t ctx_ref_lock = SPIN_LOCK_UNLOCKED;
+spinlock_t ctx_ref_lock = SPIN_LOCK_UNLOCKED;
/*
* Increase the reference count on the context_info member of a task
--- linux/fs/proc/array.c Sat Dec 13 19:01:42 2003
+++ linux/fs/proc/array.c Fri Dec 19 01:39:05 2003
@@ -281,6 +281,7 @@ static inline char *task_cap(struct task
cap_t(p->cap_bset));
}
+extern spinlock_t ctx_ref_lock;
int proc_pid_status(struct task_struct *task, char * buffer)
{
@@ -300,6 +301,7 @@ int proc_pid_status(struct task_struct *
}
buffer = task_sig(task, buffer);
buffer = task_cap(task, buffer);
+ spin_lock(&ctx_ref_lock);
if (task->s_info) {
int i;
@@ -345,6 +347,7 @@ int proc_pid_status(struct task_struct *
buffer += sprintf (buffer,"ipv4root: 0\n");
buffer += sprintf (buffer,"ipv4root_bcast: 0\n");
}
+ spin_unlock(&ctx_ref_lock);
#if defined(CONFIG_ARCH_S390)
buffer = task_show_regs(task, buffer);
#endif
> Let me know if anyone else has any insight into this.
but I guess we'll have a better solution soon,
I'm rewriting the proc code anyway ...
HTH,
Herbert
> Thanks.
>
> -Nathan
> _______________________________________________
> Vserver mailing list
> Vserver_at_list.linux-vserver.org
> http://list.linux-vserver.org/mailman/listinfo/vserver
_______________________________________________
Vserver mailing list
Vserver_at_list.linux-vserver.org
http://list.linux-vserver.org/mailman/listinfo/vserver