[Qemu-devel] [PATCH] Fix vfork() syscall emulation

[Qemu-devel] [PATCH] Fix vfork() syscall emulation

[Kirill A. Shutemov]

vfork() is a kind of fork, not thread despite CLONE_VM

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 linux-user/syscall.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 56b4138..124d14e 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2788,7 +2788,7 @@ static int do_fork(CPUState *env, unsigned int flags, abi_ulong newsp,
     sigset_t sigmask;
 #endif
 
-    if (flags & CLONE_VM) {
+    if (!(flags & CLONE_VFORK) && (flags & CLONE_VM)) {
 #if defined(USE_NPTL)
         new_thread_info info;
         pthread_attr_t attr;
@@ -2857,8 +2857,8 @@ static int do_fork(CPUState *env, unsigned int flags, abi_ulong newsp,
 #endif
 #endif
     } else {
-        /* if no CLONE_VM, we consider it is a fork */
-        if ((flags & ~(CSIGNAL | CLONE_NPTL_FLAGS2)) != 0)
+        /* we consider it is a fork or vfork */
+        if ((flags & ~(CSIGNAL | CLONE_NPTL_FLAGS2 | CLONE_VFORK | CLONE_VM)) != 0)
             return -EINVAL;
         fork_start();
         ret = fork();
-- 
1.5.6.5.GIT

[Qemu-devel] [PATCH] Fix getgroups() syscall emulation

[Kirill A. Shutemov]

According to man page getgroups(2):

If size is zero, list is not modified, but the total number of
supplementary group IDs for the process is returned.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 linux-user/syscall.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 124d14e..948ea3b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5247,6 +5247,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
             grouplist = alloca(gidsetsize * sizeof(gid_t));
             ret = get_errno(getgroups(gidsetsize, grouplist));
+            if (gidsetsize == 0)
+                break;
             if (!is_error(ret)) {
                 target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 2, 0);
                 if (!target_grouplist)
@@ -5397,6 +5399,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
             grouplist = alloca(gidsetsize * sizeof(gid_t));
             ret = get_errno(getgroups(gidsetsize, grouplist));
+            if (gidsetsize == 0)
+                break;
             if (!is_error(ret)) {
                 target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 4, 0);
                 if (!target_grouplist) {
-- 
1.5.6.5.GIT

[Qemu-devel] [PATCH] Swap only altered elements of the grouplist

[Kirill A. Shutemov]

getgroups returns the number of supplementary group IDs is returned.
So it's unnessary to swap the entire array. It can dramatically speed up
the syscall: on recent Linux kernel NGROUPS_MAX=65536.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 linux-user/syscall.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 948ea3b..ba7cde1 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5253,7 +5253,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 2, 0);
                 if (!target_grouplist)
                     goto efault;
-                for(i = 0;i < gidsetsize; i++)
+                for(i = 0;i < ret; i++)
                     target_grouplist[i] = tswap16(grouplist[i]);
                 unlock_user(target_grouplist, arg2, gidsetsize * 2);
             }
@@ -5407,7 +5407,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                     ret = -TARGET_EFAULT;
                     goto fail;
                 }
-                for(i = 0;i < gidsetsize; i++)
+                for(i = 0;i < ret; i++)
                     target_grouplist[i] = tswap32(grouplist[i]);
                 unlock_user(target_grouplist, arg2, gidsetsize * 4);
             }
-- 
1.5.6.5.GIT

[Qemu-devel] [PATCH] Fix pread() and pwrite() syscall on ARM EABI

[Kirill A. Shutemov]

pread() and pwrite() have differences with arguments on ARM EABI and
OABI.

Please, see arch/arm/kernel/entry-common.S in Linux kernel source for
additional information.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 linux-user/syscall.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ba7cde1..88b44b8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5047,12 +5047,24 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_pread
     case TARGET_NR_pread:
+#ifdef TARGET_ARM
+        if (((CPUARMState *)cpu_env)->eabi)
+        {
+            arg4 = arg5;
+        }
+#endif
         if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
             goto efault;
         ret = get_errno(pread(arg1, p, arg3, arg4));
         unlock_user(p, arg2, ret);
         break;
     case TARGET_NR_pwrite:
+#ifdef TARGET_ARM
+        if (((CPUARMState *)cpu_env)->eabi)
+        {
+            arg4 = arg5;
+        }
+#endif
         if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
             goto efault;
         ret = get_errno(pwrite(arg1, p, arg3, arg4));
-- 
1.5.6.5.GIT

[Qemu-devel] [PATCH] Implement fstatat64() syscall

[Kirill A. Shutemov]

Move transformation of struct stat64 into the separate function and
implement fstatat64() using it.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 linux-user/syscall.c |  141 +++++++++++++++++++++++++++++--------------------
 1 files changed, 83 insertions(+), 58 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 88b44b8..ac7e7d9 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -156,6 +156,7 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 arg4,type5 arg5,	\
 #define __NR_sys_faccessat __NR_faccessat
 #define __NR_sys_fchmodat __NR_fchmodat
 #define __NR_sys_fchownat __NR_fchownat
+#define __NR_sys_fstatat64 __NR_fstatat64
 #define __NR_sys_getcwd1 __NR_getcwd
 #define __NR_sys_getdents __NR_getdents
 #define __NR_sys_getdents64 __NR_getdents64
@@ -200,6 +201,10 @@ _syscall4(int,sys_fchmodat,int,dirfd,const char *,pathname,
 _syscall5(int,sys_fchownat,int,dirfd,const char *,pathname,
           uid_t,owner,gid_t,group,int,flags)
 #endif
+#if defined(TARGET_NR_fstatat64) && defined(__NR_fstatat64)
+_syscall4(int,sys_fstatat64,int,dirfd,const char *,pathname,
+         struct stat *,buf,int,flags)
+#endif
 _syscall2(int,sys_getcwd1,char *,buf,size_t,size)
 #if TARGET_ABI_BITS == 32
 _syscall3(int, sys_getdents, uint, fd, struct dirent *, dirp, uint, count);
@@ -3149,6 +3154,67 @@ static inline abi_long host_to_target_timespec(abi_ulong target_addr,
     return 0;
 }
 
+#ifdef TARGET_NR_stat64
+static inline abi_long host_to_target_stat64(void *cpu_env,
+					     abi_ulong target_addr,
+					     struct stat *host_st)
+{
+#ifdef TARGET_ARM
+	if (((CPUARMState *)cpu_env)->eabi) {
+		struct target_eabi_stat64 *target_st;
+
+		if (!lock_user_struct(VERIFY_WRITE, target_st, target_addr, 0))
+			return -TARGET_EFAULT;
+		memset(target_st, 0, sizeof(struct target_eabi_stat64));
+		__put_user(host_st->st_dev, &target_st->st_dev);
+		__put_user(host_st->st_ino, &target_st->st_ino);
+#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
+		__put_user(host_st->st_ino, &target_st->__st_ino);
+#endif
+		__put_user(host_st->st_mode, &target_st->st_mode);
+		__put_user(host_st->st_nlink, &target_st->st_nlink);
+		__put_user(host_st->st_uid, &target_st->st_uid);
+		__put_user(host_st->st_gid, &target_st->st_gid);
+		__put_user(host_st->st_rdev, &target_st->st_rdev);
+		__put_user(host_st->st_size, &target_st->st_size);
+		__put_user(host_st->st_blksize, &target_st->st_blksize);
+		__put_user(host_st->st_blocks, &target_st->st_blocks);
+		__put_user(host_st->st_atime, &target_st->target_st_atime);
+		__put_user(host_st->st_mtime, &target_st->target_st_mtime);
+		__put_user(host_st->st_ctime, &target_st->target_st_ctime);
+		unlock_user_struct(target_st, target_addr, 1);
+	} else
+#endif
+	{
+		struct target_stat64 *target_st;
+
+		if (!lock_user_struct(VERIFY_WRITE, target_st, target_addr, 0))
+			return -TARGET_EFAULT;
+		memset(target_st, 0, sizeof(struct target_stat64));
+		__put_user(host_st->st_dev, &target_st->st_dev);
+		__put_user(host_st->st_ino, &target_st->st_ino);
+#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
+		__put_user(host_st->st_ino, &target_st->__st_ino);
+#endif
+		__put_user(host_st->st_mode, &target_st->st_mode);
+		__put_user(host_st->st_nlink, &target_st->st_nlink);
+		__put_user(host_st->st_uid, &target_st->st_uid);
+		__put_user(host_st->st_gid, &target_st->st_gid);
+		__put_user(host_st->st_rdev, &target_st->st_rdev);
+		/* XXX: better use of kernel struct */
+		__put_user(host_st->st_size, &target_st->st_size);
+		__put_user(host_st->st_blksize, &target_st->st_blksize);
+		__put_user(host_st->st_blocks, &target_st->st_blocks);
+		__put_user(host_st->st_atime, &target_st->target_st_atime);
+		__put_user(host_st->st_mtime, &target_st->target_st_mtime);
+		__put_user(host_st->st_ctime, &target_st->target_st_ctime);
+		unlock_user_struct(target_st, target_addr, 1);
+	}
+
+	return 0;
+}
+#endif
+
 #if defined(USE_NPTL)
 /* ??? Using host futex calls even when target atomic operations
    are not really atomic probably breaks things.  However implementing
@@ -5154,7 +5220,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             goto efault;
         ret = get_errno(stat(path(p), &st));
         unlock_user(p, arg1, 0);
-        goto do_stat64;
+	if (!is_error(ret))
+		ret = host_to_target_stat64(cpu_env, arg2, &st);
+	break;
 #endif
 #ifdef TARGET_NR_lstat64
     case TARGET_NR_lstat64:
@@ -5162,67 +5230,24 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             goto efault;
         ret = get_errno(lstat(path(p), &st));
         unlock_user(p, arg1, 0);
-        goto do_stat64;
+	if (!is_error(ret))
+		ret = host_to_target_stat64(cpu_env, arg2, &st);
+	break;
 #endif
 #ifdef TARGET_NR_fstat64
     case TARGET_NR_fstat64:
-        {
-            ret = get_errno(fstat(arg1, &st));
-        do_stat64:
-            if (!is_error(ret)) {
-#ifdef TARGET_ARM
-                if (((CPUARMState *)cpu_env)->eabi) {
-                    struct target_eabi_stat64 *target_st;
-
-                    if (!lock_user_struct(VERIFY_WRITE, target_st, arg2, 0))
-                        goto efault;
-                    memset(target_st, 0, sizeof(struct target_eabi_stat64));
-                    __put_user(st.st_dev, &target_st->st_dev);
-                    __put_user(st.st_ino, &target_st->st_ino);
-#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
-                    __put_user(st.st_ino, &target_st->__st_ino);
-#endif
-                    __put_user(st.st_mode, &target_st->st_mode);
-                    __put_user(st.st_nlink, &target_st->st_nlink);
-                    __put_user(st.st_uid, &target_st->st_uid);
-                    __put_user(st.st_gid, &target_st->st_gid);
-                    __put_user(st.st_rdev, &target_st->st_rdev);
-                    __put_user(st.st_size, &target_st->st_size);
-                    __put_user(st.st_blksize, &target_st->st_blksize);
-                    __put_user(st.st_blocks, &target_st->st_blocks);
-                    __put_user(st.st_atime, &target_st->target_st_atime);
-                    __put_user(st.st_mtime, &target_st->target_st_mtime);
-                    __put_user(st.st_ctime, &target_st->target_st_ctime);
-                    unlock_user_struct(target_st, arg2, 1);
-                } else
+	ret = get_errno(fstat(arg1, &st));
+	if (!is_error(ret))
+		ret = host_to_target_stat64(cpu_env, arg2, &st);
+        break;
 #endif
-                {
-                    struct target_stat64 *target_st;
-
-                    if (!lock_user_struct(VERIFY_WRITE, target_st, arg2, 0))
-                        goto efault;
-                    memset(target_st, 0, sizeof(struct target_stat64));
-                    __put_user(st.st_dev, &target_st->st_dev);
-                    __put_user(st.st_ino, &target_st->st_ino);
-#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
-                    __put_user(st.st_ino, &target_st->__st_ino);
-#endif
-                    __put_user(st.st_mode, &target_st->st_mode);
-                    __put_user(st.st_nlink, &target_st->st_nlink);
-                    __put_user(st.st_uid, &target_st->st_uid);
-                    __put_user(st.st_gid, &target_st->st_gid);
-                    __put_user(st.st_rdev, &target_st->st_rdev);
-                    /* XXX: better use of kernel struct */
-                    __put_user(st.st_size, &target_st->st_size);
-                    __put_user(st.st_blksize, &target_st->st_blksize);
-                    __put_user(st.st_blocks, &target_st->st_blocks);
-                    __put_user(st.st_atime, &target_st->target_st_atime);
-                    __put_user(st.st_mtime, &target_st->target_st_mtime);
-                    __put_user(st.st_ctime, &target_st->target_st_ctime);
-                    unlock_user_struct(target_st, arg2, 1);
-                }
-            }
-        }
+#if defined(TARGET_NR_fstatat64) && defined(__NR_fstatat64)
+    case TARGET_NR_fstatat64:
+        if (!(p = lock_user_string(arg2)))
+            goto efault;
+	ret = get_errno(sys_fstatat64(arg1, path(p), &st, arg4));
+	if (!is_error(ret))
+		ret = host_to_target_stat64(cpu_env, arg3, &st);
         break;
 #endif
 #ifdef USE_UID16
-- 
1.5.6.5.GIT

[Qemu-devel] [PATCH] Implement futimesat() syscall

[Kirill A. Shutemov]

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 linux-user/syscall.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ac7e7d9..e90f100 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -157,6 +157,7 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 arg4,type5 arg5,	\
 #define __NR_sys_fchmodat __NR_fchmodat
 #define __NR_sys_fchownat __NR_fchownat
 #define __NR_sys_fstatat64 __NR_fstatat64
+#define __NR_sys_futimesat __NR_futimesat
 #define __NR_sys_getcwd1 __NR_getcwd
 #define __NR_sys_getdents __NR_getdents
 #define __NR_sys_getdents64 __NR_getdents64
@@ -205,6 +206,10 @@ _syscall5(int,sys_fchownat,int,dirfd,const char *,pathname,
 _syscall4(int,sys_fstatat64,int,dirfd,const char *,pathname,
          struct stat *,buf,int,flags)
 #endif
+#if defined(TARGET_NR_futimesat) && defined(__NR_futimesat)
+_syscall3(int,sys_futimesat,int,dirfd,const char *,pathname,
+          const struct timeval *,times)
+#endif
 _syscall2(int,sys_getcwd1,char *,buf,size_t,size)
 #if TARGET_ABI_BITS == 32
 _syscall3(int, sys_getdents, uint, fd, struct dirent *, dirp, uint, count);
@@ -3662,6 +3667,26 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             unlock_user(p, arg1, 0);
         }
         break;
+#if defined(TARGET_NR_futimesat) && defined(__NR_futimesat)
+    case TARGET_NR_futimesat:
+        {
+            struct timeval *tvp, tv[2];
+            if (arg3) {
+                if (copy_from_user_timeval(&tv[0], arg3)
+                    || copy_from_user_timeval(&tv[1],
+                                              arg3 + sizeof(struct target_timeval)))
+                    goto efault;
+                tvp = tv;
+            } else {
+                tvp = NULL;
+            }
+            if (!(p = lock_user_string(arg2)))
+                goto efault;
+            ret = get_errno(sys_futimesat(arg1, path(p), tvp));
+            unlock_user(p, arg2, 0);
+        }
+        break;
+#endif
 #ifdef TARGET_NR_stty
     case TARGET_NR_stty:
         goto unimplemented;
-- 
1.5.6.5.GIT

[Qemu-devel] [PATCH] Imaplement ioctls MTIOCTOP, MTIOCGET and MTIOCPOS

[Kirill A. Shutemov]

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 linux-user/ioctls.h        |    4 ++++
 linux-user/syscall.c       |    1 +
 linux-user/syscall_defs.h  |    4 ++++
 linux-user/syscall_types.h |    6 ++++++
 4 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index c1ca2d5..685cc71 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -310,3 +310,7 @@
   IOCTL(LOOP_GET_STATUS64, IOC_W, MK_PTR(MK_STRUCT(STRUCT_loop_info64)))
 #endif
   IOCTL(LOOP_CHANGE_FD, 0, TYPE_INT)
+
+  IOCTL(MTIOCTOP, IOC_W, MK_PTR(MK_STRUCT(STRUCT_mtop)))
+  IOCTL(MTIOCGET, IOC_R, MK_PTR(MK_STRUCT(STRUCT_mtget)))
+  IOCTL(MTIOCPOS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_mtpos)))
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e90f100..ce99cf6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -70,6 +70,7 @@
 #include <linux/soundcard.h>
 #include <linux/dirent.h>
 #include <linux/kd.h>
+#include <linux/mtio.h>
 #include "linux_loop.h"
 
 #include "qemu.h"
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 52242b6..9896522 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1923,6 +1923,10 @@ struct target_eabi_flock64 {
 #define TARGET_VFAT_IOCTL_READDIR_BOTH    TARGET_IORU('r', 1)
 #define TARGET_VFAT_IOCTL_READDIR_SHORT   TARGET_IORU('r', 2)
 
+#define TARGET_MTIOCTOP        TARGET_IOW('m', 1, struct mtop)
+#define TARGET_MTIOCGET        TARGET_IOR('m', 2, struct mtget)
+#define TARGET_MTIOCPOS        TARGET_IOR('m', 3, struct mtpos)
+
 struct target_sysinfo {
     abi_long uptime;                /* Seconds since boot */
     abi_ulong loads[3];             /* 1, 5, and 15 minute load averages */
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index bb4fb4e..283d32d 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -108,3 +108,9 @@ STRUCT(loop_info64,
        MK_ARRAY(TYPE_CHAR, 64),  /* lo_crypt_name */
        MK_ARRAY(TYPE_CHAR, 32),  /* lo_encrypt_key */
        MK_ARRAY(TYPE_ULONGLONG, 2))  /* lo_init */
+
+
+STRUCT(mtop, TYPE_SHORT, TYPE_INT)
+STRUCT(mtget, TYPE_LONG, TYPE_LONG, TYPE_LONG, TYPE_LONG, TYPE_LONG,
+       TYPE_INT, TYPE_INT)
+STRUCT(mtpos, TYPE_LONG)
-- 
1.5.6.5.GIT

[Qemu-devel] [PATCH] Fix building with 2.6.27 kernel headers

[Kirill A. Shutemov]

linux/dirent unexported in 2.6.27

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 linux-user/syscall.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ce99cf6..4cde5e8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -28,6 +28,7 @@
 #include <fcntl.h>
 #include <time.h>
 #include <limits.h>
+#include <dirent.h>
 #include <sys/types.h>
 #include <sys/ipc.h>
 #include <sys/msg.h>
@@ -68,7 +69,6 @@
 #include <linux/cdrom.h>
 #include <linux/hdreg.h>
 #include <linux/soundcard.h>
-#include <linux/dirent.h>
 #include <linux/kd.h>
 #include <linux/mtio.h>
 #include "linux_loop.h"
-- 
1.5.6.5.GIT

Re: [Qemu-devel] [PATCH] Fix building with 2.6.27 kernel headers

[Riku Voipio]

On Thu, Sep 18, 2008 at 06:07:06PM +0300, Kirill A. Shutemov wrote:
> linux/dirent unexported in 2.6.27
> 
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>

Looks correct,

Acked-By: Riku Voipio <riku.voipio@iki.fi>

> ---
>  linux-user/syscall.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ce99cf6..4cde5e8 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -28,6 +28,7 @@
>  #include <fcntl.h>
>  #include <time.h>
>  #include <limits.h>
> +#include <dirent.h>
>  #include <sys/types.h>
>  #include <sys/ipc.h>
>  #include <sys/msg.h>
> @@ -68,7 +69,6 @@
>  #include <linux/cdrom.h>
>  #include <linux/hdreg.h>
>  #include <linux/soundcard.h>
> -#include <linux/dirent.h>
>  #include <linux/kd.h>
>  #include <linux/mtio.h>
>  #include "linux_loop.h"
> -- 
> 1.5.6.5.GIT
> 
> 

-- 
"rm -rf" only sounds scary if you don't have backups

Re: [Qemu-devel] [PATCH] Implement futimesat() syscall

[Riku Voipio]

On Thu, Sep 18, 2008 at 06:07:04PM +0300, Kirill A. Shutemov wrote:
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>

Acked-by: Riku Voipio <riku.voipio@iki.fi>

> ---
>  linux-user/syscall.c |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ac7e7d9..e90f100 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -157,6 +157,7 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 arg4,type5 arg5,	\
>  #define __NR_sys_fchmodat __NR_fchmodat
>  #define __NR_sys_fchownat __NR_fchownat
>  #define __NR_sys_fstatat64 __NR_fstatat64
> +#define __NR_sys_futimesat __NR_futimesat
>  #define __NR_sys_getcwd1 __NR_getcwd
>  #define __NR_sys_getdents __NR_getdents
>  #define __NR_sys_getdents64 __NR_getdents64
> @@ -205,6 +206,10 @@ _syscall5(int,sys_fchownat,int,dirfd,const char *,pathname,
>  _syscall4(int,sys_fstatat64,int,dirfd,const char *,pathname,
>           struct stat *,buf,int,flags)
>  #endif
> +#if defined(TARGET_NR_futimesat) && defined(__NR_futimesat)
> +_syscall3(int,sys_futimesat,int,dirfd,const char *,pathname,
> +          const struct timeval *,times)
> +#endif
>  _syscall2(int,sys_getcwd1,char *,buf,size_t,size)
>  #if TARGET_ABI_BITS == 32
>  _syscall3(int, sys_getdents, uint, fd, struct dirent *, dirp, uint, count);
> @@ -3662,6 +3667,26 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              unlock_user(p, arg1, 0);
>          }
>          break;
> +#if defined(TARGET_NR_futimesat) && defined(__NR_futimesat)
> +    case TARGET_NR_futimesat:
> +        {
> +            struct timeval *tvp, tv[2];
> +            if (arg3) {
> +                if (copy_from_user_timeval(&tv[0], arg3)
> +                    || copy_from_user_timeval(&tv[1],
> +                                              arg3 + sizeof(struct target_timeval)))
> +                    goto efault;
> +                tvp = tv;
> +            } else {
> +                tvp = NULL;
> +            }
> +            if (!(p = lock_user_string(arg2)))
> +                goto efault;
> +            ret = get_errno(sys_futimesat(arg1, path(p), tvp));
> +            unlock_user(p, arg2, 0);
> +        }
> +        break;
> +#endif
>  #ifdef TARGET_NR_stty
>      case TARGET_NR_stty:
>          goto unimplemented;
> -- 
> 1.5.6.5.GIT
> 
> 

-- 
"rm -rf" only sounds scary if you don't have backups

Re: [Qemu-devel] [PATCH] Implement fstatat64() syscall

[Riku Voipio]

On Thu, Sep 18, 2008 at 06:07:03PM +0300, Kirill A. Shutemov wrote:
> Move transformation of struct stat64 into the separate function and
> implement fstatat64() using it.
> 
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> ---
>  linux-user/syscall.c |  141 +++++++++++++++++++++++++++++--------------------
>  1 files changed, 83 insertions(+), 58 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 88b44b8..ac7e7d9 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -156,6 +156,7 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 arg4,type5 arg5,	\
>  #define __NR_sys_faccessat __NR_faccessat
>  #define __NR_sys_fchmodat __NR_fchmodat
>  #define __NR_sys_fchownat __NR_fchownat
> +#define __NR_sys_fstatat64 __NR_fstatat64
>  #define __NR_sys_getcwd1 __NR_getcwd
>  #define __NR_sys_getdents __NR_getdents
>  #define __NR_sys_getdents64 __NR_getdents64
> @@ -200,6 +201,10 @@ _syscall4(int,sys_fchmodat,int,dirfd,const char *,pathname,
>  _syscall5(int,sys_fchownat,int,dirfd,const char *,pathname,
>            uid_t,owner,gid_t,group,int,flags)
>  #endif
> +#if defined(TARGET_NR_fstatat64) && defined(__NR_fstatat64)
> +_syscall4(int,sys_fstatat64,int,dirfd,const char *,pathname,
> +         struct stat *,buf,int,flags)
> +#endif
>  _syscall2(int,sys_getcwd1,char *,buf,size_t,size)
>  #if TARGET_ABI_BITS == 32
>  _syscall3(int, sys_getdents, uint, fd, struct dirent *, dirp, uint, count);
> @@ -3149,6 +3154,67 @@ static inline abi_long host_to_target_timespec(abi_ulong target_addr,
>      return 0;
>  }
>  
> +#ifdef TARGET_NR_stat64
> +static inline abi_long host_to_target_stat64(void *cpu_env,
> +					     abi_ulong target_addr,
> +					     struct stat *host_st)
> +{
> +#ifdef TARGET_ARM
> +	if (((CPUARMState *)cpu_env)->eabi) {
> +		struct target_eabi_stat64 *target_st;
> +
> +		if (!lock_user_struct(VERIFY_WRITE, target_st, target_addr, 0))
> +			return -TARGET_EFAULT;
> +		memset(target_st, 0, sizeof(struct target_eabi_stat64));
> +		__put_user(host_st->st_dev, &target_st->st_dev);
> +		__put_user(host_st->st_ino, &target_st->st_ino);
> +#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
> +		__put_user(host_st->st_ino, &target_st->__st_ino);
> +#endif
> +		__put_user(host_st->st_mode, &target_st->st_mode);
> +		__put_user(host_st->st_nlink, &target_st->st_nlink);
> +		__put_user(host_st->st_uid, &target_st->st_uid);
> +		__put_user(host_st->st_gid, &target_st->st_gid);
> +		__put_user(host_st->st_rdev, &target_st->st_rdev);
> +		__put_user(host_st->st_size, &target_st->st_size);
> +		__put_user(host_st->st_blksize, &target_st->st_blksize);
> +		__put_user(host_st->st_blocks, &target_st->st_blocks);
> +		__put_user(host_st->st_atime, &target_st->target_st_atime);
> +		__put_user(host_st->st_mtime, &target_st->target_st_mtime);
> +		__put_user(host_st->st_ctime, &target_st->target_st_ctime);
> +		unlock_user_struct(target_st, target_addr, 1);
> +	} else
> +#endif
> +	{
> +		struct target_stat64 *target_st;
> +
> +		if (!lock_user_struct(VERIFY_WRITE, target_st, target_addr, 0))
> +			return -TARGET_EFAULT;
> +		memset(target_st, 0, sizeof(struct target_stat64));
> +		__put_user(host_st->st_dev, &target_st->st_dev);
> +		__put_user(host_st->st_ino, &target_st->st_ino);
> +#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
> +		__put_user(host_st->st_ino, &target_st->__st_ino);
> +#endif
> +		__put_user(host_st->st_mode, &target_st->st_mode);
> +		__put_user(host_st->st_nlink, &target_st->st_nlink);
> +		__put_user(host_st->st_uid, &target_st->st_uid);
> +		__put_user(host_st->st_gid, &target_st->st_gid);
> +		__put_user(host_st->st_rdev, &target_st->st_rdev);
> +		/* XXX: better use of kernel struct */
> +		__put_user(host_st->st_size, &target_st->st_size);
> +		__put_user(host_st->st_blksize, &target_st->st_blksize);
> +		__put_user(host_st->st_blocks, &target_st->st_blocks);
> +		__put_user(host_st->st_atime, &target_st->target_st_atime);
> +		__put_user(host_st->st_mtime, &target_st->target_st_mtime);
> +		__put_user(host_st->st_ctime, &target_st->target_st_ctime);
> +		unlock_user_struct(target_st, target_addr, 1);
> +	}
> +
> +	return 0;
> +}
> +#endif

This is suboptimal - we same code (list of __put_user()) twice. We
should have smaller if/else in the beginning of the function that sets
target_st.

> +
>  #if defined(USE_NPTL)
>  /* ??? Using host futex calls even when target atomic operations
>     are not really atomic probably breaks things.  However implementing
> @@ -5154,7 +5220,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              goto efault;
>          ret = get_errno(stat(path(p), &st));
>          unlock_user(p, arg1, 0);
> -        goto do_stat64;
> +	if (!is_error(ret))
> +		ret = host_to_target_stat64(cpu_env, arg2, &st);
> +	break;
>  #endif
>  #ifdef TARGET_NR_lstat64
>      case TARGET_NR_lstat64:
> @@ -5162,67 +5230,24 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              goto efault;
>          ret = get_errno(lstat(path(p), &st));
>          unlock_user(p, arg1, 0);
> -        goto do_stat64;
> +	if (!is_error(ret))
> +		ret = host_to_target_stat64(cpu_env, arg2, &st);
> +	break;
>  #endif
>  #ifdef TARGET_NR_fstat64
>      case TARGET_NR_fstat64:
> -        {
> -            ret = get_errno(fstat(arg1, &st));
> -        do_stat64:
> -            if (!is_error(ret)) {
> -#ifdef TARGET_ARM
> -                if (((CPUARMState *)cpu_env)->eabi) {
> -                    struct target_eabi_stat64 *target_st;
> -
> -                    if (!lock_user_struct(VERIFY_WRITE, target_st, arg2, 0))
> -                        goto efault;
> -                    memset(target_st, 0, sizeof(struct target_eabi_stat64));
> -                    __put_user(st.st_dev, &target_st->st_dev);
> -                    __put_user(st.st_ino, &target_st->st_ino);
> -#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
> -                    __put_user(st.st_ino, &target_st->__st_ino);
> -#endif
> -                    __put_user(st.st_mode, &target_st->st_mode);
> -                    __put_user(st.st_nlink, &target_st->st_nlink);
> -                    __put_user(st.st_uid, &target_st->st_uid);
> -                    __put_user(st.st_gid, &target_st->st_gid);
> -                    __put_user(st.st_rdev, &target_st->st_rdev);
> -                    __put_user(st.st_size, &target_st->st_size);
> -                    __put_user(st.st_blksize, &target_st->st_blksize);
> -                    __put_user(st.st_blocks, &target_st->st_blocks);
> -                    __put_user(st.st_atime, &target_st->target_st_atime);
> -                    __put_user(st.st_mtime, &target_st->target_st_mtime);
> -                    __put_user(st.st_ctime, &target_st->target_st_ctime);
> -                    unlock_user_struct(target_st, arg2, 1);
> -                } else
> +	ret = get_errno(fstat(arg1, &st));
> +	if (!is_error(ret))
> +		ret = host_to_target_stat64(cpu_env, arg2, &st);
> +        break;
>  #endif
> -                {
> -                    struct target_stat64 *target_st;
> -
> -                    if (!lock_user_struct(VERIFY_WRITE, target_st, arg2, 0))
> -                        goto efault;
> -                    memset(target_st, 0, sizeof(struct target_stat64));
> -                    __put_user(st.st_dev, &target_st->st_dev);
> -                    __put_user(st.st_ino, &target_st->st_ino);
> -#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
> -                    __put_user(st.st_ino, &target_st->__st_ino);
> -#endif
> -                    __put_user(st.st_mode, &target_st->st_mode);
> -                    __put_user(st.st_nlink, &target_st->st_nlink);
> -                    __put_user(st.st_uid, &target_st->st_uid);
> -                    __put_user(st.st_gid, &target_st->st_gid);
> -                    __put_user(st.st_rdev, &target_st->st_rdev);
> -                    /* XXX: better use of kernel struct */
> -                    __put_user(st.st_size, &target_st->st_size);
> -                    __put_user(st.st_blksize, &target_st->st_blksize);
> -                    __put_user(st.st_blocks, &target_st->st_blocks);
> -                    __put_user(st.st_atime, &target_st->target_st_atime);
> -                    __put_user(st.st_mtime, &target_st->target_st_mtime);
> -                    __put_user(st.st_ctime, &target_st->target_st_ctime);
> -                    unlock_user_struct(target_st, arg2, 1);
> -                }
> -            }
> -        }
> +#if defined(TARGET_NR_fstatat64) && defined(__NR_fstatat64)
> +    case TARGET_NR_fstatat64:
> +        if (!(p = lock_user_string(arg2)))
> +            goto efault;
> +	ret = get_errno(sys_fstatat64(arg1, path(p), &st, arg4));
> +	if (!is_error(ret))
> +		ret = host_to_target_stat64(cpu_env, arg3, &st);
>          break;
>  #endif
>  #ifdef USE_UID16
> -- 
> 1.5.6.5.GIT
> 
> 

-- 
"rm -rf" only sounds scary if you don't have backups

Re: [Qemu-devel] [PATCH] Implement fstatat64() syscall

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 4925 bytes --]

On Fri, Sep 19, 2008 at 05:09:18PM +0300, Riku Voipio wrote:
> On Thu, Sep 18, 2008 at 06:07:03PM +0300, Kirill A. Shutemov wrote:
> > Move transformation of struct stat64 into the separate function and
> > implement fstatat64() using it.
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> > ---
> >  linux-user/syscall.c |  141 +++++++++++++++++++++++++++++--------------------
> >  1 files changed, 83 insertions(+), 58 deletions(-)
> > 
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 88b44b8..ac7e7d9 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -156,6 +156,7 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 arg4,type5 arg5,	\
> >  #define __NR_sys_faccessat __NR_faccessat
> >  #define __NR_sys_fchmodat __NR_fchmodat
> >  #define __NR_sys_fchownat __NR_fchownat
> > +#define __NR_sys_fstatat64 __NR_fstatat64
> >  #define __NR_sys_getcwd1 __NR_getcwd
> >  #define __NR_sys_getdents __NR_getdents
> >  #define __NR_sys_getdents64 __NR_getdents64
> > @@ -200,6 +201,10 @@ _syscall4(int,sys_fchmodat,int,dirfd,const char *,pathname,
> >  _syscall5(int,sys_fchownat,int,dirfd,const char *,pathname,
> >            uid_t,owner,gid_t,group,int,flags)
> >  #endif
> > +#if defined(TARGET_NR_fstatat64) && defined(__NR_fstatat64)
> > +_syscall4(int,sys_fstatat64,int,dirfd,const char *,pathname,
> > +         struct stat *,buf,int,flags)
> > +#endif
> >  _syscall2(int,sys_getcwd1,char *,buf,size_t,size)
> >  #if TARGET_ABI_BITS == 32
> >  _syscall3(int, sys_getdents, uint, fd, struct dirent *, dirp, uint, count);
> > @@ -3149,6 +3154,67 @@ static inline abi_long host_to_target_timespec(abi_ulong target_addr,
> >      return 0;
> >  }
> >  
> > +#ifdef TARGET_NR_stat64
> > +static inline abi_long host_to_target_stat64(void *cpu_env,
> > +					     abi_ulong target_addr,
> > +					     struct stat *host_st)
> > +{
> > +#ifdef TARGET_ARM
> > +	if (((CPUARMState *)cpu_env)->eabi) {
> > +		struct target_eabi_stat64 *target_st;
> > +
> > +		if (!lock_user_struct(VERIFY_WRITE, target_st, target_addr, 0))
> > +			return -TARGET_EFAULT;
> > +		memset(target_st, 0, sizeof(struct target_eabi_stat64));
> > +		__put_user(host_st->st_dev, &target_st->st_dev);
> > +		__put_user(host_st->st_ino, &target_st->st_ino);
> > +#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
> > +		__put_user(host_st->st_ino, &target_st->__st_ino);
> > +#endif
> > +		__put_user(host_st->st_mode, &target_st->st_mode);
> > +		__put_user(host_st->st_nlink, &target_st->st_nlink);
> > +		__put_user(host_st->st_uid, &target_st->st_uid);
> > +		__put_user(host_st->st_gid, &target_st->st_gid);
> > +		__put_user(host_st->st_rdev, &target_st->st_rdev);
> > +		__put_user(host_st->st_size, &target_st->st_size);
> > +		__put_user(host_st->st_blksize, &target_st->st_blksize);
> > +		__put_user(host_st->st_blocks, &target_st->st_blocks);
> > +		__put_user(host_st->st_atime, &target_st->target_st_atime);
> > +		__put_user(host_st->st_mtime, &target_st->target_st_mtime);
> > +		__put_user(host_st->st_ctime, &target_st->target_st_ctime);
> > +		unlock_user_struct(target_st, target_addr, 1);
> > +	} else
> > +#endif
> > +	{
> > +		struct target_stat64 *target_st;
> > +
> > +		if (!lock_user_struct(VERIFY_WRITE, target_st, target_addr, 0))
> > +			return -TARGET_EFAULT;
> > +		memset(target_st, 0, sizeof(struct target_stat64));
> > +		__put_user(host_st->st_dev, &target_st->st_dev);
> > +		__put_user(host_st->st_ino, &target_st->st_ino);
> > +#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
> > +		__put_user(host_st->st_ino, &target_st->__st_ino);
> > +#endif
> > +		__put_user(host_st->st_mode, &target_st->st_mode);
> > +		__put_user(host_st->st_nlink, &target_st->st_nlink);
> > +		__put_user(host_st->st_uid, &target_st->st_uid);
> > +		__put_user(host_st->st_gid, &target_st->st_gid);
> > +		__put_user(host_st->st_rdev, &target_st->st_rdev);
> > +		/* XXX: better use of kernel struct */
> > +		__put_user(host_st->st_size, &target_st->st_size);
> > +		__put_user(host_st->st_blksize, &target_st->st_blksize);
> > +		__put_user(host_st->st_blocks, &target_st->st_blocks);
> > +		__put_user(host_st->st_atime, &target_st->target_st_atime);
> > +		__put_user(host_st->st_mtime, &target_st->target_st_mtime);
> > +		__put_user(host_st->st_ctime, &target_st->target_st_ctime);
> > +		unlock_user_struct(target_st, target_addr, 1);
> > +	}
> > +
> > +	return 0;
> > +}
> > +#endif
> 
> This is suboptimal - we same code (list of __put_user()) twice. We
> should have smaller if/else in the beginning of the function that sets
> target_st.

Pay attention that struct in 'if' and in 'else' is different. There is no
way to make it pretty(without dirty preprocessing hacks).

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH] Swap only altered elements of the grouplist

[Riku Voipio]

On Thu, Sep 18, 2008 at 06:07:01PM +0300, Kirill A. Shutemov wrote:
> getgroups returns the number of supplementary group IDs is returned.
> So it's unnessary to swap the entire array. It can dramatically speed up
> the syscall: on recent Linux kernel NGROUPS_MAX=65536.

looks ok too.

> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> ---
>  linux-user/syscall.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 948ea3b..ba7cde1 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5253,7 +5253,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 2, 0);
>                  if (!target_grouplist)
>                      goto efault;
> -                for(i = 0;i < gidsetsize; i++)
> +                for(i = 0;i < ret; i++)
>                      target_grouplist[i] = tswap16(grouplist[i]);
>                  unlock_user(target_grouplist, arg2, gidsetsize * 2);
>              }
> @@ -5407,7 +5407,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                      ret = -TARGET_EFAULT;
>                      goto fail;
>                  }
> -                for(i = 0;i < gidsetsize; i++)
> +                for(i = 0;i < ret; i++)
>                      target_grouplist[i] = tswap32(grouplist[i]);
>                  unlock_user(target_grouplist, arg2, gidsetsize * 4);
>              }
> -- 
> 1.5.6.5.GIT
> 
> 

-- 
"rm -rf" only sounds scary if you don't have backups

Re: [Qemu-devel] [PATCH] Fix getgroups() syscall emulation

[Riku Voipio]

On Thu, Sep 18, 2008 at 06:07:00PM +0300, Kirill A. Shutemov wrote:
> According to man page getgroups(2):
> 
> If size is zero, list is not modified, but the total number of
> supplementary group IDs for the process is returned.

Looks ok to me.

> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> ---
>  linux-user/syscall.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 124d14e..948ea3b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5247,6 +5247,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  
>              grouplist = alloca(gidsetsize * sizeof(gid_t));
>              ret = get_errno(getgroups(gidsetsize, grouplist));
> +            if (gidsetsize == 0)
> +                break;
>              if (!is_error(ret)) {
>                  target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 2, 0);
>                  if (!target_grouplist)
> @@ -5397,6 +5399,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  
>              grouplist = alloca(gidsetsize * sizeof(gid_t));
>              ret = get_errno(getgroups(gidsetsize, grouplist));
> +            if (gidsetsize == 0)
> +                break;
>              if (!is_error(ret)) {
>                  target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 4, 0);
>                  if (!target_grouplist) {
> -- 
> 1.5.6.5.GIT
> 
> 

-- 
"rm -rf" only sounds scary if you don't have backups

Re: [Qemu-devel] [PATCH] Fix vfork() syscall emulation

[andrzej zaborowski]

2008/9/18 Kirill A. Shutemov <kirill@shutemov.name>:
> vfork() is a kind of fork, not thread despite CLONE_VM

According to clone(2) it can be either, the only difference is that
vfork() suspends the parent process.  So if CLONE_VM is set, I think
still the pthread / clone way should be used and the child thread
should be waited on.

On the other hand the patch makes fork() and vfork() be treated identically?

Cheers

Re: [Qemu-devel] [PATCH] Fix vfork() syscall emulation

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

On Sat, Sep 20, 2008 at 04:56:45AM +0200, andrzej zaborowski wrote:
> 2008/9/18 Kirill A. Shutemov <kirill@shutemov.name>:
> > vfork() is a kind of fork, not thread despite CLONE_VM
> 
> According to clone(2) it can be either, the only difference is that
> vfork() suspends the parent process.  So if CLONE_VM is set, I think
> still the pthread / clone way should be used and the child thread
> should be waited on.

vfork() suspends the parent process until a call of execve(2) or _exit(2).
If child call execnv(2) it replaces whole process, not only the thread.
If child call _exit(2) it stops while process, not only the thread.

> On the other hand the patch makes fork() and vfork() be treated identically?

$ cat usr/klibc/vfork.c
/*
 * vfork.c
 *
 * Emulate vfork() with fork() if necessary
 */

#include <unistd.h>
#include <klibc/compiler.h>
#include <klibc/sysconfig.h>

#if !_KLIBC_NO_MMU && !_KLIBC_REAL_VFORK
int vfork(void)
{
        return fork();
}
#endif

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH] Fix vfork() syscall emulation

[andrzej zaborowski]

2008/9/20 Kirill A. Shutemov <kirill@shutemov.name>:
> On Sat, Sep 20, 2008 at 04:56:45AM +0200, andrzej zaborowski wrote:
>> 2008/9/18 Kirill A. Shutemov <kirill@shutemov.name>:
>> > vfork() is a kind of fork, not thread despite CLONE_VM
>>
>> According to clone(2) it can be either, the only difference is that
>> vfork() suspends the parent process.  So if CLONE_VM is set, I think
>> still the pthread / clone way should be used and the child thread
>> should be waited on.
>
> vfork() suspends the parent process until a call of execve(2) or _exit(2).
> If child call execnv(2) it replaces whole process, not only the thread.
> If child call _exit(2) it stops while process, not only the thread.

Do you mean that's the current behavior in qemu?  That's not what clone(2) says.

>
>> On the other hand the patch makes fork() and vfork() be treated identically?
>
> $ cat usr/klibc/vfork.c
> /*
>  * vfork.c
>  *
>  * Emulate vfork() with fork() if necessary
>  */
>
> #include <unistd.h>
> #include <klibc/compiler.h>
> #include <klibc/sysconfig.h>
>
> #if !_KLIBC_NO_MMU && !_KLIBC_REAL_VFORK
> int vfork(void)
> {
>        return fork();
> }
> #endif

Well, that's libc. clone with CLONE_VFORK and without it are still not
the same thing.  CLONE_VM and CLONE_VFORK are ortogonal to one
another, not the opposite of.

Cheers

Re: [Qemu-devel] [PATCH] Fix vfork() syscall emulation

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 2249 bytes --]

On Sat, Sep 20, 2008 at 02:45:57PM +0200, andrzej zaborowski wrote:
> 2008/9/20 Kirill A. Shutemov <kirill@shutemov.name>:
> > On Sat, Sep 20, 2008 at 04:56:45AM +0200, andrzej zaborowski wrote:
> >> 2008/9/18 Kirill A. Shutemov <kirill@shutemov.name>:
> >> > vfork() is a kind of fork, not thread despite CLONE_VM
> >>
> >> According to clone(2) it can be either, the only difference is that
> >> vfork() suspends the parent process.  So if CLONE_VM is set, I think
> >> still the pthread / clone way should be used and the child thread
> >> should be waited on.
> >
> > vfork() suspends the parent process until a call of execve(2) or _exit(2).
> > If child call execnv(2) it replaces whole process, not only the thread.
> > If child call _exit(2) it stops while process, not only the thread.
> 
> Do you mean that's the current behavior in qemu?  That's not what clone(2) says.

Currently, qemu with NPTL(I've tested on ARM EABI) on CLONE_VM create
thread using pthread interface. Every thread has its own stack.

vfork() is clone() with flags CLONE_VM and CLONE_VFORK. 

man vfork(2):

   Linux Description
       vfork(),  just  like  fork(2), creates a child process of the calling
       process.  For details and return value and errors, see fork(2).

       vfork() is a special case of clone(2).  It is used to create new pro-
       cesses without copying the page tables of the parent process.  It may
       be useful in performance sensitive applications where a child will be
       created which then immediately issues an execve(2).

       vfork()  differs  from  fork(2) in that the parent is suspended until
       the child makes a call to execve(2) or _exit(2).   The  child  shares
       all  memory  with its parent, including the stack, until execve(2) is
       issued by the child.  The child must  not  return  from  the  current
       function or call exit(3), but may call _exit(2).

       Signal handlers are inherited, but not shared.  Signals to the parent
       arrive after the child releases the parent's memory.

So, implementation vfork() using pthread is wrong.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH] Fix vfork() syscall emulation

[andrzej zaborowski]

2008/9/20 Kirill A. Shutemov <kirill@shutemov.name>:
> So, implementation vfork() using pthread is wrong.

Agreed, but implementation of vfork() using fork() is wrong, too. If
we allow a hack, it should be commented, the second thing that needs
to be commented is why the value of CLONE_VM flag is ignored if
CLONE_VFORK is set -- on Linux it's not ignored.

Cheers

Re: [Qemu-devel] [PATCH] Fix vfork() syscall emulation

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]

On Sat, Sep 20, 2008 at 03:52:55PM +0200, andrzej zaborowski wrote:
> 2008/9/20 Kirill A. Shutemov <kirill@shutemov.name>:
> > So, implementation vfork() using pthread is wrong.
> 
> Agreed, but implementation of vfork() using fork() is wrong, too.

Why? 

man 2 vfork():

BUGS
       It  is  rather  unfortunate  that Linux revived this specter from the
       past.  The BSD man page states: "This system call will be  eliminated
       when  proper system sharing mechanisms are implemented.  Users should
       not depend on the memory sharing semantics of vfork() as it will,  in
       that case, be made synonymous to fork(2)."

If any program doesn't work with vfork() implemented using fork(). it's 
program bug.


> If
> we allow a hack, it should be commented, the second thing that needs
> to be commented is why the value of CLONE_VM flag is ignored if
> CLONE_VFORK is set -- on Linux it's not ignored.

vfork() is a hack itself. It was introduced when fork() was very expensive.
Linux fork() is implemented using copy-on-write pages, so the only penalty 
incurred by fork() is the time and memory required to duplicate the parent's
page tables. It's quite cheap. So I think emulate vfork() using fork() is 
correct.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH] Fix vfork() syscall emulation

[andrzej zaborowski]

2008/9/20 Kirill A. Shutemov <kirill@shutemov.name>:
> On Sat, Sep 20, 2008 at 03:52:55PM +0200, andrzej zaborowski wrote:
>> 2008/9/20 Kirill A. Shutemov <kirill@shutemov.name>:
>> > So, implementation vfork() using pthread is wrong.
>>
>> Agreed, but implementation of vfork() using fork() is wrong, too.
>
> Why?
>
> man 2 vfork():
>
> BUGS
>       It  is  rather  unfortunate  that Linux revived this specter from the
>       past.  The BSD man page states: "This system call will be  eliminated
>       when  proper system sharing mechanisms are implemented.  Users should
>       not depend on the memory sharing semantics of vfork() as it will,  in
>       that case, be made synonymous to fork(2)."
>
> If any program doesn't work with vfork() implemented using fork(). it's
> program bug.
>
>
>> If
>> we allow a hack, it should be commented, the second thing that needs
>> to be commented is why the value of CLONE_VM flag is ignored if
>> CLONE_VFORK is set -- on Linux it's not ignored.
>
> vfork() is a hack itself. It was introduced when fork() was very expensive.

Ok, perhaps I'm nit picking.  clone(2) specifies some semantics for
CLONE_VFORK regardless of the purpose and this implementation is
nowhere near these semantics.  I'll just add the same comment that
klibc has and push the patch.

Cheers

Re: [Qemu-devel] [PATCH] Fix vfork() syscall emulation

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

On Sat, Sep 20, 2008 at 04:35:25PM +0200, andrzej zaborowski wrote:
> 2008/9/20 Kirill A. Shutemov <kirill@shutemov.name>:
> > On Sat, Sep 20, 2008 at 03:52:55PM +0200, andrzej zaborowski wrote:
> >> 2008/9/20 Kirill A. Shutemov <kirill@shutemov.name>:
> >> > So, implementation vfork() using pthread is wrong.
> >>
> >> Agreed, but implementation of vfork() using fork() is wrong, too.
> >
> > Why?
> >
> > man 2 vfork():
> >
> > BUGS
> >       It  is  rather  unfortunate  that Linux revived this specter from the
> >       past.  The BSD man page states: "This system call will be  eliminated
> >       when  proper system sharing mechanisms are implemented.  Users should
> >       not depend on the memory sharing semantics of vfork() as it will,  in
> >       that case, be made synonymous to fork(2)."
> >
> > If any program doesn't work with vfork() implemented using fork(). it's
> > program bug.
> >
> >
> >> If
> >> we allow a hack, it should be commented, the second thing that needs
> >> to be commented is why the value of CLONE_VM flag is ignored if
> >> CLONE_VFORK is set -- on Linux it's not ignored.
> >
> > vfork() is a hack itself. It was introduced when fork() was very expensive.
> 
> Ok, perhaps I'm nit picking.  clone(2) specifies some semantics for
> CLONE_VFORK regardless of the purpose and this implementation is
> nowhere near these semantics.  I'll just add the same comment that
> klibc has and push the patch.

Thanks!

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH] Fix vfork() syscall emulation

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

On Sat, Sep 20, 2008 at 04:56:45AM +0200, andrzej zaborowski wrote:
> 2008/9/18 Kirill A. Shutemov <kirill@shutemov.name>:
> > vfork() is a kind of fork, not thread despite CLONE_VM
> 
> According to clone(2) it can be either, the only difference is that
> vfork() suspends the parent process.  So if CLONE_VM is set, I think
> still the pthread / clone way should be used and the child thread
> should be waited on.

Also process created with vfork(2) share stack with parent. Every thread
has its own stack.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]