svn commit: r267760 - head/sys/kern

Mateusz Guzik mjguzik at gmail.com
Fri Jul 11 11:19:38 UTC 2014


On Fri, Jul 11, 2014 at 12:55:51PM +0300, Konstantin Belousov wrote:
> On Fri, Jul 11, 2014 at 04:43:51AM +0200, Mateusz Guzik wrote:
> > In both cases the same mechanism blocks both exec and exit, this can be
> > split if needed (p_lock would still cover exit, p_something would cover
> > exec).
> > 
> > Here is a version with sx lock:
> > 
> > http://people.freebsd.org/~mjg/patches/exec-exit-hold-wait.patch
> > 
> > I'm not really happy with this. Reading foreign fdt is very rare and
> > this adds lock + unlock for every exec and exit.
> > 
> > On the other hand mere counter version is rather simple:
> > 
> > http://people.freebsd.org/~mjg/patches/exec-exit-hold-nolock.patch
> > 
> > I don't have strong opinion here, but prefer the latter.
> 
> I suggest the name 'imagelock' for the beast.
> 

Sounds good.

> The nolock version requires two atomics on both entry and leave from the
> protected region, while sx-locked variant requires only one atomic for
> entry and leave.
> 
> I am not sure why you decided to acquire p->p_keeplock in after the
> proc lock in pget(), which indeed causes the complications of dropping
> the proc_lock and rechecking to avoid LOR.  Did you tried to add a flag
> to pfind*() functions to indicate that p_keeplock should be acquired,
> instead ?

Lock is taken later to avoid waiting for finished exec/exit of processes
we cannot return, so that e.g. procstat -fa does not trip over that
much.

Right now only PROC_LOCK guarantees stability of p->p_ucred across pget
operation. Without that the code would have to crget() and various
functions modified to accept cred instead of proc, or 'imagelock'
mechanism would have to be extended to also protect against cred
changes.

That said, the code could be indeed changed to sx requiring one atomic
on entry and leave, but that would still leave us with such atomics in
exit and exec and the last 2 are way more common than the first one,
thus I prefer counter case which only adds lock + unlock on leave.

-- 
Mateusz Guzik <mjguzik gmail.com>


More information about the svn-src-head mailing list