Re: [RFC][PATCH] 2/6 POSIX message queues

Re: [RFC][PATCH] 2/6 POSIX message queues

[Manfred Spraul]

Arnd wrote:

>Nothing difficult here, but slow and avoidable if you have the
>structures laid out properly.
>
I thought about using s32 for the kernel mq_attr structure, and didn't 
like it: It would mean that 64-bit archs running native 64-bit apps must 
do a conversion - glibc must expose a structure with long values.
Additionally, the posix message queue API uses 3 structures with long 
values, and only one of them is new - we'll need wrappers anyway.
The actual values in the mq_attr structure would fit into s32:
- mq_flags is 0 or O_NONBLOCK
- mq_maxmsg is less than 32k (kmalloc'ed array of pointers)
- mq_msgsize is theoretically unlimited, but the current implementation 
arbitrarily limits the value to 1 MB.

--
    Manfred

Re: [RFC][PATCH] 2/6 POSIX message queues

[Arnd Bergmann]

Krzysztof Benedyczak wrote:

> +
> +struct mq_attr {
> +	long	mq_flags;	/* message queue flags			*/
> +	long	mq_maxmsg;	/* maximum number of messages		*/
> +	long	mq_msgsize;	/* maximum message size			*/
> +	long	mq_curmsgs;	/* number of messages currently queued	*/
> +};
> +

Does POSIX mandate that these have to be 'long'? If you can change them
all to any of 'int', '__s32' or '__s64', the handlers for 32 bit system
call emulation on 64 bit machines will get a lot simpler because the
32 bit user structures are then identical to the 64 bit ones.

	Arnd <><

Re: [RFC][PATCH] 2/6 POSIX message queues

[Krzysztof Benedyczak]

On Fri, 20 Feb 2004, Arnd Bergmann wrote:

> Krzysztof Benedyczak wrote:
>
> > +
> > +struct mq_attr {
> > +	long	mq_flags;	/* message queue flags			*/
> > +	long	mq_maxmsg;	/* maximum number of messages		*/
> > +	long	mq_msgsize;	/* maximum message size			*/
> > +	long	mq_curmsgs;	/* number of messages currently queued	*/
> > +};
> > +
>
> Does POSIX mandate that these have to be 'long'? If you can change them
> all to any of 'int', '__s32' or '__s64', the handlers for 32 bit system
> call emulation on 64 bit machines will get a lot simpler because the
> 32 bit user structures are then identical to the 64 bit ones.
>
Yes, POSIX defines it to longs. And quess that compability issues should
be handled in kernel?

Krzysztof Benedyczak

Re: [RFC][PATCH] 2/6 POSIX message queues

[Arnd Bergmann]

On Friday 20 February 2004 19:11, Krzysztof Benedyczak wrote:
> On Fri, 20 Feb 2004, Arnd Bergmann wrote:
> > Krzysztof Benedyczak wrote:
> > > +
> > > +struct mq_attr {
> > > +	long	mq_flags;	/* message queue flags			*/
> > > +	long	mq_maxmsg;	/* maximum number of messages		*/
> > > +	long	mq_msgsize;	/* maximum message size			*/
> > > +	long	mq_curmsgs;	/* number of messages currently queued	*/
> > > +};
> > > +
> >
> > Does POSIX mandate that these have to be 'long'? If you can change them
> > all to any of 'int', '__s32' or '__s64', the handlers for 32 bit system
> > call emulation on 64 bit machines will get a lot simpler because the
> > 32 bit user structures are then identical to the 64 bit ones.
>
> Yes, POSIX defines it to longs. And quess that compability issues should
> be handled in kernel?

Yes, compatibility can only be handled in kernel. For each system call
that passes structures with a different layout, you need to do something
like this:

struct compat_mq_attr {
	compat_long_t	mq_flags;
	compat_long_t	mq_maxmsg;
	compat_long_t	mq_msgsize;
	compat_long_t	mq_curmsgs;
};
asmlinkage long compat_sys_mq_getsetattr(mqd_t mqdes,
		const struct compat_mq_attr __user *u_mqstat,
		struct compat_mq_attr __user *u_omqstat)
{
	struct mq_attr mqstat, omqstat;
	mm_segment_t oldfs;
	int err, err2;

	err = get_user(mqstat.mq_flags, &u_mqstat->mq_flags)
	    | get_user(mqstat.mq_maxmsg, &u_mqstat->mq_maxmsgs)
	    | get_user(mqstat.mq_msgsize, &u_mqstat->mq_msgsize)
	    | get_user(mqstat.mq_curmsgs, &u_mqstat->mq_curmsgs);
	if (err)
		return -EFAULT;
	oldfs = get_fs();
	set_fs(KERNEL_DS);
	err2 = sys_mq_getsetattr(mqdes, &mqstat, &omqstat);
	set_fs(old_fs);
	err = get_user(omqstat.mq_flags, &u_omqstat->mq_flags)
	    | get_user(omqstat.mq_maxmsg, &u_omqstat->mq_maxmsgs)
	    | get_user(omqstat.mq_msgsize, &u_omqstat->mq_msgsize)
	    | get_user(omqstat.mq_curmsgs, &u_omqstat->mq_curmsgs);
	if (err)
		return -EFAULT;
	return err2;
}

Nothing difficult here, but slow and avoidable if you have the
structures laid out properly. Normally you can do this with padding,
but that is no good here because 'long' members need sign-extension,
not zero-extension.

	Arnd <><

[RFC][PATCH] 2/6 POSIX message queues

[Krzysztof Benedyczak]

// $Header$
// Kernel Version:
//  VERSION = 2
//  PATCHLEVEL = 6
//  SUBLEVEL = 2
//  EXTRAVERSION =
--- 2.6/ipc/util.c	2004-02-14 18:47:10.000000000 +0100
+++ build-2.6/ipc/util.c	2004-02-14 18:44:34.000000000 +0100
@@ -24,6 +24,7 @@
 #include <linux/security.h>
 #include <linux/rcupdate.h>
 #include <linux/workqueue.h>
+#include <linux/mqueue.h>

 #if defined(CONFIG_SYSVIPC)

--- 2.6/arch/i386/kernel/entry.S	2004-02-14 18:47:10.000000000 +0100
+++ build-2.6/arch/i386/kernel/entry.S	2004-02-14 18:43:43.000000000 +0100
@@ -882,5 +882,11 @@
 	.long sys_utimes
  	.long sys_fadvise64_64
 	.long sys_ni_syscall	/* sys_vserver */
+	.long sys_mq_open
+	.long sys_mq_unlink	/* 275 */
+	.long sys_mq_timedsend
+	.long sys_mq_timedreceive
+	.long sys_mq_notify
+	.long sys_mq_getsetattr

 syscall_table_size=(.-sys_call_table)
--- 2.6/include/asm-i386/unistd.h	2004-02-14 18:47:10.000000000 +0100
+++ build-2.6/include/asm-i386/unistd.h	2004-02-14 18:43:43.000000000 +0100
@@ -279,8 +279,14 @@
 #define __NR_utimes		271
 #define __NR_fadvise64_64	272
 #define __NR_vserver		273
+#define __NR_mq_open 		274
+#define __NR_mq_unlink		(__NR_mq_open+1)
+#define __NR_mq_timedsend	(__NR_mq_open+2)
+#define __NR_mq_timedreceive	(__NR_mq_open+3)
+#define __NR_mq_notify		(__NR_mq_open+4)
+#define __NR_mq_getsetattr	(__NR_mq_open+5)

-#define NR_syscalls 274
+#define NR_syscalls 280

 /* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */

--- 2.6/include/linux/mqueue.h	1970-01-01 01:00:00.000000000 +0100
+++ build-2.6/include/linux/mqueue.h	2004-02-14 18:50:50.000000000 +0100
@@ -0,0 +1,50 @@
+/* Copyright (C) 2003 Krzysztof Benedyczak & Michal Wronski
+
+   This program is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   It is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this software; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#ifndef _LINUX_MQUEUE_H
+#define _LINUX_MQUEUE_H
+
+#define MQ_PRIO_MAX 	32768
+
+typedef int mqd_t;
+
+struct mq_attr {
+	long	mq_flags;	/* message queue flags			*/
+	long	mq_maxmsg;	/* maximum number of messages		*/
+	long	mq_msgsize;	/* maximum message size			*/
+	long	mq_curmsgs;	/* number of messages currently queued	*/
+};
+
+#define NOTIFY_NONE	0
+#define NOTIFY_WOKENUP	1
+#define NOTIFY_REMOVED	2
+
+#ifdef __KERNEL__
+#include <linux/types.h>
+#include <linux/time.h>
+#include <linux/signal.h>
+#include <linux/linkage.h>
+
+asmlinkage long sys_mq_open(const char __user *name, int oflag, mode_t mode, struct mq_attr __user *attr);
+asmlinkage long sys_mq_unlink(const char __user *name);
+asmlinkage long mq_timedsend(mqd_t mqdes, const char __user *msg_ptr, size_t msg_len, unsigned int msg_prio, const struct timespec __user *abs_timeout);
+asmlinkage ssize_t mq_timedreceive(mqd_t mqdes, char __user *msg_ptr, size_t msg_len, unsigned int __user *msg_prio, const struct timespec __user *abs_timeout);
+asmlinkage long mq_notify(mqd_t mqdes, const struct sigevent __user *notification);
+asmlinkage long mq_getsetattr(mqd_t mqdes, const struct mq_attr __user *mqstat, struct mq_attr __user *omqstat);
+#endif
+
+#endif
--- 2.6/kernel/sys.c	2004-02-14 17:19:39.000000000 +0100
+++ build-2.6/kernel/sys.c	2004-02-14 18:45:23.000000000 +0100
@@ -249,6 +249,12 @@
 cond_syscall(sys_epoll_create)
 cond_syscall(sys_epoll_ctl)
 cond_syscall(sys_epoll_wait)
+cond_syscall(sys_mq_open)
+cond_syscall(sys_mq_unlink)
+cond_syscall(sys_mq_timedsend)
+cond_syscall(sys_mq_timedreceive)
+cond_syscall(sys_mq_notify)
+cond_syscall(sys_mq_getsetattr)

 /* arch-specific weak syscall entries */
 cond_syscall(sys_pciconfig_read)

Re: [RFC][PATCH] 2/6 POSIX message queues

[Christoph Hellwig]

> +#ifndef _LINUX_MQUEUE_H
> +#define _LINUX_MQUEUE_H
> +
> +#define MQ_PRIO_MAX 	32768
> +
> +typedef int mqd_t;
> +
> +struct mq_attr {
> +	long	mq_flags;	/* message queue flags			*/
> +	long	mq_maxmsg;	/* maximum number of messages		*/
> +	long	mq_msgsize;	/* maximum message size			*/
> +	long	mq_curmsgs;	/* number of messages currently queued	*/
> +};
> +
> +#define NOTIFY_NONE	0
> +#define NOTIFY_WOKENUP	1
> +#define NOTIFY_REMOVED	2
> +
> +#ifdef __KERNEL__
> +#include <linux/types.h>
> +#include <linux/time.h>

This looks like you want glibc to include it, please don't - add a copy
without the kernel part to glibc instead.

> +asmlinkage long sys_mq_open(const char __user *name, int oflag, mode_t mode, struct mq_attr __user *attr);
> +asmlinkage long sys_mq_unlink(const char __user *name);
> +asmlinkage long mq_timedsend(mqd_t mqdes, const char __user *msg_ptr, size_t msg_len, unsigned int msg_prio, const struct timespec __user *abs_timeout);
> +asmlinkage ssize_t mq_timedreceive(mqd_t mqdes, char __user *msg_ptr, size_t msg_len, unsigned int __user *msg_prio, const struct timespec __user *abs_timeout);
> +asmlinkage long mq_notify(mqd_t mqdes, const struct sigevent __user *notification);
> +asmlinkage long mq_getsetattr(mqd_t mqdes, const struct mq_attr __user *mqstat, struct mq_attr __user *omqstat);

this probably fits better into randy's new syscall.h now in -mm.  Also
what do you need the prototypes for?

Re: [RFC][PATCH] 2/6 POSIX message queues

[Sam Ravnborg]

On Thu, Feb 19, 2004 at 02:53:31PM +0000, Christoph Hellwig wrote:
> > +#ifndef _LINUX_MQUEUE_H
> > +#define _LINUX_MQUEUE_H
> > +
> > +#define MQ_PRIO_MAX 	32768
> > +
> > +typedef int mqd_t;
> > +
> > +struct mq_attr {
> > +	long	mq_flags;	/* message queue flags			*/
> > +	long	mq_maxmsg;	/* maximum number of messages		*/
> > +	long	mq_msgsize;	/* maximum message size			*/
> > +	long	mq_curmsgs;	/* number of messages currently queued	*/
> > +};
> > +
> > +#define NOTIFY_NONE	0
> > +#define NOTIFY_WOKENUP	1
> > +#define NOTIFY_REMOVED	2
> > +
> > +#ifdef __KERNEL__
> > +#include <linux/types.h>
> > +#include <linux/time.h>
> 
> This looks like you want glibc to include it, please don't - add a copy
> without the kernel part to glibc instead.

In the previous threads about this the concept of kernel-only and shared
user and kernel headers has been seen as a good solution.
So a split in a kernel-only part, and another kernel+user part seems more
worth-while. And less error-prone.

Maybe something like:
mqueue.h	for kernel-only
mqueue_abi.h	for kernel+user

	Sam

Re: [RFC][PATCH] 2/6 POSIX message queues

[Manfred Spraul]

Sam Ravnborg wrote:

>Maybe something like:
>mqueue.h	for kernel-only
>mqueue_abi.h	for kernel+user
>  
>
I don't think that this is necessary. Everything in <linux/> is kernel 
only. user space should copy the headers and remove the kernel only 
parts. kernel+user files mean that it's not possible rename structures 
or move them around. Perhaps someone wants to move all 16-bit uid 
structures into a <linux/compat/> directly - shared headers make that 
impossible.

I agree that the placement of the #ifdef is a bit arbitrary - it's a 
hint that the structures outside are visible to user space and must 
remain unchanged.

Christoph: I'm sure there will be users that must call the message queue 
functions directly from C code. E.g. the 32-bit emulation layers on 
64/32 bit archs. And they need the prototypes.

--
    Manfred