> > The overlap check in ras_install is also over-complicated:
> > 
> > 	nras = 0;
> > 	LIST_FOREACH(rp, &p->p_raslist, ras_list) {
> > 		if (++nras >= ras_per_proc ||
> > 		    (addr < rp->ras_endaddr && endaddr > rp->ras_startaddr)) {
> > 			simple_unlock(&p->p_raslock);
> > 			return (EINVAL);
> > 		}
> > 	}
> 
> This only catches if new range is within existing range, not
> if it partially overlaps start or end of other range.

No - read it carefully. eg for:
    addr = 10
    endaddr = 20
    rp->ras_start_addr = 15
    rp->ras_end_addr = 25
(addr < rp->ras_endaddr && endaddr > rp->ras_startaddr)
is (10 < 25 && 20 > 15) which is true - and hence an error.

> > I also see no reason to have both p_lwplock and p_raslock, and propose
> > replacing both with a single p_lock.
> 
> This would probably be good. What are they used for? Just to prevent
> simultaneous access to the struct proc by several it's lwps? If that
> is the case, having only 'p_lwplock' would be adequate IMO.

There are (will be) places where proc structure fields need protecting
against accesses by other processes [1].  So calling the field p_lock
(because is locks proc structure fields) is better.

	David

[1] I want to be able to update a field in the parent (and don't want to
rely on the 'biglock').

-- 
David Laight: david@l8s.co.uk