Message ID | 1481545990-7247-3-git-send-email-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
On Monday, December 12, 2016 10:32:55 AM CET Adhemerval Zanella wrote: > Since current IPC_64 default now on kernel is 0x100, some architectures > require it to 0 (for instance x86_64) while others continue to use a > non zero default regardless (powerpc). I find the explanation is a bit confusing, it's not that the kernel requires IPC_64 to be 0x100, it's that some architectures support the the old-style IPC and require IPC_64 to be passed, while new architectures should default to the new version. > index 0000000..1a31396 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h ... > + > +#include <sys/ipc.h> /* For __key_t */ > + > +#define __IPC_64 0x0 > + > +struct __old_ipc_perm > +{ > + __key_t __key; /* Key. */ > + unsigned int uid; /* Owner's user ID. */ > + unsigned int gid; /* Owner's group ID. */ > + unsigned int cuid; /* Creator's user ID. */ > + unsigned int cgid; /* Creator's group ID. */ > + unsigned int mode; /* Read/write permission. */ > + unsigned short int __seq; /* Sequence number. */ > +}; I don't understand this part at all: the #define line first says that the old IPC is not supported, but then you define the structure anyway? What is the structure actually used for in glibc? My interpretation is that it's only needed to implement the SHLIB_COMPAT version of the library calls that don't exist on ARM64 either. Arnd
On 12/12/2016 10:47, Arnd Bergmann wrote: > On Monday, December 12, 2016 10:32:55 AM CET Adhemerval Zanella wrote: > >> Since current IPC_64 default now on kernel is 0x100, some architectures >> require it to 0 (for instance x86_64) while others continue to use a >> non zero default regardless (powerpc). > > I find the explanation is a bit confusing, it's not that the kernel > requires IPC_64 to be 0x100, it's that some architectures support > the the old-style IPC and require IPC_64 to be passed, while new > architectures should default to the new version. Right, I did not want to expose some kernel internal implementation, but I think your explanation should be better. I will change to " Some architectures support the old-style IPC and require IPC_64 equal to 0x100 to be passed along SysV IPC syscalls, while new architectures should default to new IPC version (without the flags being set). This patch refactor current ipc_priv.h Linux headers in two directions: - Remove cross platform references (for instance alpha including powerpc definition) and add required definition for each required port. The idea is to avoid tie one architecture definition with another and make platform change independent. - Move all common definitions (the ipc syscall commands) on a common header, ipc_ops.h. " Do you think it is less confusing? > >> index 0000000..1a31396 >> --- /dev/null >> +++ b/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h > ... >> + >> +#include <sys/ipc.h> /* For __key_t */ >> + >> +#define __IPC_64 0x0 >> + >> +struct __old_ipc_perm >> +{ >> + __key_t __key; /* Key. */ >> + unsigned int uid; /* Owner's user ID. */ >> + unsigned int gid; /* Owner's group ID. */ >> + unsigned int cuid; /* Creator's user ID. */ >> + unsigned int cgid; /* Creator's group ID. */ >> + unsigned int mode; /* Read/write permission. */ >> + unsigned short int __seq; /* Sequence number. */ >> +}; > > I don't understand this part at all: the #define line first says > that the old IPC is not supported, but then you define the structure > anyway? > > What is the structure actually used for in glibc? My interpretation > is that it's only needed to implement the SHLIB_COMPAT version > of the library calls that don't exist on ARM64 either. You are right and it is mainly a limitation on how my code defines the 'union semun' in semctl.c. I will change it to use an opaque type instead of a pointer to __old_ipc_perm.
On Dez 12 2016, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > - Move all common definitions (the ipc syscall commands) on a common > header, ipc_ops.h. Shouldn't that be in bits/? Andreas.
On Monday, December 12, 2016 10:57:51 AM CET Adhemerval Zanella wrote: > > On 12/12/2016 10:47, Arnd Bergmann wrote: > > On Monday, December 12, 2016 10:32:55 AM CET Adhemerval Zanella wrote: > > > >> Since current IPC_64 default now on kernel is 0x100, some architectures > >> require it to 0 (for instance x86_64) while others continue to use a > >> non zero default regardless (powerpc). > > > > I find the explanation is a bit confusing, it's not that the kernel > > requires IPC_64 to be 0x100, it's that some architectures support > > the the old-style IPC and require IPC_64 to be passed, while new > > architectures should default to the new version. > > Right, I did not want to expose some kernel internal implementation, > but I think your explanation should be better. I will change to > > " > Some architectures support the old-style IPC and require IPC_64 > equal to 0x100 to be passed along SysV IPC syscalls, while new > architectures should default to new IPC version (without the > flags being set). > > This patch refactor current ipc_priv.h Linux headers in two directions: > > - Remove cross platform references (for instance alpha including powerpc > definition) and add required definition for each required port. The > idea is to avoid tie one architecture definition with another and make > platform change independent. > > - Move all common definitions (the ipc syscall commands) on a common > header, ipc_ops.h. > " > > Do you think it is less confusing? Sounds good, thanks! > > > >> index 0000000..1a31396 > >> --- /dev/null > >> +++ b/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h > > ... > >> + > >> +#include <sys/ipc.h> /* For __key_t */ > >> + > >> +#define __IPC_64 0x0 > >> + > >> +struct __old_ipc_perm > >> +{ > >> + __key_t __key; /* Key. */ > >> + unsigned int uid; /* Owner's user ID. */ > >> + unsigned int gid; /* Owner's group ID. */ > >> + unsigned int cuid; /* Creator's user ID. */ > >> + unsigned int cgid; /* Creator's group ID. */ > >> + unsigned int mode; /* Read/write permission. */ > >> + unsigned short int __seq; /* Sequence number. */ > >> +}; > > > > I don't understand this part at all: the #define line first says > > that the old IPC is not supported, but then you define the structure > > anyway? > > > > What is the structure actually used for in glibc? My interpretation > > is that it's only needed to implement the SHLIB_COMPAT version > > of the library calls that don't exist on ARM64 either. > > You are right and it is mainly a limitation on how my code defines the > 'union semun' in semctl.c. I will change it to use an opaque type > instead of a pointer to __old_ipc_perm. I guess we can even remove the entire definition of struct __old_semid_ds etc on all architectures since we just pass a pointer from the compat entry point to the kernel without looking at the contents? Arnd
On 12/12/2016 11:05, Andreas Schwab wrote: > On Dez 12 2016, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > >> - Move all common definitions (the ipc syscall commands) on a common >> header, ipc_ops.h. > > Shouldn't that be in bits/? My understanding is IPCOP_* definitions are used only internally for IPC calls, users should use the sysv exported symbol instead of using syscall plus IPCOP_*.
On Dez 12 2016, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > On 12/12/2016 11:05, Andreas Schwab wrote: >> On Dez 12 2016, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: >> >>> - Move all common definitions (the ipc syscall commands) on a common >>> header, ipc_ops.h. >> >> Shouldn't that be in bits/? > > My understanding is IPCOP_* definitions are used only internally for > IPC calls, users should use the sysv exported symbol instead of > using syscall plus IPCOP_*. Yes, I misunderstood. Andreas.
On 12/12/2016 11:25, Arnd Bergmann wrote: > On Monday, December 12, 2016 10:57:51 AM CET Adhemerval Zanella wrote: >> >> On 12/12/2016 10:47, Arnd Bergmann wrote: >>> On Monday, December 12, 2016 10:32:55 AM CET Adhemerval Zanella wrote: >>> >>>> Since current IPC_64 default now on kernel is 0x100, some architectures >>>> require it to 0 (for instance x86_64) while others continue to use a >>>> non zero default regardless (powerpc). >>> >>> I find the explanation is a bit confusing, it's not that the kernel >>> requires IPC_64 to be 0x100, it's that some architectures support >>> the the old-style IPC and require IPC_64 to be passed, while new >>> architectures should default to the new version. >> >> Right, I did not want to expose some kernel internal implementation, >> but I think your explanation should be better. I will change to >> >> " >> Some architectures support the old-style IPC and require IPC_64 >> equal to 0x100 to be passed along SysV IPC syscalls, while new >> architectures should default to new IPC version (without the >> flags being set). >> >> This patch refactor current ipc_priv.h Linux headers in two directions: >> >> - Remove cross platform references (for instance alpha including powerpc >> definition) and add required definition for each required port. The >> idea is to avoid tie one architecture definition with another and make >> platform change independent. >> >> - Move all common definitions (the ipc syscall commands) on a common >> header, ipc_ops.h. >> " >> >> Do you think it is less confusing? > > Sounds good, thanks! > >>> >>>> index 0000000..1a31396 >>>> --- /dev/null >>>> +++ b/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h >>> ... >>>> + >>>> +#include <sys/ipc.h> /* For __key_t */ >>>> + >>>> +#define __IPC_64 0x0 >>>> + >>>> +struct __old_ipc_perm >>>> +{ >>>> + __key_t __key; /* Key. */ >>>> + unsigned int uid; /* Owner's user ID. */ >>>> + unsigned int gid; /* Owner's group ID. */ >>>> + unsigned int cuid; /* Creator's user ID. */ >>>> + unsigned int cgid; /* Creator's group ID. */ >>>> + unsigned int mode; /* Read/write permission. */ >>>> + unsigned short int __seq; /* Sequence number. */ >>>> +}; >>> >>> I don't understand this part at all: the #define line first says >>> that the old IPC is not supported, but then you define the structure >>> anyway? >>> >>> What is the structure actually used for in glibc? My interpretation >>> is that it's only needed to implement the SHLIB_COMPAT version >>> of the library calls that don't exist on ARM64 either. >> >> You are right and it is mainly a limitation on how my code defines the >> 'union semun' in semctl.c. I will change it to use an opaque type >> instead of a pointer to __old_ipc_perm. > > I guess we can even remove the entire definition of > struct __old_semid_ds etc on all architectures since we > just pass a pointer from the compat entry point to the > kernel without looking at the contents? Yeap, this is what I ended up doing in my local branch (remove aarch64 ipc_priv.h ununsed definitions and cleanup semctl old ipc struct definitions).
diff --git a/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h b/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h new file mode 100644 index 0000000..1a31396 --- /dev/null +++ b/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h @@ -0,0 +1,32 @@ +/* Old SysV permission definition for Linux. AArch64 version. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library 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. + + The GNU C Library 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 the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <sys/ipc.h> /* For __key_t */ + +#define __IPC_64 0x0 + +struct __old_ipc_perm +{ + __key_t __key; /* Key. */ + unsigned int uid; /* Owner's user ID. */ + unsigned int gid; /* Owner's group ID. */ + unsigned int cuid; /* Creator's user ID. */ + unsigned int cgid; /* Creator's group ID. */ + unsigned int mode; /* Read/write permission. */ + unsigned short int __seq; /* Sequence number. */ +}; diff --git a/sysdeps/unix/sysv/linux/alpha/ipc_priv.h b/sysdeps/unix/sysv/linux/alpha/ipc_priv.h index 67883be..06444fc 100644 --- a/sysdeps/unix/sysv/linux/alpha/ipc_priv.h +++ b/sysdeps/unix/sysv/linux/alpha/ipc_priv.h @@ -1 +1,32 @@ -#include <sysdeps/unix/sysv/linux/powerpc/ipc_priv.h> +/* Old SysV permission definition for Linux. Alpha version. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library 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. + + The GNU C Library 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 the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <sys/ipc.h> /* For __key_t */ + +#define __IPC_64 0x100 + +struct __old_ipc_perm +{ + __key_t __key; /* Key. */ + unsigned int uid; /* Owner's user ID. */ + unsigned int gid; /* Owner's group ID. */ + unsigned int cuid; /* Creator's user ID. */ + unsigned int cgid; /* Creator's group ID. */ + unsigned int mode; /* Read/write permission. */ + unsigned short int __seq; /* Sequence number. */ +}; diff --git a/sysdeps/unix/sysv/linux/ipc_ops.h b/sysdeps/unix/sysv/linux/ipc_ops.h new file mode 100644 index 0000000..ea3028c --- /dev/null +++ b/sysdeps/unix/sysv/linux/ipc_ops.h @@ -0,0 +1,30 @@ +/* The codes for the functions to use the ipc syscall multiplexer. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library 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. + + The GNU C Library 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 the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#define IPCOP_semop 1 +#define IPCOP_semget 2 +#define IPCOP_semctl 3 +#define IPCOP_semtimedop 4 +#define IPCOP_msgsnd 11 +#define IPCOP_msgrcv 12 +#define IPCOP_msgget 13 +#define IPCOP_msgctl 14 +#define IPCOP_shmat 21 +#define IPCOP_shmdt 22 +#define IPCOP_shmget 23 +#define IPCOP_shmctl 24 diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h index 7ded463..9b97f00 100644 --- a/sysdeps/unix/sysv/linux/ipc_priv.h +++ b/sysdeps/unix/sysv/linux/ipc_priv.h @@ -1,4 +1,5 @@ -/* Copyright (C) 1995-2016 Free Software Foundation, Inc. +/* Old SysV permission definition for Linux. Default version. + Copyright (C) 1995-2016 Free Software Foundation, Inc. This file is part of the GNU C Library. The GNU C Library is free software; you can redistribute it and/or @@ -15,7 +16,7 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -#include <sys/ipc.h> +#include <sys/ipc.h> /* For __key_t */ #define __IPC_64 0x100 @@ -30,17 +31,9 @@ struct __old_ipc_perm unsigned short int __seq; /* Sequence number. */ }; +#define SEMCTL_ARG_ADDRESS(__arg) &__arg.array -/* The codes for the functions to use the ipc syscall multiplexer. */ -#define IPCOP_semop 1 -#define IPCOP_semget 2 -#define IPCOP_semctl 3 -#define IPCOP_semtimedop 4 -#define IPCOP_msgsnd 11 -#define IPCOP_msgrcv 12 -#define IPCOP_msgget 13 -#define IPCOP_msgctl 14 -#define IPCOP_shmat 21 -#define IPCOP_shmdt 22 -#define IPCOP_shmget 23 -#define IPCOP_shmctl 24 +#define MSGRCV_ARGS(__msgp, __msgtyp) \ + ((long int []){ (long int) __msgp, __msgtyp }) + +#include <ipc_ops.h> diff --git a/sysdeps/unix/sysv/linux/mips/ipc_priv.h b/sysdeps/unix/sysv/linux/mips/ipc_priv.h deleted file mode 100644 index 67883be..0000000 --- a/sysdeps/unix/sysv/linux/mips/ipc_priv.h +++ /dev/null @@ -1 +0,0 @@ -#include <sysdeps/unix/sysv/linux/powerpc/ipc_priv.h> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/ipc_priv.h b/sysdeps/unix/sysv/linux/mips/mips64/ipc_priv.h new file mode 100644 index 0000000..9f47d89 --- /dev/null +++ b/sysdeps/unix/sysv/linux/mips/mips64/ipc_priv.h @@ -0,0 +1,32 @@ +/* Old SysV permission definition for Linux. MIPS64 version. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library 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. + + The GNU C Library 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 the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <sys/ipc.h> + +#define __IPC_64 0x100 + +struct __old_ipc_perm +{ + __key_t __key; /* Key. */ + int uid; /* Owner's user ID. */ + int gid; /* Owner's group ID. */ + int cuid; /* Creator's user ID. */ + int cgid; /* Creator's group ID. */ + int mode; /* Read/write permission. */ + unsigned short int __seq; /* Sequence number. */ +}; diff --git a/sysdeps/unix/sysv/linux/powerpc/ipc_priv.h b/sysdeps/unix/sysv/linux/powerpc/ipc_priv.h index baae7ab..4f72f58 100644 --- a/sysdeps/unix/sysv/linux/powerpc/ipc_priv.h +++ b/sysdeps/unix/sysv/linux/powerpc/ipc_priv.h @@ -1,4 +1,5 @@ -/* Copyright (C) 1995-2016 Free Software Foundation, Inc. +/* Old SysV permission definition for Linux. PowerPC version. + Copyright (C) 1995-2016 Free Software Foundation, Inc. This file is part of the GNU C Library. The GNU C Library is free software; you can redistribute it and/or @@ -15,7 +16,7 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -#include <sys/ipc.h> +#include <sys/ipc.h> /* For __key_t */ #define __IPC_64 0x100 @@ -30,17 +31,9 @@ struct __old_ipc_perm unsigned short int __seq; /* Sequence number. */ }; +#define SEMCTL_ARG_ADDRESS(__arg) &__arg.array -/* The codes for the functions to use the ipc syscall multiplexer. */ -#define IPCOP_semop 1 -#define IPCOP_semget 2 -#define IPCOP_semctl 3 -#define IPCOP_semtimedop 4 -#define IPCOP_msgsnd 11 -#define IPCOP_msgrcv 12 -#define IPCOP_msgget 13 -#define IPCOP_msgctl 14 -#define IPCOP_shmat 21 -#define IPCOP_shmdt 22 -#define IPCOP_shmget 23 -#define IPCOP_shmctl 24 +#define MSGRCV_ARGS(__msgp, __msgtyp) \ + ((long int []){ (long int) __msgp, __msgtyp }) + +#include <ipc_ops.h> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/ipc_priv.h b/sysdeps/unix/sysv/linux/sparc/sparc64/ipc_priv.h new file mode 100644 index 0000000..386ff8a --- /dev/null +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/ipc_priv.h @@ -0,0 +1,41 @@ +/* Old SysV permission definition for Linux. x86_64 version. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library 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. + + The GNU C Library 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 the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <sys/ipc.h> /* For __key_t */ + +#define __IPC_64 0x0 + +struct __old_ipc_perm +{ + __key_t __key; /* Key. */ + unsigned int uid; /* Owner's user ID. */ + unsigned int gid; /* Owner's group ID. */ + unsigned int cuid; /* Creator's user ID. */ + unsigned int cgid; /* Creator's group ID. */ + unsigned int mode; /* Read/write permission. */ + unsigned short int __seq; /* Sequence number. */ +}; + +/* SPARC semctl multiplex syscall expects the union pointed address, not + the union address itself. */ +#define SEMCTL_ARG_ADDRESS(__arg) __arg.array + +/* Also for msgrcv it does not use the kludge on final 2 arguments. */ +#define MSGRCV_ARGS(__msgp, __msgtyp) __msgp, __msgtyp + +#include <ipc_ops.h> diff --git a/sysdeps/unix/sysv/linux/x86_64/ipc_priv.h b/sysdeps/unix/sysv/linux/x86_64/ipc_priv.h new file mode 100644 index 0000000..d39db53 --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86_64/ipc_priv.h @@ -0,0 +1,32 @@ +/* Old SysV permission definition for Linux. x86_64 version. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library 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. + + The GNU C Library 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 the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <sys/ipc.h> /* For __key_t */ + +#define __IPC_64 0x0 + +struct __old_ipc_perm +{ + __key_t __key; /* Key. */ + unsigned short uid; /* Owner's user ID. */ + unsigned short gid; /* Owner's group ID. */ + unsigned short cuid; /* Creator's user ID. */ + unsigned short cgid; /* Creator's group ID. */ + unsigned short mode; /* Read/write permission. */ + unsigned short int __seq; /* Sequence number. */ +};