Re: cvs commit: src/sys/ddb db_ps.c src/sys/i386/i386 trap.c vm86.c src/sys/kern init_main.c kern_clock.c kern_exit.c kern_fork.c kern_resource.c kern_sig.c kern_synch.c kern_upcall.c lwkt_thread.c usched_bsd4.c vfs_aio.c src/sys/netproto/smb ...

["Simon 'corecode' Schubert" <corecode@xxxxxxxxxxxx>]

Matthew Dillon wrote:
> :http://www.dragonflybsd.org/cvsweb/src/sys/kern/init_main.c.diff?r1=1.47&r2=1.48&f=u
>
>     We want to avoid using LIST_FIRST for this case.  e.g. you are doing 
>
>     p = curproc;
>     lp = LIST_FIRST(...)
>
>     Instead of doing that, use curthread->td_lwp (assuming its set up at that point).

Yea, my bad, I overlooked that.

> :http://www.dragonflybsd.org/cvsweb/src/sys/kern/kern_exit.c.diff?r1=1.46&r2=1.47&f=u
>
>     p->p_usched->heuristic_exiting(q, p);
>     p->p_usched->heuristic_exiting(td->td_lwp, &p->p_lwp);
>
>     Avoid using &p->p_lwp if possible, since the embedded lwp is going to go away at 
>     some point.  If this code is going to have to deal with all the LWPs then put an
>     XXX comment in there or do a LIST_FOREACH and/or otherwise assert that there is
>     only one lwp so this doesn't bite us in the ass later on.

My idea was to use p_lwp to signal that this place needs to be revisited 
in one way or the other instead of using a bogus LIST_FIRST(). This 
helps me keeping correct code which i can find again lateron by 
searching for p_lwp.

In the process of converting functions to use lwp, the use of p->p_lwp 
will automatically be replaced by lp.

> :http://www.dragonflybsd.org/cvsweb/src/sys/kern/vfs_aio.c.diff?r1=1.18&r2=1.19&f=u
>
>     Same issue.  Deserves a comment at least (or maybe fork should take a proc pointer,
>     since you can't fork an LWP anyway).
>
> -	p = &proc0;
> -	error = fork1(p, RFPROC|RFMEM|RFNOWAIT, &np);
> +	lp = &proc0.p_lwp;
> +	error = fork1(lp, RFPROC|RFMEM|RFNOWAIT, &np);

Fork needs to take a lwp because the new proc's only lwp will be a copy 
of the current lwp.  When we split out lwp from proc we'll have a lwp0 
anyways.

> :http://www.dragonflybsd.org/cvsweb/src/sys/netproto/smb/smb_subr.c.diff?r1=1.14&r2=1.15&f=u
>
>     Same issue.  I think it's becoming clear that we want fork*() to take a proc pointer.
>
> -	error = fork1(&proc0, RFMEM | RFFDG | RFPROC | flags, &p2);
> +	error = fork1(&proc0.p_lwp, RFMEM | RFFDG | RFPROC | flags, &p2);
>
> -	start_forked_proc(&proc0, p2);
> +	start_forked_proc(&proc0.p_lwp, p2);

Same again.  lwp0 will do the job here.

> :http://www.dragonflybsd.org/cvsweb/src/sys/vm/vm_glue.c.diff?r1=1.34&r2=1.35&f=u
>
>     Same issue.  Add XXX comment, KKASERT that there is only one thread if accessing
>     &p->p_lwp.
>
> -		p->p_usched->remrunqueue(p);
> +		p->p_usched->remrunqueue(&p->p_lwp);

No, we need to loop through all lwps, but I wanted to do this in a later 
step (3, I think).

> :http://www.dragonflybsd.org/cvsweb/src/sys/vm/vm_pageout.c.diff?r1=1.14&r2=1.15&f=u
> :
>
>     Same issue.
>
> -			bigproc->p_usched->resetpriority(bigproc);
> +			bigproc->p_usched->resetpriority(&bigproc->p_lwp);
>
>     Why is this important?  Even though these steps are intermediate steps and
>     will undergo further changes, we do not want the complexity of the work to introduce
>     bugs as it progresses.  It is better to assert the assumed condition in the intermediate
>     step and take a panic later on when you've forgotten something than for the system 
>     to become corrupt and take hours to debug.

I try to avoid producing avalances by forcing all functions down the 
code path to use lwp as well, so I'm using p->p_lwp and lp->lwp_proc to 
keep stuf from propagating.  This won't introduce bugs at the moment, 
because they are exchangable.  As we proceed converting functions these 
uses will go away.  Basically p_lwp is a compiletime XXX:  if there is 
still code that uses p_lwp, we're not ready yet to use multiple threads 
per process.

cheers
  simon

--
Serve - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /"\
Work - Mac      +++  space for low $$$ NOW!1  +++      Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \