Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 16 Apr 2005 12:54:44 GMT
From:      David Xu <davidxu@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 75318 for review
Message-ID:  <200504161254.j3GCsi5N068447@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help 
http://perforce.freebsd.org/chv.cgi?CH=75318

Change 75318 by davidxu@davidxu_tiger on 2005/04/16 12:54:24

	1. Save tid to parent thread correctly.
	2. Update comments.
	3. Fix a long standing bug when thread is created with
	   THR_SUSPENDED, thr_wake can not resume it! (julian ?)

Affected files ...

.. //depot/projects/davidxu_thread/src/sys/kern/kern_thr.c#15 edit
.. //depot/projects/davidxu_thread/src/sys/sys/thr.h#7 edit

Differences ...

==== //depot/projects/davidxu_thread/src/sys/kern/kern_thr.c#15 (text+ko) ====

@@ -183,6 +183,7 @@
 
 int
 thr_create2(struct thread *td, struct thr_create2_args *uap)
+    /* struct thr_param * */
 {
 	struct thr_param param;
 	stack_t stack;
@@ -204,7 +205,11 @@
 	    (p->p_numthreads >= max_threads_per_proc)) {
 		return (EPROCLIM);
 	}
+
+	/* Check PTHREAD_SCOPE_SYSTEM */
 	scope_sys = (param.flags & THR_SYSTEM_SCOPE) != 0;
+
+	/* sysctl overrides user's flag */
 	if (thr_scope == 1)
 		scope_sys = 0;
 	else if (thr_scope == 2)
@@ -214,12 +219,17 @@
 	newtd = thread_alloc();
 
 	/*
-	 * Try the copyout as soon as we allocate the td so we don't have to
-	 * tear things down in a failure case below.
+	 * Try the copyout as soon as we allocate the td so we don't
+	 * have to tear things down in a failure case below.
+	 * Here we copy out tid to two places, one for child and one
+	 * for parent, because pthread can create a detached thread,
+	 * if parent wants to safely access child tid, it has to provide 
+	 * its storage, because child thread may exit quickly and
+	 * memory is freed before parent thread can access it.
 	 */
 	id = newtd->td_tid;
 	if ((error = copyout(&id, param.child_tid, sizeof(long))) ||
-	    (error = copyout(&id, &param.parent_tid, sizeof(long)))) {
+	    (error = copyout(&id, param.parent_tid, sizeof(long)))) {
 		thread_free(newtd);
 		return (error);
 	}
@@ -240,6 +250,7 @@
 	/* Setup user TLS address and TLS pointer register. */
 	cpu_set_user_tls(newtd, param.tls_base, param.tls_size, param.tls_seg);
 	if ((td->td_proc->p_flag & P_HADTHREADS) == 0) {
+		/* Treat initial thread as it has PTHREAD_SCOPE_PROCESS. */
 		p->p_procscopegrp = kg;
 		mtx_lock_spin(&sched_lock);
 		sched_set_concurrency(kg,
@@ -258,6 +269,10 @@
 		sched_init_concurrency(newkg);
 		PROC_LOCK(td->td_proc);
 	} else {
+		/*
+		 * Try to create a KSE group which will be shared
+		 * by all PTHREAD_SCOPE_PROCESS threads.
+		 */
 retry:
 		PROC_LOCK(td->td_proc);
 		if ((newkg = p->p_procscopegrp) == NULL) {
@@ -329,10 +344,6 @@
 		suword((void *)uap->state, 1);
 
 	PROC_LOCK(p);
-	#if 0
-	SIGFILLSET(td->td_siglist);
-	sigrepost(td);
-	#endif
 	mtx_lock_spin(&sched_lock);
 	/*
 	 * Shutting down last thread in the proc.  This will actually
@@ -439,8 +450,13 @@
 	}
 	mtx_lock_spin(&sched_lock);
 	ttd->td_flags |= TDF_THRWAKEUP;
-	mtx_unlock_spin(&sched_lock);
-	wakeup((void *)ttd);
+	if (TD_CAN_RUN(ttd)) { /* if thread was created with suspended */
+		setrunnable(ttd);
+		mtx_unlock_spin(&sched_lock);
+	} else {
+		mtx_unlock_spin(&sched_lock);
+		wakeup((void *)ttd);
+	}
 	PROC_UNLOCK(td->td_proc);
 	return (0);
 }

==== //depot/projects/davidxu_thread/src/sys/sys/thr.h#7 (text+ko) ====

@@ -34,10 +34,6 @@
 #define	THR_SUSPENDED		0x0001
 /* Create the system scope thread. */
 #define	THR_SYSTEM_SCOPE	0x0002
-/* Remember user TID address in kernel, when exits,
-   write zero to the address and do umtx_wake()
-   on the address, the pthread_join can use this feature. */
-#define	THR_REMBER_TID_ADDRESS	0x0004
 
 struct thr_param {
     void	(*start_func)(void *);	/* thread entry function. */
@@ -48,7 +44,7 @@
     size_t	tls_size;		/* tls size. */
     int		tls_seg;		/* which seg is to set for tls. */
     long	*child_tid;		/* address to store new TID. */
-    long	parent_tid;		/* parent accesses the new TID here. */
+    long	*parent_tid;		/* parent accesses the new TID here. */
     int		*user_crit;		/* reserved */
     int		flags;
 };

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200504161254.j3GCsi5N068447>