On Sat, Jun 05, 2010 at 12:02:35AM +0100, Mindaugas Rasiukevicius wrote: > Hello, > > Chuck Silvers <c...@chuq.com> wrote: > > > > I rewrote my COMPAT_LINUX changes using the 1-proc-many-LWPs model. > > the patch is at: > > > > ftp://ftp.netbsd.org/pub/NetBSD/misc/chs/linux/diff.linux-nptl-take2.23 > > > > I like this patch a lot! Especially good clean-ups, as a result. > Few comments: > > - Thanks for implementing ucas_*() for extra architectures. This is > planned to be used for POSIX shared synchronisation primitives, thus > become a public API. In the longer term, LINUX_UCAS_INT_UP should be > moved to generic code or, preferably, die.
sure, I was just being a bit lazy. > - From proc_alloc(): > > - /* allocate next free pid */ > + p->p_pid = proc_alloc_pid(p); > + kdtrace_proc_ctor(NULL, p); > + mutex_exit(proc_lock); > + return p; > > The kdtrace_proc_ctor() should not allocate memory with proc_lock held. > Neither it needs to be called late - I fixed this on HEAD, so proc_lock > can now be released by proc_alloc_pid(). ok, I like that better too. I also changed proc_alloc_pid() to set p->p_pid while proc_lock is still held (if it wasn't set already). > - From linux_clone_nptl(): > > + lwp_lock(l2); > + if ((l->l_flag & (LW_WREBOOT | LW_WSUSPEND | LW_WEXIT)) == 0) { > + if (p->p_stat == SSTOP || (p->p_sflag & PS_STOPPING) != 0) > + l2->l_stat = LSSTOP; > + else { > > This needs lwp_unlock_to(l2, spc->spc_lwplock), as LSSTOP state is protected > by this lock. This is also broken in sys__lwp_create() - will fix it shortly. I'll take your word for it... I haven't learned all the scheduler goop, I just copied it from sys__lwp_create() (as you noticed). I've updated it with the new code. > - From sys_obreak(): > > + new = round_page((vaddr_t)SCARG(uap, nsize)); > + if (new == 0) { > + return ENOMEM; > + } > > IIRC, some architectures check for zero from assembly in libc stubs, before > going to the syscall. that check was there before, I just moved it earlier in the function. it looks like all of the netbsd libc stubs check the value against "end" or "_end" before calling the syscall, so we should be able to remove it for native processes. a bunch of other emulations use this same function though, and I don't see a lot of benefit in trying to make this more specific. -Chuck