Threads can not be included in more than one threadlist.

[ahmednofal]

Hello, thanks for the great project,

The current implementation of the threadlist uses nodes embedded in the thread struct itself, which prevents a thread from being in two threadlists simultaneously.

An example of the API provided by the threadlist which uses the inner threadlistnode struct instance inside the thread instance, which is singular

void
threadlist_addhead(struct threadlist *tl, struct thread *t)
{
	DEBUGASSERT(tl != NULL);
	DEBUGASSERT(t != NULL);

	threadlist_insertafternode(&tl->tl_head, t);
	tl->tl_count++;
}

which is dependent on :

static
void
threadlist_insertafternode(struct threadlistnode *onlist, struct thread *t)
{
	struct threadlistnode *addee;

	addee = &t->t_listnode;

	DEBUGASSERT(addee->tln_prev == NULL);
	DEBUGASSERT(addee->tln_next == NULL);

	addee->tln_prev = onlist;
	addee->tln_next = onlist->tln_next;
	addee->tln_prev->tln_next = addee;
	addee->tln_next->tln_prev = addee;
}

as you can see it is using the inner struct pointer to modify the structure of the list

void
threadlist_remove(struct threadlist *tl, struct thread *t)
{
	threadlist_removenode(&t->t_listnode);
	DEBUGASSERT(tl->tl_count > 0);
	tl->tl_count--;
}

static
void
threadlist_removenode(struct threadlistnode *tln)
{
	DEBUGASSERT(tln != NULL);
	DEBUGASSERT(tln->tln_prev != NULL);
	DEBUGASSERT(tln->tln_next != NULL);

	tln->tln_prev->tln_next = tln->tln_next;
	tln->tln_next->tln_prev = tln->tln_prev;
	tln->tln_prev = NULL;
	tln->tln_next = NULL;
}

And the thread struct itself :

struct thread {
	/*
	 * These go up front so they're easy to get to even if the
	 * debugger is messed up.
	 */

	/*
	 * Name of this thread. Used to be dynamically allocated using kmalloc, but
	 * this can cause small changes in the amount of available memory due to the
	 * fact that it was cleaned up in exorcise. This produces more predictable
	 * behavior at the cost of a small amount of memory overhead and the
	 * inability to give threads huge names.
	 */

	char t_name[MAX_NAME_LENGTH];
	const char *t_wchan_name;	/* Name of wait channel, if sleeping */
	threadstate_t t_state;		/* State this thread is in */

	/*
	 * Thread subsystem internal fields.
	 */
	struct thread_machdep t_machdep; /* Any machine-dependent goo */
	struct threadlistnode t_listnode; /* Link for run/sleep/zombie lists */
	void *t_stack;			/* Kernel-level stack */
	struct switchframe *t_context;	/* Saved register context (on stack) */
	struct cpu *t_cpu;		/* CPU thread runs on */
	struct proc *t_proc;		/* Process thread belongs to */
	HANGMAN_ACTOR(t_hangman);	/* Deadlock detector hook */

	/*
	 * Interrupt state fields.
	 *
	 * t_in_interrupt is true if current execution is in an
	 * interrupt handler, which means the thread's normal context
	 * of execution is stopped somewhere in the middle of doing
	 * something else. This makes assorted operations unsafe.
	 *
	 * See notes in spinlock.c regarding t_curspl and t_iplhigh_count.
	 *
	 * Exercise for the student: why is this material per-thread
	 * rather than per-cpu or global?
	 */
	bool t_in_interrupt;		/* Are we in an interrupt? */
	int t_curspl;			/* Current spl*() state */
	int t_iplhigh_count;		/* # of times IPL has been raised */

	/*
	 * Public fields
	 */

	/* add more here as needed */
};

This issue arouse while trying to implement the rwlock synch primitive for assgn1, I wanted to include a struct threadlist readers in the struct rwlock. If any of those threads are on a wchan threadlist and through using the threadlist API mentioned above they will be removed from the wchan list and put on a different (readers in this case) threadlist.

A better implementation (but less efficient) would be to duplicate the node in the process and insert it in the theadlist as a means to separate the threadlistnode from the thread instance itself.