kern_threads.c.. lock question..

[John Baldwin]

On 07-May-2003 Julian Elischer wrote:
> 
> 
> 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).

I thought singlethreading only suspended other threads and that
they can be resumed later if need be.  I suppose in that case
you wouldn't use P_SINGLE_EXIT though.

> 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.

Please let's optimize later and get things working _first_.
Right now current has this really annoying piss-ant of a bug
where I lose my keyboard every so often in X, probably due to
a locking bug I haven't managed to track down yet (feels an
awful lot like a lost wakeup).  If you want to remove the
p_singlethread check, that is fine.  Part of the reason I
put the lock there was so that the check of multiple values
would at least get a consistent snapshot to work with when
making its decision.

-- 

John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/