Re: need better POSIX semaphore support

On Tuesday 01 June 2010 1:05:26 pm Kostik Belousov wrote:
> On Tue, Jun 01, 2010 at 11:41:09AM -0400, John Baldwin wrote:
> > On Sunday 30 May 2010 11:06:22 am Kostik Belousov wrote:
> > > On Sun, May 30, 2010 at 06:30:35PM +0400, Dmitry Marakasov wrote:
> > > > Hi!
> > > > 
> > > > Not long ago, POSIX semaphores support was enabled by default as it's
> > > > becoming more widely used, by e.g. firefox. However, the support
> > > > for these is still incomplete: we only have systemwide limit of 30
> > > > semaphores, and that doesn't seem to be configurable neither online with
> > > > sysctl, nor at boottime from loader.conf. I only was able to raise
> > > > semaphore count by changing SEM_MAX in kernel sources.
> > > > 
> > > > The real appliaction which needs more semaphores is lightspark
> > > > (graphics/lightspark-devel) flash plugin - it uses ~40 sems for simple
> > > > clips and ~250 for something like youtube videos.
> > > > 
> > > > Until there more apps that require proper semaphore support, I guess
> > > > we need to improve it asap. Given the amount of memory used by ksem,
> > > > the least can be done is SEM_MAX bumped up to 5120 or so for
> > > > non-embedded kernels. 5120 semaphores require just 644k of kernel
> > > > memory (judging by vmstat), and is "ought to be enough for anybody".
> > > > Another good thing would be to make it configurable at boot-time
> > > > or even better in runtime.
> > > 
> > > HEAD contains different implementation. Apparently, it did not made
> > > into stable/8 yet, so it will not appear in the 8.1.
> > 
> > The one thing I don't like about this approach is you can write the
> > variable even when sem.ko isn't loaded. The SEM_* values should really
> > only exist when sem.ko is loaded I think, which requires moving them
> > into uipc_sem.c.
> 
> I think the values should exist always, because sysconf(3) returns
> error (i.e. -1 and errno set) when sysctl fails. sysconf(3) interprets
> 0 result as "feature not supported".
> 
> I modified the patch to only allow change of value when the module is loaded.
> Also, the module unload now clears mib. As usual, module unload races are
> not handled.

I think this looks good, thanks!

-- 
John Baldwin