Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 10 May 2002 13:21:09 -0700
From:      Julian Elischer <julian@elischer.org>
To:        Jonathan Mini <mini@FreeBSD.org>
Cc:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   Re: PERFORCE change 11120 for review
Message-ID:  <3CDC2BB5.366A7BF0@elischer.org>
References:  <200205101530.g4AFUn510685@freefall.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Jonathan Mini wrote:
> 
> http://people.freebsd.org/~peter/p4db/chv.cgi?CH=11120
> 
> Change 11120 by mini@mini_stylus on 2002/05/10 08:30:36
> 
>         Give UMA control over thread allocation and caching:
> 
>         - Use uma init/fini/ctor/dtor functions.

good

>         - Instead of keeping a PCPU list of zombie threads to be
>           reaped at earliest convenvience, keep a single "spare"
>           thread in the KSE.

clever.. not sure it isn't too clever.. but looks good.

>         - When abandoning a thread (in thread_exit()), push the
>           thread into its KSE's spare thread slot, and free the
>           thread that is already there (if any).

I assume there will only be a spare thread at other times in KSE type processes.

>         - When performing an upcall, pull the spare thread (if
>           available) before allocating a new thread from uma. This
>           is especially useful in msleep(), where not blocking again
>           is highly preferable.
>         - When pulling the KSE spare thread, allocate a new spare
>           thread for the KSE before returning to userland for the
>           upcall.

I presume only for KSE mode processes.
i.e. for KSE mode processes there is always a spare thread available 
unless we have just used it and have not yet returned to userland.
I can see some holes may need patching but should work..

> 

> +       /* Reassign this thread's KSE. */
> +       if (ke != NULL) {
> +           ke->ke_thread = NULL;

WHOA! just because ke == td->td_kse doesn't mean that
ke->ke_thread == td  

UNLESS you can guarantee that thread unlink is always run with td == curthread
or td->td_state == TDS_RUNNING.



> +           td->td_kse = NULL;
> +           ke->ke_state = KES_UNQUEUED;
> +           kse_reassign(ke);
> +       }
> 
> -               /* put the thread back in the zone */
> -               uma_zfree(thread_zone, td);
> +       /* Unlink this thread from its proc. */
> +       if (p != NULL) {
> +           TAILQ_REMOVE(&p->p_threads, td, td_plist);
> +           if (kg != NULL)
> +               TAILQ_REMOVE(&kg->kg_threads, td, td_kglist);
> +           p->p_numthreads--;

    what of the kg->kg_numthreads..

> +           if (P_SHOULDSTOP(p) == P_STOPPED_SNGL) {
> +               if (p->p_numthreads ==
> +                   ((p->p_flag & P_SINGLE_EXIT) ? 1 : (p->p_suspcount + 1))) {
> +                       setrunqueue(p->p_singlethread);
> +                       p->p_singlethread = NULL;
> +               }
> +           }
>         }
> +       if (kg != NULL)
> +           kg->kg_numthreads--;

move this up?

> +       td->td_state    = TDS_SURPLUS;
> +       td->td_proc     = NULL;
> +       td->td_ksegrp   = NULL;
> +       td->td_last_kse = NULL;
> +}
> +
> +/*
> + * Initialize global thread allocation resources.
> + */
> +void
> +threadinit(void)
> +{
> +
> +       thread_zone = uma_zcreate("THREAD", sizeof (struct thread),
> +           thread_ctor, thread_dtor, thread_init, thread_fini,
> +           UMA_ALIGN_CACHE, 0);
>  }
> 

plus other comments already noted by others..
(tabbing and sleeping with locks)

-- 
+------------------------------------+       ______ _  __
|   __--_|\  Julian Elischer         |       \     U \/ / hard at work in 
|  /       \ julian@elischer.org     +------>x   USA    \ a very strange
| (   OZ    )                                \___   ___ | country !
+- X_.---._/    presently in San Francisco       \_/   \\
          v


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe p4-projects" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3CDC2BB5.366A7BF0>