Msg05631.html

From Linix VServer
Jump to navigationJump to search

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Vserver] SMP oops 2.4.23 v1.22[edit]


  • From: Herbert Poetzl
  • Date: Fri, 19 Dec 2003 01:37:16 +0100 (CET)



On Thu, Dec 18, 2003 at 05:09:10PM -0500, nathan@xxxxxxxx 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@xxxxxxxxxxxxxxxxxxxxxx
> http://list.linux-vserver.org/mailman/listinfo/vserver