Date: Fri, 7 Jul 2006 17:28:51 GMT From: Roman Divacky <rdivacky@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 100906 for review Message-ID: <200607071728.k67HSphB053593@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=100906 Change 100906 by rdivacky@rdivacky_witten on 2006/07/07 17:28:09 o introduce locking using rwlocks o fix bug in linux_clone() regarding pid o introduce em_find() Affected files ... .. //depot/projects/soc2006/rdivacky_linuxolator/i386/linux/linux.h#7 edit .. //depot/projects/soc2006/rdivacky_linuxolator/i386/linux/linux_machdep.c#10 edit .. //depot/projects/soc2006/rdivacky_linuxolator/i386/linux/linux_sysvec.c#6 edit Differences ... ==== //depot/projects/soc2006/rdivacky_linuxolator/i386/linux/linux.h#7 (text+ko) ==== @@ -32,6 +32,9 @@ #define _I386_LINUX_LINUX_H_ #include <sys/signal.h> /* for sigval union */ +#include <sys/param.h> +#include <sys/lock.h> +#include <sys/rwlock.h> #include <i386/linux/linux_syscall.h> @@ -769,4 +772,13 @@ SLIST_ENTRY(linux_emuldata) emuldatas; }; +#define EMUL_RLOCK(l) rw_rlock(l) +#define EMUL_RUNLOCK(l) rw_runlock(l) +#define EMUL_WLOCK(l) rw_wlock(l) +#define EMUL_WUNLOCK(l) rw_wunlock(l) + +/* for em_find use */ +#define EMUL_LOCKED 1 +#define EMUL_UNLOCKED 0 + #endif /* !_I386_LINUX_LINUX_H_ */ ==== //depot/projects/soc2006/rdivacky_linuxolator/i386/linux/linux_machdep.c#10 (text+ko) ==== @@ -65,10 +65,12 @@ SLIST_HEAD(emuldata_head, linux_emuldata) emuldata_head = SLIST_HEAD_INITIALIZER(emuldata_head); struct linux_emuldata *emuldata_headp; /* where we store the emulation data */ +struct rwlock emul_lock; static int linux_proc_init(struct thread *, pid_t); int linux_proc_exit(struct thread *); int linux_userret(struct thread *); +static struct linux_emuldata *em_find(pid_t pid, int locked); struct l_descriptor { l_uint entry_number; @@ -285,6 +287,25 @@ return (linux_select(td, &newsel)); } +/* this returns locked reference to the emuldata entry (if found) */ +static struct linux_emuldata * +em_find(pid_t pid, int locked) +{ + struct linux_emuldata *em; + + if (locked == EMUL_UNLOCKED) + EMUL_RLOCK(&emul_lock); + + SLIST_FOREACH(em, &emuldata_head, emuldatas) + if (em->pid == pid) + break; + + if (em == NULL && locked == EMUL_UNLOCKED) + EMUL_RUNLOCK(&emul_lock); + + return (em); +} + int linux_fork(struct thread *td, struct linux_fork_args *args) { @@ -308,10 +329,12 @@ return (error); #if 0 + EMUL_RLOCK(&emul_data); /* impossible to not find it */ SLIST_FOREACH(em, &emuldata_head, emuldatas) if (em->pid == td->td_retval[0]) break; + EMUL_RUNLOCK(&emul_data); #endif return (0); } @@ -405,17 +428,21 @@ return (error); /* create the emuldata */ - linux_proc_init(td, td->td_retval[0]); - SLIST_FOREACH(em, &emuldata_head, emuldatas) - if (em->pid == td->td_retval[0]) - break; + error = linux_proc_init(td, p2->p_pid); + /* reference it - no need to check this */ + em = em_find(p2->p_pid, EMUL_UNLOCKED); + KASSERT(em != NULL, "no emuldata after proc_init()!\n"); /* and adjust it */ if (args->flags & CLONE_PARENT_SETTID) { - if (args->parent_tidptr == NULL) + if (args->parent_tidptr == NULL) { + EMUL_RUNLOCK(&emul_lock); return (EINVAL); + } error = copyout(&td->td_proc->p_pid, args->parent_tidptr, sizeof(td->td_proc->p_pid)); - if (error) + if (error) { + EMUL_RUNLOCK(&emul_lock); return (error); + } } if (args->flags & CLONE_CHILD_CLEARTID) @@ -427,6 +454,7 @@ em->child_set_tid = args->child_tidptr; else em->child_set_tid = NULL; + EMUL_RUNLOCK(&emul_lock); PROC_LOCK(p2); p2->p_sigparent = exit_signal; @@ -1022,7 +1050,6 @@ linux_proc_init(struct thread *td, pid_t child) { struct linux_emuldata *em, *p_em; - int found = 0; struct proc *p; /* XXX: locking? */ @@ -1030,16 +1057,13 @@ /* non-exec call */ MALLOC(em, struct linux_emuldata *, sizeof *em, M_LINUX, M_WAITOK | M_ZERO); em->pid = child; + EMUL_WLOCK(&emul_lock); SLIST_INSERT_HEAD(&emuldata_head, em, emuldatas); } else { - found = 0; /* lookup the old one */ - SLIST_FOREACH(em, &emuldata_head, emuldatas) - if (em->pid == td->td_proc->p_pid) { - found = 1; - break; - } - if (found == 0) { + em = em_find(td->td_proc->p_pid, EMUL_UNLOCKED); + /* XXX: this might be turned into KASSERT once its tested enough */ + if (em == NULL) { /* this should not happen */ #ifdef DEBUG printf("emuldata not found in exec case.\n"); @@ -1052,22 +1076,24 @@ em->child_set_tid = NULL; /* SLIST is inefficient - use hash instead */ - /* find the emuldata for the parent process */ - SLIST_FOREACH(p_em, &emuldata_head, emuldatas) - if (p_em->pid == td->td_proc->p_pid) { - found = 1; - break; + /* I hope I rewrote the semantics right */ + if (child != 0) { + /* find the emuldata for the parent process */ + p_em = em_find(td->td_proc->p_pid, EMUL_LOCKED); + if (p_em == NULL) { + em->clear_tid = NULL; + em->set_tid = NULL; + } else { + em->clear_tid = p_em->clear_tid; + em->set_tid = p_em->set_tid; } - - if (found) { - em->clear_tid = p_em->clear_tid; - em->set_tid = p_em->set_tid; + EMUL_WUNLOCK(&emul_lock); } else { em->clear_tid = NULL; em->set_tid = NULL; + EMUL_RUNLOCK(&emul_lock); } - /* XXX: sched_lock locking? */ /* find the newly created thread and set the P_LINUX flag */ @@ -1087,21 +1113,34 @@ int error; /* find the emuldata */ - SLIST_FOREACH(em, &emuldata_head, emuldatas) - if (em->pid == td->td_proc->p_pid) - break; + em = em_find(td->td_proc->p_pid, EMUL_UNLOCKED); +#ifdef DEBUG + if (em == NULL) { + printf("we didnt find emuldata for the exiting process.\n"); + return (0); + } +#endif if (em->clear_tid != NULL) { int null = 0; error = copyout(&null, em->clear_tid, sizeof(null)); - if (error) + if (error) { + EMUL_RUNLOCK(&emul_lock); return (error); + } /* TODO: futexes stuff */ } + EMUL_RUNLOCK(&emul_lock); + /* XXX: there is a race but I think we can ommit that + * because its not very possible that the same process + * will exit on different cpus etc. + */ + EMUL_WLOCK(&emul_lock); SLIST_REMOVE(&emuldata_head, em, linux_emuldata, emuldatas); + EMUL_WUNLOCK(&emul_lock); /* clean the stuff up */ FREE(em, M_LINUX); @@ -1116,13 +1155,20 @@ int error = 0; /* find the emuldata */ - SLIST_FOREACH(em, &emuldata_head, emuldatas) - if (em->pid == td->td_proc->p_pid) - break; + em = em_find(td->td_proc->p_pid, EMUL_UNLOCKED); + +#ifdef DEBUG + if (child == NULL) { + printf("we didnt find emuldata for the userreting process.\n"); + EMUL_RUNLOCK(&emul_lock); + return (0); + } +#endif if (em->set_tid != NULL) error = copyout(&td->td_proc->p_pid, em->set_tid, sizeof(td->td_proc->p_pid)); + EMUL_RUNLOCK(&emul_lock); return (error); } @@ -1132,12 +1178,19 @@ struct linux_emuldata *em; /* find the emuldata */ - SLIST_FOREACH(em, &emuldata_head, emuldatas) - if (em->pid == td->td_proc->p_pid) - break; + em = em_find(td->td_proc->p_pid, EMUL_UNLOCKED); + +#ifdef DEBUG + if (child == NULL) { + printf("we didnt find emuldata for the userreting process.\n"); + EMUL_RUNLOCK(&emul_lock); + return (0); + } +#endif em->clear_tid = args->tidptr; td->td_retval[0] = td->td_proc->p_pid; + EMUL_RUNLOCK(&emul_lock); return 0; } ==== //depot/projects/soc2006/rdivacky_linuxolator/i386/linux/linux_sysvec.c#6 (text+ko) ==== @@ -111,6 +111,7 @@ extern int linux_userret(struct thread *); extern int linux_proc_exit(struct thread *); +extern struct rwlock emul_lock; /* * Linux syscalls return negative errno's, we do positive and map them @@ -917,6 +918,7 @@ SLIST_INIT(&emuldata_head); linux_proc_exit_p = linux_proc_exit; linux_userret_p = linux_userret; + rw_init(&emul_lock, "emuldata lock"); break; case MOD_UNLOAD: for (brandinfo = &linux_brandlist[0]; *brandinfo != NULL; @@ -940,6 +942,7 @@ printf("Could not deinstall ELF interpreter entry\n"); linux_proc_exit_p = NULL; linux_userret_p = NULL; + rw_destroy(&emul_lock); break; default: return EOPNOTSUPP;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200607071728.k67HSphB053593>