Subject: Re: tsleep() in vfs_shutdown (update)
To: Nathan J. Williams <nathanw@MIT.EDU>
From: Manuel Bouyer <bouyer@antioche.lip6.fr>
List: tech-kern
Date: 08/28/2000 12:18:52
On Sun, Aug 27, 2000 at 05:20:39PM -0400, Nathan J. Williams wrote:
> <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

Yes, but then I can't make a difference between processes which were
SSTOP before, and processes SSTOP by suspendshed()

> 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.

The goal of the code is more general than this (actually allowing
suspend/resume of sheduling of some class of processes); its immediate use
is for vfs_shutdown().
> 
> > 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.

P_SUSPEND then ?

> 
> 
> > 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()?

fork1() is:
int fork1(struct proc *p1, int flags, int exitsig, void *stack,
  size_t stacksize, void (*func)(void *), void *arg, register_t *retval,
  struct proc **rnewprocp)

So fork1() can be called with p1 != curproc (p1 may be suspended, but
not curproc). Now, in the current code nothing may do this, but I can't see
any reason to dissalow it (otherwise change the interface to remove p1 and
always use curproc).

--
Manuel Bouyer, LIP6, Universite Paris VI.           Manuel.Bouyer@lip6.fr
--