Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 1 Jun 2010 20:05:26 +0300
From:      Kostik Belousov <kostikbel@gmail.com>
To:        John Baldwin <jhb@freebsd.org>
Cc:        freebsd-current@freebsd.org, freebsd-stable@freebsd.org, Dmitry Marakasov <amdmi3@amdmi3.ru>
Subject:   Re: need better POSIX semaphore support
Message-ID:  <20100601170526.GJ83316@deviant.kiev.zoral.com.ua>
In-Reply-To: <201006011141.09699.jhb@freebsd.org>
References:  <20100530143034.GH43302@hades.panopticon> <20100530150622.GJ83316@deviant.kiev.zoral.com.ua> <201006011141.09699.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--zruoG3PDhkeTLwq7
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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!
> > >=20
> > > 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 w=
ith
> > > sysctl, nor at boottime from loader.conf. I only was able to raise
> > > semaphore count by changing SEM_MAX in kernel sources.
> > >=20
> > > 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.
> > >=20
> > > 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.
> >=20
> > HEAD contains different implementation. Apparently, it did not made
> > into stable/8 yet, so it will not appear in the 8.1.
>=20
> 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 loade=
d.
Also, the module unload now clears mib. As usual, module unload races are
not handled.

diff --git a/sys/kern/posix4_mib.c b/sys/kern/posix4_mib.c
index 5242b31..cbe140d 100644
--- a/sys/kern/posix4_mib.c
+++ b/sys/kern/posix4_mib.c
@@ -45,6 +45,8 @@ __FBSDID("$FreeBSD$");
 static int facility[CTL_P1003_1B_MAXID - 1];
 static int facility_initialized[CTL_P1003_1B_MAXID - 1];
=20
+static int p31b_sysctl_proc(SYSCTL_HANDLER_ARGS);
+
 /* OID_AUTO isn't working with sysconf(3).  I guess I'd have to
  * modify it to do a lookup by name from the index.
  * For now I've left it a top-level sysctl.
@@ -55,16 +57,21 @@ static int facility_initialized[CTL_P1003_1B_MAXID - 1];
 SYSCTL_DECL(_p1003_1b);
=20
 #define P1B_SYSCTL(num, name)  \
-SYSCTL_INT(_p1003_1b, num, \
-	name, CTLFLAG_RD, facility + num - 1, 0, "");
+	SYSCTL_INT(_p1003_1b, num, name, CTLFLAG_RD, facility + num - 1, 0, "");
+#define P1B_SYSCTL_RW(num, name)  \
+	SYSCTL_PROC(_p1003_1b, num, name, CTLTYPE_INT | CTLFLAG_RW, NULL, num, \
+	    p31b_sysctl_proc, "I", "");
=20
 #else
=20
 SYSCTL_DECL(_kern_p1003_1b);
=20
 #define P1B_SYSCTL(num, name)  \
-SYSCTL_INT(_kern_p1003_1b, OID_AUTO, \
-	name, CTLFLAG_RD, facility + num - 1, 0, "");
+	SYSCTL_INT(_kern_p1003_1b, OID_AUTO, name, CTLFLAG_RD, \
+	    facility + num - 1, 0, "");
+#define P1B_SYSCTL_RW(num, name)  \
+	SYSCTL_PROC(_p1003_1b, OID_AUTO, name, CTLTYPE_INT | CTLFLAG_RW, NULL, \
+	    num, p31b_sysctl_proc, "I", "");
 SYSCTL_NODE(_kern, OID_AUTO, p1003_1b, CTLFLAG_RW, 0, "P1003.1B");
=20
 #endif
@@ -91,13 +98,28 @@ P1B_SYSCTL(CTL_P1003_1B_DELAYTIMER_MAX, delaytimer_max);
 P1B_SYSCTL(CTL_P1003_1B_MQ_OPEN_MAX, mq_open_max);
 P1B_SYSCTL(CTL_P1003_1B_PAGESIZE, pagesize);
 P1B_SYSCTL(CTL_P1003_1B_RTSIG_MAX, rtsig_max);
-P1B_SYSCTL(CTL_P1003_1B_SEM_NSEMS_MAX, sem_nsems_max);
+P1B_SYSCTL_RW(CTL_P1003_1B_SEM_NSEMS_MAX, sem_nsems_max);
 P1B_SYSCTL(CTL_P1003_1B_SEM_VALUE_MAX, sem_value_max);
 P1B_SYSCTL(CTL_P1003_1B_SIGQUEUE_MAX, sigqueue_max);
 P1B_SYSCTL(CTL_P1003_1B_TIMER_MAX, timer_max);
=20
 #define P31B_VALID(num)	((num) >=3D 1 && (num) < CTL_P1003_1B_MAXID)
=20
+static int
+p31b_sysctl_proc(SYSCTL_HANDLER_ARGS)
+{
+	int error, num, val;
+
+	num =3D arg2;
+	if (!P31B_VALID(num))
+		return (EINVAL);
+	val =3D facility_initialized[num] ? facility[num - 1] : 0;
+	error =3D sysctl_handle_int(oidp, &val, 0, req);
+	if (error =3D=3D 0 && req->newptr !=3D NULL && facility_initialized[num])
+		facility[num - 1] =3D val;
+	return (error);
+}
+
 /* p31b_setcfg: Set the configuration
  */
 void
@@ -110,6 +132,14 @@ p31b_setcfg(int num, int value)
 	}
 }
=20
+void
+p31b_unsetcfg(int num)
+{
+
+	facility[num - 1] =3D 0;
+	facility_initialized[num -1] =3D 0;
+}
+
 int
 p31b_getcfg(int num)
 {
diff --git a/sys/kern/uipc_sem.c b/sys/kern/uipc_sem.c
index d9229ea..0b8f132 100644
--- a/sys/kern/uipc_sem.c
+++ b/sys/kern/uipc_sem.c
@@ -976,6 +976,8 @@ ksem_module_destroy(void)
 	sx_destroy(&ksem_dict_lock);
 	mtx_destroy(&ksem_count_lock);
 	mtx_destroy(&sem_lock);
+	p31b_unsetcfg(CTL_P1003_1B_SEM_VALUE_MAX);
+	p31b_unsetcfg(CTL_P1003_1B_SEM_NSEMS_MAX);
 }
=20
 static int
diff --git a/sys/sys/posix4.h b/sys/sys/posix4.h
index ea379c0..34f77f4 100644
--- a/sys/sys/posix4.h
+++ b/sys/sys/posix4.h
@@ -64,6 +64,7 @@ int p31b_proc(struct proc *, pid_t, struct proc **);
 void p31b_setcfg(int, int);
 int p31b_getcfg(int);
 int p31b_iscfg(int);
+void p31b_unsetcfg(int);
=20
 #ifdef _KPOSIX_PRIORITY_SCHEDULING
=20

--zruoG3PDhkeTLwq7
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (FreeBSD)

iEYEARECAAYFAkwFPdYACgkQC3+MBN1Mb4icFwCeLtYMI7m6kuZ5t9X8YNFEg8dQ
7FkAn24sDS1zePA7HC5oDcgQ3cP3Gbu3
=T0aL
-----END PGP SIGNATURE-----

--zruoG3PDhkeTLwq7--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100601170526.GJ83316>