About this list Date view Thread view Subject view Author view Attachment view

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


About this list Date view Thread view Subject view Author view Attachment view
[Next/Previous Months] [Main vserver Project Homepage] [Howto Subscribe/Unsubscribe] [Paul Sladen's vserver stuff]
Generated on Fri 19 Dec 2003 - 00:44:08 GMT by hypermail 2.1.3