[PATCH 1/3] Revert "ipc/mqueue: update maximums for the mqueue, subsystem"

[PATCH 1/3] Revert "ipc/mqueue: update maximums for the mqueue, subsystem"

[KOSAKI Motohiro]

Current linux-next doesn't work following test.

  # MSGSZ=10000
  # echo $MSGSZ > /proc/sys/fs/mqueue/msgsize_max
  # ./mq_open -c  /testN
  # ./mq_send -b $MSGSZ /testN
     => EMSGSIZE

Because mq_open(path, flags, mode, NULL) don't respect msgsize_max knob anymore.

Commit e30a8a7b58 (ipc/mqueue: update maximums for the mqueue subsystem) claim
Commit b231cca4381ee ("message queues: increase range limits") changed the
maximum size of a message in a message queue from INT_MAX to 8192*128. and The patch
fix up the maximum value. However it also changes default mqueue value and
changed mqueue.msg_max and mqueue.msgsize_max sysctl semantics.

In general, basic compatibility rule is:
1. Increase hard coded maximum limitation
   -> safe
2. Decrease hard coded maximum limitation
   -> unsafe
3. Increase sysctl default value
   -> safe if the new value don't exceed another limitation *and*
      every application don't depend on old value.
4. Decrease sysctl default value
   -> unsafe
5. Change hard coded value to flexible knob
   -> safe if default value is unchanged.
6. Change knob to hard coded value
   -> unsafe

"ipc/mqueue: switch back to using non-max values on create" breaks rule 6 and
"ipc/mqueue: update maximums for the mqueue subsystem" breaks rule 3.

Thus, this patch reverts following three commits.

- ipc/mqueue: switch back to using non-max values on create
- ipc/mqueue: update maximums for the mqueue subsystem
- ipc-mqueue-update-maximums-for-the-mqueue-subsystem-checkpatch-fixes

The right way is to only bump up hard coded maximum value and don't touch
default value. Look! Commit b231cca4381ee ("message queues: increase range limits")
only changed hard coded value. If anyone think it is regression, they have to
touch only the same place. Unnecessary change brings another regression. The
subsequent patches do it.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Amerigo Wang <amwang@redhat.com>
Cc: Serge E. Hallyn <serue@us.ibm.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Joe Korty <joe.korty@ccur.com>
---
 include/linux/ipc_namespace.h |   45 +++++++++-------------------------------
 ipc/mqueue.c                  |   15 +++----------
 2 files changed, 14 insertions(+), 46 deletions(-)

diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index e2bac00..1372b56 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -90,41 +90,16 @@ static inline void shm_destroy_orphaned(struct ipc_namespace *ns) {}

 #ifdef CONFIG_POSIX_MQUEUE
 extern int mq_init_ns(struct ipc_namespace *ns);
-/*
- * POSIX Message Queue default values:
- *
- * MIN_*: Lowest value an admin can set the maximum unprivileged limit to
- * DFLT_*MAX: Default values for the maximum unprivileged limits
- * DFLT_{MSG,MSGSIZE}: Default values used when the user doesn't supply
- *   an attribute to the open call and the queue must be created
- * HARD_*: Highest value the maximums can be set to.  These are enforced
- *   on CAP_SYS_RESOURCE apps as well making them inviolate (so make them
- *   suitably high)
- *
- * POSIX Requirements:
- *   Per app minimum openable message queues - 8.  This does not map well
- *     to the fact that we limit the number of queues on a per namespace
- *     basis instead of a per app basis.  So, make the default high enough
- *     that no given app should have a hard time opening 8 queues.
- *   Minimum maximum for HARD_MSGMAX - 32767.  I bumped this to 65536.
- *   Minimum maximum for HARD_MSGSIZEMAX - POSIX is silent on this.  However,
- *     we have run into a situation where running applications in the wild
- *     require this to be at least 5MB, and preferably 10MB, so I set the
- *     value to 16MB in hopes that this user is the worst of the bunch and
- *     the new maximum will handle anyone else.  I may have to revisit this
- *     in the future.
- */
-#define MIN_QUEUESMAX			1
-#define DFLT_QUEUESMAX		      256
-#define HARD_QUEUESMAX		     1024
-#define MIN_MSGMAX			1
-#define DFLT_MSG		       64U
-#define DFLT_MSGMAX		     1024
-#define HARD_MSGMAX		    65536
-#define MIN_MSGSIZEMAX		      128
-#define DFLT_MSGSIZE		     8192U
-#define DFLT_MSGSIZEMAX		(1024*1024)
-#define HARD_MSGSIZEMAX	     (16*1024*1024)
+/* default values */
+#define MIN_QUEUESMAX  1
+#define DFLT_QUEUESMAX 256     /* max number of message queues */
+#define HARD_QUEUESMAX 1024
+#define MIN_MSGMAX     1
+#define DFLT_MSGMAX    10      /* max number of messages in each queue */
+#define HARD_MSGMAX    (32768*sizeof(void *)/4)
+#define MIN_MSGSIZEMAX  128
+#define DFLT_MSGSIZEMAX 8192   /* max message size */
+#define HARD_MSGSIZEMAX (8192*128)
 #else
 static inline int mq_init_ns(struct ipc_namespace *ns) { return 0; }
 #endif
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 229a5fb..1eb0198 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -143,18 +143,14 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
 		info->qsize = 0;
 		info->user = NULL;	/* set when all is ok */
 		memset(&info->attr, 0, sizeof(info->attr));
-		info->attr.mq_maxmsg = min(ipc_ns->mq_msg_max, DFLT_MSG);
-		info->attr.mq_msgsize =
-			min(ipc_ns->mq_msgsize_max, DFLT_MSGSIZE);
+		info->attr.mq_maxmsg = ipc_ns->mq_msg_max;
+		info->attr.mq_msgsize = ipc_ns->mq_msgsize_max;
 		if (attr) {
 			info->attr.mq_maxmsg = attr->mq_maxmsg;
 			info->attr.mq_msgsize = attr->mq_msgsize;
 		}
 		mq_msg_tblsz = info->attr.mq_maxmsg * sizeof(struct msg_msg *);
-		if (mq_msg_tblsz > KMALLOC_MAX_SIZE)
-			info->messages = vmalloc(mq_msg_tblsz);
-		else
-			info->messages = kmalloc(mq_msg_tblsz, GFP_KERNEL);
+		info->messages = kmalloc(mq_msg_tblsz, GFP_KERNEL);
 		if (!info->messages)
 			goto out_inode;

@@ -275,10 +271,7 @@ static void mqueue_evict_inode(struct inode *inode)
 	spin_lock(&info->lock);
 	for (i = 0; i < info->attr.mq_curmsgs; i++)
 		free_msg(info->messages[i]);
-	if (info->attr.mq_maxmsg * sizeof(struct msg_msg *) > KMALLOC_MAX_SIZE)
-		vfree(info->messages);
-	else
-		kfree(info->messages);
+	kfree(info->messages);
 	spin_unlock(&info->lock);

 	/* Total amount of bytes accounted for the mqueue */
-- 
1.7.5.2

[PATCH 2/3] ipc/mqueue: bump up mqueue hard coded limit

[KOSAKI Motohiro]

Quote from Doug's original patch:
    Commit b231cca4381ee ("message queues: increase range limits") changed the
    maximum size of a message in a message queue from INT_MAX to 8192*128.
    Unfortunately, we had customers that relied on a size much larger than
    8192*128 on their production systems.  After reviewing POSIX, we found
    that it is silent on the maximum message size.  We did find a couple other
    areas in which it was not silent.  Fix up the mqueue maximums so that the
    customer's system can continue to work, and document both the POSIX and
    real world requirements in ipc_namespace.h so that we don't have this
    issue crop back up.

    Also, commit 9cf18e1dd74c ("ipc: HARD_MSGMAX should be higher not lower on
    64bit") fiddled with HARD_MSGMAX without realizing that the number was
    intentionally in place to limit the msg queue depth to one that was small
    enough to kmalloc an array of pointers (hence why we divided 128k by
    sizeof(long)).  If we wish to meet POSIX requirements, we have no choice
    but to change our allocation to a vmalloc instead (at least for the large
    queue size case).  With that, it's possible to increase our allowed
    maximum to the POSIX requirements (or more if we choose).

changes:
 o Don't change default value. It's a part of ABI.
 o Use PAGE_SIZE for vmalloc threshold. KMALLOC_MAX_SIZE is
   clearly too large and bring a fragmentation issue.
 o use is_vmalloc_addr()
 o bump up HARD_QUEUESMAX too.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Amerigo Wang <amwang@redhat.com>
Cc: Serge E. Hallyn <serue@us.ibm.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Joe Korty <joe.korty@ccur.com>
---
 include/linux/ipc_namespace.h |   41 +++++++++++++++++++++++++++++++----------
 ipc/mqueue.c                  |   11 +++++++++--
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index 1372b56..6bb4c3b 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -90,16 +90,37 @@ static inline void shm_destroy_orphaned(struct ipc_namespace *ns) {}

 #ifdef CONFIG_POSIX_MQUEUE
 extern int mq_init_ns(struct ipc_namespace *ns);
-/* default values */
-#define MIN_QUEUESMAX  1
-#define DFLT_QUEUESMAX 256     /* max number of message queues */
-#define HARD_QUEUESMAX 1024
-#define MIN_MSGMAX     1
-#define DFLT_MSGMAX    10      /* max number of messages in each queue */
-#define HARD_MSGMAX    (32768*sizeof(void *)/4)
-#define MIN_MSGSIZEMAX  128
-#define DFLT_MSGSIZEMAX 8192   /* max message size */
-#define HARD_MSGSIZEMAX (8192*128)
+/*
+ * POSIX Message Queue default values:
+ *
+ * MIN_*: Lowest value an admin can set the maximum unprivileged limit to
+ * DFLT_*MAX: Default values for the maximum unprivileged limits
+ * HARD_*: Highest value the maximums can be set to.  These are enforced
+ *   on CAP_SYS_RESOURCE apps as well making them inviolate (so make them
+ *   suitably high)
+ *
+ * POSIX Requirements:
+ *   Per app minimum openable message queues - 8.  This does not map well
+ *     to the fact that we limit the number of queues on a per namespace
+ *     basis instead of a per app basis.  So, make the default high enough
+ *     that no given app should have a hard time opening 8 queues.
+ *   Minimum maximum for HARD_MSGMAX - 32767.  I bumped this to 65536.
+ *   Minimum maximum for HARD_MSGSIZEMAX - POSIX is silent on this.  However,
+ *     we have run into a situation where running applications in the wild
+ *     require this to be at least 5MB, and preferably 10MB, so I set the
+ *     value to 16MB in hopes that this user is the worst of the bunch and
+ *     the new maximum will handle anyone else.  I may have to revisit this
+ *     in the future.
+ */
+#define MIN_QUEUESMAX		    1
+#define DFLT_QUEUESMAX		  256
+#define HARD_QUEUESMAX	  (1024*1024)
+#define MIN_MSGMAX		    1
+#define DFLT_MSGMAX		   10
+#define HARD_MSGMAX		65536
+#define MIN_MSGSIZEMAX		  128
+#define DFLT_MSGSIZEMAX		 8192
+#define HARD_MSGSIZEMAX	 (16*1024*1024)
 #else
 static inline int mq_init_ns(struct ipc_namespace *ns) { return 0; }
 #endif
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 1eb0198..7e01da7 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -150,7 +150,11 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
 			info->attr.mq_msgsize = attr->mq_msgsize;
 		}
 		mq_msg_tblsz = info->attr.mq_maxmsg * sizeof(struct msg_msg *);
-		info->messages = kmalloc(mq_msg_tblsz, GFP_KERNEL);
+
+		if (mq_msg_tblsz > PAGE_SIZE)
+			info->messages = vmalloc(mq_msg_tblsz);
+		else
+			info->messages = kmalloc(mq_msg_tblsz, GFP_KERNEL);
 		if (!info->messages)
 			goto out_inode;

@@ -271,7 +275,10 @@ static void mqueue_evict_inode(struct inode *inode)
 	spin_lock(&info->lock);
 	for (i = 0; i < info->attr.mq_curmsgs; i++)
 		free_msg(info->messages[i]);
-	kfree(info->messages);
+	if (is_vmalloc_addr(info->messages))
+		vfree(info->messages);
+	else
+		kfree(info->messages);
 	spin_unlock(&info->lock);

 	/* Total amount of bytes accounted for the mqueue */
-- 
1.7.5.2

[PATCH 3/3] ipc/mqueue: separate mqueue default value from maximum, value

[KOSAKI Motohiro]

commit b231cca438 (message queues: increase range limits)
changed mqueue default value when attr parameter is specified NULL
from hard coded value to fs.mqueue.{msg,msgsize}_max sysctl value.

This made large side effect. When user need to use two mqueue
applications 1) using !NULL attr parameter and it require big
message size and 2) using NULL attr parameter and only need small
size message, app (1) require to raise fs.mqueue.msgsize_max and
app (2) consume large memory size even though it doesn't need.

The solution is to separate default value from maximum value.

Note: msg_default and msgsize_default have slightly strange
initial value. It is necessary to keep backward compatibility.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Amerigo Wang <amwang@redhat.com>
Cc: Serge E. Hallyn <serue@us.ibm.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Joe Korty <joe.korty@ccur.com>
---
 Documentation/sysctl/fs.txt   |    7 +++++++
 include/linux/ipc_namespace.h |    3 +++
 ipc/mq_sysctl.c               |   18 ++++++++++++++++++
 ipc/mqueue.c                  |    8 ++++++--
 ipc/msgutil.c                 |    4 ++++
 5 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/Documentation/sysctl/fs.txt b/Documentation/sysctl/fs.txt
index 88fd7f5..5f644b3 100644
--- a/Documentation/sysctl/fs.txt
+++ b/Documentation/sysctl/fs.txt
@@ -225,6 +225,13 @@ a queue must be less or equal then msg_max.
 maximum  message size value (it is every  message queue's attribute set during
 its creation).

+/proc/sys/fs/mqueue/msg_default is  a read/write  file for setting/getting the
+default number of messages in a queue value if attr parameter of mq_open(2) is
+NULL. If it exceed msg_max, the default value is initialized msg_max instead.
+
+/proc/sys/fs/mqueue/msgsize_default is a read/write file for setting/getting
+the default message size value if attr parameter of mq_open(2) is NULL. If it
+exceed msgsize_max, the default value is initialized msgsize_max instead.

 4. /proc/sys/fs/epoll - Configuration options for the epoll interface
 --------------------------------------------------------
diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index 6bb4c3b..4483234 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -63,6 +63,9 @@ struct ipc_namespace {
 	unsigned int    mq_msg_max;      /* initialized to DFLT_MSGMAX */
 	unsigned int    mq_msgsize_max;  /* initialized to DFLT_MSGSIZEMAX */

+	unsigned int    mq_msg_default;
+	unsigned int    mq_msgsize_default;
+
 	/* user_ns which owns the ipc ns */
 	struct user_namespace *user_ns;
 };
diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
index e22336a..383d638 100644
--- a/ipc/mq_sysctl.c
+++ b/ipc/mq_sysctl.c
@@ -73,6 +73,24 @@ static ctl_table mq_sysctls[] = {
 		.extra1		= &msg_maxsize_limit_min,
 		.extra2		= &msg_maxsize_limit_max,
 	},
+	{
+		.procname	= "msg_default",
+		.data		= &init_ipc_ns.mq_msg_default,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_mq_dointvec_minmax,
+		.extra1		= &msg_max_limit_min,
+		.extra2		= &msg_max_limit_max,
+	},
+	{
+		.procname	= "msgsize_default",
+		.data		= &init_ipc_ns.mq_msgsize_default,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_mq_dointvec_minmax,
+		.extra1		= &msg_maxsize_limit_min,
+		.extra2		= &msg_maxsize_limit_max,
+	},
 	{}
 };

diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 7e01da7..8b2de4f 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -143,8 +143,10 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
 		info->qsize = 0;
 		info->user = NULL;	/* set when all is ok */
 		memset(&info->attr, 0, sizeof(info->attr));
-		info->attr.mq_maxmsg = ipc_ns->mq_msg_max;
-		info->attr.mq_msgsize = ipc_ns->mq_msgsize_max;
+		info->attr.mq_maxmsg = min(ipc_ns->mq_msg_default,
+					   ipc_ns->mq_msg_max);
+		info->attr.mq_msgsize = min(ipc_ns->mq_msgsize_default,
+					    ipc_ns->mq_msgsize_max);
 		if (attr) {
 			info->attr.mq_maxmsg = attr->mq_maxmsg;
 			info->attr.mq_msgsize = attr->mq_msgsize;
@@ -1262,6 +1264,8 @@ int mq_init_ns(struct ipc_namespace *ns)
 	ns->mq_queues_max    = DFLT_QUEUESMAX;
 	ns->mq_msg_max       = DFLT_MSGMAX;
 	ns->mq_msgsize_max   = DFLT_MSGSIZEMAX;
+	ns->mq_msg_default   = HARD_MSGMAX;
+	ns->mq_msgsize_default  = HARD_MSGSIZEMAX;

 	ns->mq_mnt = kern_mount_data(&mqueue_fs_type, ns);
 	if (IS_ERR(ns->mq_mnt)) {
diff --git a/ipc/msgutil.c b/ipc/msgutil.c
index 8b5ce5d3..e2d6b0e 100644
--- a/ipc/msgutil.c
+++ b/ipc/msgutil.c
@@ -31,6 +31,10 @@ struct ipc_namespace init_ipc_ns = {
 	.mq_queues_max   = DFLT_QUEUESMAX,
 	.mq_msg_max      = DFLT_MSGMAX,
 	.mq_msgsize_max  = DFLT_MSGSIZEMAX,
+
+	/* strange default for backward compatibility. */
+	.mq_msg_default  = HARD_MSGMAX,
+	.mq_msgsize_default  = HARD_MSGSIZEMAX,
 #endif
 	.user_ns = &init_user_ns,
 };
-- 
1.7.5.2

Re: [PATCH 1/3] Revert "ipc/mqueue: update maximums for the mqueue, subsystem"

[KOSAKI Motohiro]

Self NAK this seris. I'll post new one.