Subject: Re: tsleep() in vfs_shutdown (update)
To: Manuel Bouyer <bouyer@antioche.lip6.fr>
From: Nathan J. Williams <nathanw@MIT.EDU>
List: tech-kern
Date: 08/27/2000 17:20:39
<bouyer@antioche.lip6.fr> (Manuel Bouyer) writes:

> attached is an updated patch to allow tsleep() in vfs_shutdown() instead of
> delay(). Per Jason's comments, setrunnable() & friends now mark processes
> SHOLD instead of SRUN when needed. I also renamed stopsched() to
> suspendsched() and added a resumesched(), to put back SHOLD processes in the
> run queue.

Scheduler fiddling like this makes me nervous.

Why is a new process state needed? We already have SSTOP; processes
placed in that state aren't going anywhere. I don't see any code that
actually calls resumesched(), and the goal of this code is to allow
sleeping and scheduling during shutdown, so I don't see the harm in
using this state for that purpose.

> suspendsched()/resumesched() uses a counter, so that it's possible to call
> suspendsched() from different places without special care. I added a new
> p_flag, P_HOLD, set for processes which should be set SHOLD rather than
> SRUN when they become runnable again.

Can the flag be named something else? P_HOLD is just going to invite
confusion with PHOLD(), and the two have nothing to do with each
other.


> Index: kern/kern_fork.c
> ===================================================================
> RCS file: /cvsroot/syssrc/sys/kern/kern_fork.c,v
> retrieving revision 1.66.2.1
> diff -u -r1.66.2.1 kern_fork.c
> --- kern/kern_fork.c	2000/07/04 16:05:34	1.66.2.1
> +++ kern/kern_fork.c	2000/08/27 19:16:17
> @@ -290,7 +290,7 @@
>  	 * Increase reference counts on shared objects.
>  	 * The p_stats and p_sigacts substructs are set in vm_fork.
>  	 */
> -	p2->p_flag = P_INMEM | (p1->p_flag & P_SUGID);
> +	p2->p_flag = P_INMEM | (p1->p_flag & (P_SUGID | P_HOLD));

When is a process with P_HOLD set going to call fork1()?

I'll figure out the other reasons that this bothers me soon...

        - Nathan