On Tue, 11 Dec 2007 16:38:46 +0100
[EMAIL PROTECTED] wrote:
> [PATCH 01/02]
>
> This patch computes msg_ctlmni to make it scale with system memory.
> msg_ctlmni is now set to make the message queues occupy 1/32 of the available
> memory.
>
> Some cleaning has also been done in the MSGXXX constants:
> . MSGPOOL: the msgctl man page says it's not used, but it also defines it as
> a size in bytes (the code expresses it in Kbytes).
> . MSGSEG definition has been removed since it used only once in msgctl().
>
The objective seems reasonable.
>
> ===================================================================
> --- linux-2.6.24-rc4.orig/include/linux/msg.h 2007-12-11 11:57:53.000000000
> +0100
> +++ linux-2.6.24-rc4/include/linux/msg.h 2007-12-11 12:10:01.000000000
> +0100
> @@ -49,17 +49,17 @@ struct msginfo {
> unsigned short msgseg;
> };
>
> +#define MSG_MEM_SCALE 32 /* Scaling factor to compute msgmni */
> +
> #define MSGMNI 16 /* <= IPCMNI */ /* max # of msg queue identifiers
> */
> #define MSGMAX 8192 /* <= INT_MAX */ /* max size of message (bytes) */
> #define MSGMNB 16384 /* <= INT_MAX */ /* default max size of a message
> queue */
>
> /* unused */
> -#define MSGPOOL (MSGMNI*MSGMNB/1024) /* size in kilobytes of message pool */
> +#define MSGPOOL (MSGMNI * MSGMNB) /* size in bytes of message pool */
> #define MSGTQL MSGMNB /* number of system message headers */
> #define MSGMAP MSGMNB /* number of entries in message map */
> #define MSGSSZ 16 /* message segment size */
> -#define __MSGSEG ((MSGPOOL*1024)/ MSGSSZ) /* max no. of segments */
> -#define MSGSEG (__MSGSEG <= 0xffff ? __MSGSEG : 0xffff)
>
> #ifdef __KERNEL__
> #include <linux/list.h>
> Index: linux-2.6.24-rc4/ipc/msg.c
> ===================================================================
> --- linux-2.6.24-rc4.orig/ipc/msg.c 2007-12-11 11:57:58.000000000 +0100
> +++ linux-2.6.24-rc4/ipc/msg.c 2007-12-11 12:12:32.000000000 +0100
> @@ -27,6 +27,7 @@
> #include <linux/msg.h>
> #include <linux/spinlock.h>
> #include <linux/init.h>
> +#include <linux/mm.h>
> #include <linux/proc_fs.h>
> #include <linux/list.h>
> #include <linux/security.h>
> @@ -81,10 +82,25 @@ static int sysvipc_msg_proc_show(struct
>
> static void __msg_init_ns(struct ipc_namespace *ns, struct ipc_ids *ids)
> {
> + struct sysinfo i;
> + unsigned long allowed;
> +
> ns->ids[IPC_MSG_IDS] = ids;
> ns->msg_ctlmax = MSGMAX;
> ns->msg_ctlmnb = MSGMNB;
> - ns->msg_ctlmni = MSGMNI;
> +
> + /*
> + * Scale msgmni with the available memory size: the memory dedicated
> + * to msg queues should occupy 1/32 of the available memory:
> + * up to 8MB : msgmni = 16 (MSGMNI)
> + * 4 GB : msgmni = 8K
> + * more than 16 GB : msgmni = 32K (IPCMNI)
> + */
> + si_meminfo(&i);
> + allowed = ((i.totalram / MSG_MEM_SCALE) * i.mem_unit) / MSGMNB;
> + ns->msg_ctlmni = min((unsigned long) IPCMNI,
> + max((unsigned long) MSGMNI, allowed));
The space after the (typecast) isn't useful IMO.
Please use min_t rather than the open-coded casts.
Even better would be to sort out the types so that neither casts nor min_t
are needed.
What about highmem machines? For those we usually want to scale data
structures according to the amount of direct-addressable memory (ie:
lowmem) rather than acording to total physical memory. I haven't a clue
how this consideration would be addressed when ipc-namespaces is taken into
consideration.
I'd suggest the addition of a printk telling people what value the kernel
calculated.
We should ensure that the calculated value is never _less_ than what the
kernel was previously giving - to avoid breaking existing things.
It's a bit of a concern that a change like this can cause an application to
work OK on machine A but then fail when it is taken over to (smaller)
machine B.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
