Date: Fri, 11 Jul 2014 13:19:25 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik <mjg@FreeBSD.org> Subject: Re: svn commit: r267760 - head/sys/kern Message-ID: <20140711111925.GB18214@dft-labs.eu> In-Reply-To: <20140711095551.GA93733@kib.kiev.ua> References: <201406230128.s5N1SIYK097224@svn.freebsd.org> <20140623064044.GD93733@kib.kiev.ua> <20140623070652.GA27040@dft-labs.eu> <20140623072519.GE93733@kib.kiev.ua> <20140623080501.GB27040@dft-labs.eu> <20140623081823.GG93733@kib.kiev.ua> <20140623131653.GC27040@dft-labs.eu> <20140623163523.GK93733@kib.kiev.ua> <20140711024351.GA18214@dft-labs.eu> <20140711095551.GA93733@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140711111925.GB18214>