2003-06-06 05:41:16

by Arvind Kandhare

[permalink] [raw]
Subject: [RFC][PATCH 2.5.70] 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
--- linux-2.5.70/include/linux/sysctl.h Tue May 27 06:30:40 2003
+++ linux-2.5.70.n/include/linux/sysctl.h Wed Jun 4 16:21:19 2003
@@ -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
--- linux-2.5.70/init/main.c Tue May 27 06:30:25 2003
+++ linux-2.5.70.n/init/main.c Wed Jun 4 16:19:46 2003
@@ -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
--- linux-2.5.70/ipc/sem.c Tue May 27 06:30:38 2003
+++ linux-2.5.70.n/ipc/sem.c Wed Jun 4 17:01:46 2003
@@ -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
--- linux-2.5.70/kernel/sysctl.c Tue May 27 06:30:23 2003
+++ linux-2.5.70.n/kernel/sysctl.c Wed Jun 4 17:02:52 2003
@@ -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),



2003-06-06 06:37:28

by William Lee Irwin III

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.5.70] Static tunable semvmx and semaem

On Fri, Jun 06, 2003 at 11:23:23AM +0530, Arvind Kandhare wrote:
> 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.

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

2003-06-06 08:49:02

by Arvind Kandhare

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.5.70] Static tunable semvmx and semaem

William Lee Irwin III wrote:

>
> Your MUA ate the whitespace in the patch.


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

>
> 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.
>

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 might look deeper into whether the audit for signedness and other
> size issues is complete once those are dealt with.
>

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
--- linux-2.5.70/include/linux/sysctl.h Tue May 27 06:30:40 2003
+++ linux-2.5.70.n/include/linux/sysctl.h Fri Jun 6 13:42:54 2003
@@ -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
--- linux-2.5.70/ipc/sem.c Tue May 27 06:30:38 2003
+++ linux-2.5.70.n/ipc/sem.c Fri Jun 6 13:42:54 2003
@@ -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
--- linux-2.5.70/kernel/sysctl.c Tue May 27 06:30:23 2003
+++ linux-2.5.70.n/kernel/sysctl.c Fri Jun 6 13:47:46 2003
@@ -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,


2003-06-06 13:02:36

by William Lee Irwin III

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.5.70] Static tunable semvmx and semaem

On Fri, Jun 06, 2003 at 02:31:35PM +0530, Arvind Kandhare wrote:
> 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'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

2003-06-06 14:50:36

by Arvind Kandhare

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.5.70] Static tunable semvmx and semaem

William Lee Irwin III wrote:


> I think the __setup()'s should be moved to ipc/sem.c;


It is always better if all the initializations related to
IPC V semaphores are in ipc/sem.c file. Thanks for the suggestion.
Below is the updated patch with __setup()calls in ipc/sem.c.

Thanks,
Arvind
P.S. I've changed my mail client to evaluation and cross checked whether
the patches works. Please get back to me in case there are any issues
while patching.

diff -Naur linux-2.5.70/include/linux/sysctl.h linux-2.5.70.n/include/linux/sysctl.h
--- linux-2.5.70/include/linux/sysctl.h Tue May 27 06:30:40 2003
+++ linux-2.5.70.n/include/linux/sysctl.h Fri Jun 6 19:33:19 2003
@@ -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
--- linux-2.5.70/ipc/sem.c Tue May 27 06:30:38 2003
+++ linux-2.5.70.n/ipc/sem.c Fri Jun 6 19:35:19 2003
@@ -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)
@@ -1295,6 +1298,29 @@
unlock_kernel();
}

+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);
+
+
#ifdef CONFIG_PROC_FS
static int sysvipc_sem_read_proc(char *buffer, char **start, off_t offset, int length, int *eof, void *data)
{
diff -Naur linux-2.5.70/kernel/sysctl.c linux-2.5.70.n/kernel/sysctl.c
--- linux-2.5.70/kernel/sysctl.c Tue May 27 06:30:23 2003
+++ linux-2.5.70.n/kernel/sysctl.c Fri Jun 6 19:49:30 2003
@@ -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),


2003-06-06 16:39:17

by Manfred Spraul

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.5.70] Static tunable semvmx and semaem

Arvind Kandhare wrote:

>
> Hi,
> Please find below patch(RFC) for implementing semvmx and semaem
> as static tunable parameters.
>
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