Static tunable semvmx and semaem

Hi,
Please find below patch(RFC) for implementing semvmx and semaem as static tunable parameters.

These can be modified at boot time using command line interface.

Please comment/suggest.

cheers,
Arvind

diff -Naur linux-2.5.70/include/linux/sysctl.h linux-2.5.70.n/include/linux/sysctl.h
@@ -130,6 +130,8 @@
 	KERN_PIDMAX=55, 

	/* int: PID # limit */

KERN_CORE_PATTERN=56, /* string: pattern for core-file names */
KERN_PANIC_ON_OOPS=57, /* int: whether we will panic on an oops */
+ KERN_SEMVMX=58, /* int: maximum limit on semval */
+ KERN_SEMAEM=59,

	/* int: maximun limit on semaem */

};

diff -Naur linux-2.5.70/init/main.c linux-2.5.70.n/init/main.c
@@ -67,6 +67,9 @@
 
 extern char *linux_banner;
 
+extern unsigned int semvmx;
+extern unsigned int semaem;
+
 static int init(void *);
 
 extern void init_IRQ(void);

@@ -141,6 +144,29 @@
 
 __setup("maxcpus=", maxcpus);
 
+static int __init _semvmx(char *str)
+{
+ get_option(&str, &semvmx);
+ if(semvmx>65535 || semvmx <=0)
+ {
+ 

	semvmx=32767;

+ }
+ return 1;
+}
+__setup("semvmx=", _semvmx);
+
+static int __init _semaem(char *str)
+{
+ get_option(&str, &semaem);
+ if(semaem>32767 || semaem <=0)
+ {
+

	semaem=16384;

+ }
+ return 1;
+}
+__setup("semaem=", _semaem);
+
+
static char * argv_init[MAX_INIT_ARGS+2] = { "init", NULL, }; char * envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, };

diff -Naur linux-2.5.70/ipc/sem.c linux-2.5.70.n/ipc/sem.c
@@ -102,6 +102,9 @@
 #define sc_semopm	(sem_ctls[2])
 #define sc_semmni	(sem_ctls[3])
 
+unsigned int semvmx=32767;
+int semaem=16384;
+
 static int used_sems;
 
 void __init sem_init (void)

@@ -280,7 +283,7 @@
 	 

	/*

* Exceeding the undo range is an error.

	 */
- 
		if (undo < (-SEMAEM - 1) || undo > SEMAEM)
+ 
		if (undo < (-semaem - 1) || undo > semaem)
 	 
	{
 	 
		/* Don't undo the undo */
 	 
		sop->sem_flg &= ~SEM_UNDO;

@@ -290,7 +293,7 @@
}
if (curr->semval < 0)

	goto would_block;
- 
	if (curr->semval > SEMVMX)
+ 
	if (curr->semval > semvmx)
 	 
	goto out_of_range;

}

@@ -482,7 +485,7 @@
seminfo.semmns = sc_semmns;
seminfo.semmsl = sc_semmsl;
seminfo.semopm = sc_semopm;
-

	seminfo.semvmx = SEMVMX;
+ 
	seminfo.semvmx = semvmx;

seminfo.semmnu = SEMMNU;
seminfo.semmap = SEMMAP;
seminfo.semume = SEMUME;
@@ -492,7 +495,7 @@

	seminfo.semaem = used_sems;

} else {

	seminfo.semusz = SEMUSZ;
- 
		seminfo.semaem = SEMAEM;
+ 
		seminfo.semaem = semaem;

}
max_id = sem_ids.max_id;
up(&sem_ids.sem);
@@ -613,7 +616,7 @@
}

for (i = 0; i < nsems; i++) {
-

		if (sem_io[i] > SEMVMX) {
+ 
		if (sem_io[i] > semvmx) {
 	 
		err = -ERANGE;
 	 
		goto out_free;
 	 
	}

@@ -672,7 +675,7 @@
int val = arg.val;
struct sem_undo *un;
err = -ERANGE;
-

	if (val > SEMVMX || val < 0)
+ 
	if (val > semvmx || val < 0)
 	 
	goto out_unlock;

for (un = sma->undo; un; un = un->id_next)

diff -Naur linux-2.5.70/kernel/sysctl.c linux-2.5.70.n/kernel/sysctl.c
@@ -79,6 +79,8 @@
 extern int msg_ctlmnb;
 extern int msg_ctlmni;
 extern int sem_ctls[];
+extern unsigned int  semvmx;
+extern int semaem;
 #endif
 
 #ifdef __sparc__

@@ -237,6 +239,10 @@
 	 0644, NULL, &proc_dointvec},
 	{KERN_SEM, "sem", &sem_ctls, 4*sizeof (int),
 	 0644, NULL, &proc_dointvec},
+ {KERN_SEMVMX, "semvmx", &semvmx, sizeof (int),
+ 0444, NULL, &proc_dointvec},
+ {KERN_SEMAEM, "semaem", &semaem, sizeof (int),
+ 0444, NULL, &proc_dointvec},
 #endif
 #ifdef CONFIG_MAGIC_SYSRQ
 	{KERN_SYSRQ, "sysrq", &sysrq_enabled, sizeof (int),

Your MUA ate the whitespace in the patch.

Also, I'm not convinced it's useful to pollute init/main.c here. Something under /proc/sys/kernel/ with the other shm bits should do.

I might look deeper into whether the audit for signedness and other size issues is complete once those are dealt with.

-- wli

Please user --ignore-whitespace for patching. I use Mozilla for email activities and can not change to pine etc. Sorry for the inconvenience caused.

As we have discussed earlier, making it dynamic can cause some race conditions in the system (Please refer thread "Changing SEMVMX to a tunable parameter" on 28 May 2003).

I am not very clear about this. Is there a standard procedure for the audit? I reviewed the code and did not find any issues related to signedness / boundary conditions. Can you please explain more about it ??

Thanks and regards,
Arvind ..
P.S. Please find the patch with the implementation as dynamic tunable parameter, but I've some reservations about it.Making it dynamic will be ideal solution but it may lead to unavoidable races.

diff -Naur linux-2.5.70/include/linux/sysctl.h linux-2.5.70.n/include/linux/sysctl.h
@@ -130,6 +130,8 @@
 	KERN_PIDMAX=55, 

	/* int: PID # limit */

KERN_CORE_PATTERN=56, /* string: pattern for core-file names */
KERN_PANIC_ON_OOPS=57, /* int: whether we will panic on an oops */
+ KERN_SEMVMX=58, /* int: maximum limit on semval */
+ KERN_SEMAEM=59,

	/* int: maximun limit on semaem */

};

diff -Naur linux-2.5.70/ipc/sem.c linux-2.5.70.n/ipc/sem.c
@@ -102,6 +102,9 @@
 #define sc_semopm	(sem_ctls[2])
 #define sc_semmni	(sem_ctls[3])
 
+unsigned int semvmx=32767;
+int semaem=16384;
+
 static int used_sems;
 
 void __init sem_init (void)

@@ -280,7 +283,7 @@
 	 

	/*

* Exceeding the undo range is an error.

	 */
- 
		if (undo < (-SEMAEM - 1) || undo > SEMAEM)
+ 
		if (undo < (-semaem - 1) || undo > semaem)
 	 
	{
 	 
		/* Don't undo the undo */
 	 
		sop->sem_flg &= ~SEM_UNDO;

@@ -290,7 +293,7 @@
}
if (curr->semval < 0)

	goto would_block;
- 
	if (curr->semval > SEMVMX)
+ 
	if (curr->semval > semvmx)
 	 
	goto out_of_range;

}

@@ -482,7 +485,7 @@
seminfo.semmns = sc_semmns;
seminfo.semmsl = sc_semmsl;
seminfo.semopm = sc_semopm;
-

	seminfo.semvmx = SEMVMX;
+ 
	seminfo.semvmx = semvmx;

seminfo.semmnu = SEMMNU;
seminfo.semmap = SEMMAP;
seminfo.semume = SEMUME;
@@ -492,7 +495,7 @@

	seminfo.semaem = used_sems;

} else {

	seminfo.semusz = SEMUSZ;
- 
		seminfo.semaem = SEMAEM;
+ 
		seminfo.semaem = semaem;

}
max_id = sem_ids.max_id;
up(&sem_ids.sem);
@@ -613,7 +616,7 @@
}

for (i = 0; i < nsems; i++) {
-

		if (sem_io[i] > SEMVMX) {
+ 
		if (sem_io[i] > semvmx) {
 	 
		err = -ERANGE;
 	 
		goto out_free;
 	 
	}

@@ -672,7 +675,7 @@
int val = arg.val;
struct sem_undo *un;
err = -ERANGE;
-

	if (val > SEMVMX || val < 0)
+ 
	if (val > semvmx || val < 0)
 	 
	goto out_unlock;

for (un = sma->undo; un; un = un->id_next)

diff -Naur linux-2.5.70/kernel/sysctl.c linux-2.5.70.n/kernel/sysctl.c
@@ -79,6 +79,8 @@
 extern int msg_ctlmnb;
 extern int msg_ctlmni;
 extern int sem_ctls[];
+extern unsigned int  semvmx;
+extern int semaem;
 #endif
 
 #ifdef __sparc__

@@ -146,6 +148,14 @@
 static void unregister_proc_table(ctl_table *, struct proc_dir_entry *);
 #endif
 
+/* Constants for minimum and maximum testing in vm_table.
+ *    We use these as one-element integer vectors. */
+static int zero = 0;
+static int one = 1;
+static int one_hundred = 100;
+static int signed_short_max=32767;
+
+
 /* The default sysctl tables: */
 
 static ctl_table root_table[] = {

@@ -237,6 +247,10 @@
 	 0644, NULL, &proc_dointvec},
 	{KERN_SEM, "sem", &sem_ctls, 4*sizeof (int),
 	 0644, NULL, &proc_dointvec},
+ {KERN_SEMVMX, "semvmx", &semvmx, sizeof (int),
+ 0644, NULL, &proc_dointvec_minmax,NULL,NULL,&zero,NULL},
+ {KERN_SEMAEM, "semaem", &semaem, sizeof (int),
+ 0644, NULL, &proc_dointvec,NULL,NULL,&zero,&signed_short_max},
 #endif
 #ifdef CONFIG_MAGIC_SYSRQ
 	{KERN_SYSRQ, "sysrq", &sysrq_enabled, sizeof (int),

@@ -268,12 +282,6 @@
 	{0}
 };
 
-/* Constants for minimum and maximum testing in vm_table.
-   We use these as one-element integer vectors. */
-static int zero = 0;
-static int one = 1;
-static int one_hundred = 100;
-
 
 static ctl_table vm_table[] = {
 	{VM_OVERCOMMIT_MEMORY, "overcommit_memory", &sysctl_overcommit_memory,

I'm sorry about the confusion. I forgot the earlier thread. Please disregard my earlier comments wrt. dynamism, as they appear to be in conflict with other notions.

I think the __setup()'s should be moved to ipc/sem.c; otherwise I think manfred's the right person to do the review here and I'll back off.

-- wli

Have you thought about signed/unsigned issues? Which unix permits values > 32767? Are there any potential user space problems?

I did a quick google search, and found this:
http://www.core-software.ch/samples/www/SunFire_3800/SunFire_3800.pdf, i.e. Solaris 2.x:
<<
SEMVMX 32767 Limits the maximum value of a semaphore. Due to the interaction with undo structures and semaem (see below), this tunable should not be increased beyond its default value of 32767, unless you can guarantee that SEM_UNDO is never and will never be used. It can be safely reduced, but doing so provides no savings. SEMAEM 16384 Limits the maximum value of an adjust-on-exit undo element. No system resources are allocated based on this value.
<<

And as I wrote, I'd prefer a patch that just does s/32767/65535/ - either it is safe, or it's unsafe. If it's unsafe, then it should remain at 32767. If it safe, then we can increase it unconditionally, because a reduction below the upper limit provides no savings.

--
Manfred