From owner-freebsd-threads@FreeBSD.ORG  Wed May  7 13:28:35 2003
Return-Path: <owner-freebsd-threads@FreeBSD.ORG>
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 3DD4B37B401; Wed,  7 May 2003 13:28:35 -0700 (PDT)
Received: from sccrmhc01.attbi.com (sccrmhc01.attbi.com [204.127.202.61])
	by mx1.FreeBSD.org (Postfix) with ESMTP
	id 31FA643FA3; Wed,  7 May 2003 13:28:34 -0700 (PDT)
	(envelope-from julian@elischer.org)
Received: from interjet.elischer.org
	(12-232-168-4.client.attbi.com[12.232.168.4])
	by attbi.com (sccrmhc01) with ESMTP
	id <2003050720283200100rf69be>; Wed, 7 May 2003 20:28:32 +0000
Received: from localhost (localhost.elischer.org [127.0.0.1])
	by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id NAA47779;
	Wed, 7 May 2003 13:28:31 -0700 (PDT)
Date: Wed, 7 May 2003 13:28:30 -0700 (PDT)
From: Julian Elischer <julian@elischer.org>
To: David Xu <davidxu@freebsd.org>
In-Reply-To: <001901c313a1$01c97c30$f001a8c0@davidw2k>
Message-ID: <Pine.BSF.4.21.0305071319450.47162-100000@InterJet.elischer.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
cc: threads@freebsd.org
cc: Daniel Eischen <eischen@pcnet1.pcnet.com>
cc: John Baldwin <jhb@freebsd.org>
Subject: Re: kern_threads.c..  lock question..
X-BeenThere: freebsd-threads@freebsd.org
X-Mailman-Version: 2.1.1
Precedence: list
List-Id: Threading on FreeBSD <freebsd-threads.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-threads>,
	<mailto:freebsd-threads-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-threads>
List-Post: <mailto:freebsd-threads@freebsd.org>
List-Help: <mailto:freebsd-threads-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-threads>,
	<mailto:freebsd-threads-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 07 May 2003 20:28:35 -0000



On Tue, 6 May 2003, David Xu wrote:

> > > 
> > > And should we disable single threading testing or do
> > > double checking in thread_user_enter()? I think per-syscall
> > > PROC_LOCK is too expensive for us.
> > 
> > I am not sure which one you refer too.. Which single_threading
> > test?
> > 
> Single threading testing in thread_user_enter(), someone put 
> a PROC_LOCK, quoted here:
>         /*              
>          * First check that we shouldn't just abort.
>          * But check if we are the single thread first!
>          */
>         PROC_LOCK(p);
>         if ((p->p_flag & P_SINGLE_EXIT) && (p->p_singlethread != td)) {
>                 mtx_lock_spin(&sched_lock);
>                 thread_stopped(p);
>                 thread_exit();
>                 /* NOTREACHED */
>         }
>         PROC_UNLOCK(p);
> 

I don't think that the lock should be needed..
The bit is set when another thread has decided to kill all other
threads, and it will not be unset until all the other threads have been
killed (including this one). The second clause (&& (p->p_singlethread !=
td)) is also realy un-needed as teh thread tha has called the
single-threading condition should not have been able to go back to
userland, and it is in fact suspended. (so I think it can never be
true).

This leaves us with:
        PROC_LOCK(p);
        if (p->p_flag & P_SINGLE_EXIT) {
                mtx_lock_spin(&sched_lock);
                thread_stopped(p);
                thread_exit();
                /* NOTREACHED */
        }
        PROC_UNLOCK(p);

but either P_SINGLE_EXIT is set or it is not.
if we miss the race we continue until we chack again later. (so what?)
We can also guarantee that it will never be UNSET before we do call
thread_exit() as that is the only condition that can clear it.
(the thread_exit() of the last thread (except the one who called the
singlthreading condition))


so I think it might be safe to cut this back to:
        if (p->p_flag & P_SINGLE_EXIT) {  
                PROC_LOCK(p);
                mtx_lock_spin(&sched_lock);
                thread_stopped(p);
                thread_exit();
                /* NOTREACHED */
        }

certainly it would be good to get this out of the path for every KSE
syscall.