On Tue, Jun 14, 2016 at 11:40 PM, Selva Nair <selva.n...@gmail.com> wrote:
> > >> But I'm open to "semaphores are cheap, we can just open/lock/release/ >> close >> it on every call" arguments :-) >> > > Very unlikely to be a drag on performance -- I think createprocess will be > where time is spent [*]. > > That said, calling semaphore_close() inside netcmd_semaphore_unlock() is > not a style to be proud of. So I'm kind of inclined to do a version 2 > along the lines you suggested but some testing will be needed to get it > right. After looking into this further, I find redoing the patch along those lines is not ideal. A semaphore_close() has to be inserted before any possible long waits such as management-hold, pull-filter reject, or any other restart loops, which is going to be ugly and fragile. The cleanest way is to close the semaphore soon after releasing it. Version 1 does that and I do not think there is any performance penalty involved. Logically, a more correct approach may be to replace all calls to netcmd_semaphore_release() by netcmd_semaphore_close() but that involves more pervasive edits. Having said that, I'm open to other suggestions. Selva
