From owner-freebsd-threads@FreeBSD.ORG Thu Apr 17 22:22:16 2003 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id CB64A37B401 for ; Thu, 17 Apr 2003 22:22:16 -0700 (PDT) Received: from mail.pcnet.com (mail.pcnet.com [204.213.232.4]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2BD6E43FDF for ; Thu, 17 Apr 2003 22:22:16 -0700 (PDT) (envelope-from eischen@pcnet1.pcnet.com) Received: from pcnet1.pcnet.com (localhost [127.0.0.1]) by mail.pcnet.com (8.12.8/8.12.1) with ESMTP id h3I5MFBg026864; Fri, 18 Apr 2003 01:22:15 -0400 (EDT) Received: from localhost (eischen@localhost)h3I5MDoo026859; Fri, 18 Apr 2003 01:22:15 -0400 (EDT) Date: Fri, 18 Apr 2003 01:22:13 -0400 (EDT) From: Daniel Eischen To: David Xu In-Reply-To: <004c01c30470$9e36ddf0$0701a8c0@tiger> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: freebsd-threads@freebsd.org Subject: Re: libpthread patch X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Apr 2003 05:22:17 -0000 On Thu, 17 Apr 2003, David Xu wrote: > > ----- Original Message ----- > From: "Daniel Eischen" > To: "David Xu" > Cc: > Sent: Thursday, April 17, 2003 5:05 AM > Subject: Re: libpthread patch > > > > There's a new patch available at: > > > > http://people.freebsd.org/~deischen/kse/libpthread.diffs > > > > This passes all the ACE tests that libc_r passes, with the > > exception of Cached_Conn_Test. > > > > It also seems to work with KDE, konqueror, kwrite, kmail, etc. > > I don't have mozilla built (and am dreading trying to), but > > it would be interesting to see if it works with that. > > > > Cool! > > > If no-one has any objections, I'd like to commit this > > soon. I'll let David review and comment to it first. > > > > David, I didn't add critical regions to _thr_alloc() and > > _thr_free(). I think that whenever they are used, we > > are already in a critical region or operating on an upcall. > > > > Hmm, I don't like to put malloc calling under critical section, > it is better to put it under a lock, otherwise this would cause dead > lock. suppose that an user thread is calling malloc(), and heap manager > got malloc spinlock, then it does somethings and the thread is preempted > by upcall from kernel, now UTS switches to another thread, that thread > starts to call pthread_create, so UTS kernel enters a critical region first, > and calls malloc, this would cause dead lock, because UTS is under critical > region and no context switch could happen. > Also I don't like thr_free under critical region, I think a GC thread is still > needed to recycle zombie thread and free extra memory, UTS kernel > should't be blocked by user thread. Despite this, I think the patch should > be committed. I tried to rework this based on your idea of doing it at thread allocation time. I just committed it. There are a few issues we've got to go over, as well as looking closely at any locking order problems. One thing, which I think you agreed to some weeks/months ago, was to have the kernel set flag in the KSE mailbox in the kse_exit() system call. This is so the UTS can know that the KSE and it's stack is truly done. I've got some code in thr_kern.c that is commented out and is expecting that this will get done. Is that still something we can do? One other thing. Is there a way to wake up a KSE without having an upcall? For instance, in _kse_lock_wait(), we do something like this: /* * Enter a loop to wait until we get the lock. */ ts.tv_sec = 0; ts.tv_nsec = 1000000; /* 1 sec */ KSE_SET_WAIT(curkse); while (_LCK_BUSY(lu)) { /* * Yield the kse and wait to be notified when the lock * is granted. */ crit = _kse_critical_enter(); __sys_nanosleep(&ts, NULL); _kse_critical_leave(crit); /* * Make sure that the wait flag is set again in case * we wokeup without the lock being granted. */ KSE_SET_WAIT(curkse); } KSE_CLEAR_WAIT(curkse); Can it be woken with kse_wakeup() or possible kse_thr_interrupt() (on a KSE mailbox) so that the nanosleep just returns? I used nanosleep, because kse_release() will force a new upcall. Hmm, perhaps we can have a parameter on kse_release to specify whether we want a normal return or a new upcall. I know that you have a patch for KSEs that never want to have an upcall, but that wouldn't work for this case where we want the KSE to upcall normally. -- Dan Eischen