Date: Tue, 3 Sep 2002 12:08:19 -0700 (PDT) From: Julian Elischer <julian@elischer.org> To: Alfred Perlstein <bright@mu.org> Cc: arch@freebsd.org Subject: Re: Process/thread states. Message-ID: <Pine.BSF.4.21.0209031144280.12482-100000@InterJet.elischer.org> In-Reply-To: <20020827000404.GM96154@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 26 Aug 2002, Alfred Perlstein wrote:
> * Julian Elischer <julian@elischer.org> [020826 17:00] wrote:
> >
> > Ok so I've mentionned this to a few peopel at different times and not got
> > any real feedback/pushback.. time fopr a wider audience.
> ...
> > e.g. something like: (yeah I know its messy but...):
> >
> >
> >
> > #define TD_ST_SUSPQ 0x01 /* uses runq field */
> > #define TD_ST_RUNQ 0x02 /* uses runq field */
> > #define TD_ST_RUNNING 0x03 /* uses no (virtual) field */
> > #define TD_ST_MTX 0x04 /* uses mtx field */
> > #define TD_ST_RQ_MASK 0x07 /* mask of non sleep states */
> > #define TD_ST_SLPQ 0x08 /* uses slpq field */
> > enum thread_state {
> > TDS_UNQUEUED = 0x00,
> > TDS_SLP = TD_ST_SLPQ,
> > TDS_RUNQ = TD_ST_RUNQ,
> > TDS_RUNNING = TD_ST_RUNNING,
> > TDS_SUSPENDED = TD_ST_SUSPQ,
> > TDS_MTX = TD_ST_MTX,
> > TDS_SUSP_SLP = TD_ST_SUSPQ|TD_ST_SLPQ,
> > TDS_RUN_SLP = TD_ST_RUNNING|TD_ST_SLPQ,
> > TDS_RUNQ_SLP = TD_ST_RUNQ|TD_ST_SLPQ,
> > TDS_MTX_SLP = TD_ST_MTX|TD_ST_SLPQ,
> > TDS_SWAPPING = TD_ST_SLPQ|TD_ST_RQ_MASK + 1,
> > TDS_IWAIT, /* needed? */
> > TDS_SURPLUS /* needed? */
> > };
>
> This seems good, another option would be to mostly expose things
> through a functional interface, like proc_set_swapping() or something.
In fact this is sort of what we have ended up doing..
we have refactored the thread states as shown above (aproximatly)
but we have changed all teh cases where thread state is manipulated or
tested to use a set of macros, such as:
TD_SET_SUSPENDED(td)
TD_SET_RUNQ(td)
TD_SET_RUNNING(td)
TD_SET_SLP(td)
TD_CLR_SLP(td)
TD_SET_SLEEPING(td)
TD_SET_SWAPPED(td)
and tests
TD_ON_SUSPQ(td)
TD_ON_RUNQ(td)
TD_IS_RUNNING(td)
TD_ON_SLPQ(td)
TD_IS_SLEEPING(td)
TD_IS_SWAPPED(td)
this changes a lot of code to look like this:
- } else if (td->td_wchan != NULL) {
+ } else if (TD_ON_SLPQ(td)) {
or
- if (td->td_state == TDS_SLP) {
+ if (TD_IS_SLEEPING(td)) {
+ TD_CLR_SLP(td);
or
- KASSERT((td->td_state != TDS_RUNQ), ("setrunqueue: bad thread
state"));
- td->td_state = TDS_RUNQ;
+ KASSERT(!TD_ON_RUNQ(td), ("setrunqueue: bad thread state"));
+ TD_SET_RUNQ(td);
or more importantly:
- } else if (td->td_wchan != NULL) {
- if (td->td_state == TDS_SLP) /* XXXKSE */
+ } else if (TD_ON_SLPQ(td)) {
+ if (TD_IS_SLEEPING(td)) /* XXXKSE */
which I think is a lot more readable.
Once the macros are in place we actually have the oportunity
to tune the implementations somewhat, and as KSE operations are
implemented and mode code is changed to handle multiple threads
the chances that we will need to tune the states will increase.
Macros will insulate us from some of this..
Is there anyone who thinks that doing these changes
is a REALLY BAD IDEA?
We'd (david and I) like to go ahead and provide a general patch to
implement this, for others to peruse, but we'd like to
hear if there will be screaming first before we do more work on it..
>
> -Alfred
>
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-arch" in the body of the message
>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0209031144280.12482-100000>