Commit Message
Copied sysdeps/unix/sysv/linux/mips/bits/ipc.h to (new file)
sysdeps/unix/sysv/linux/aarch64/bits/ipc.h
Did not change any of the contents.
This results in the "mode" file in "ipc_perm" changing from 16bits to
32bits - which matches up with the kernel definition.
Note that all other 64bit BE targets have this set in their own ipc.h
Without this patch, on Aarch64 Big Endian the mode field would end up in
the padding field, resulting in the failure of most LTP semaphore tests.
Regards,
Alan.
Changelog:
2014-10-21 Alan Hayward <alan.hayward@arm.com>
* sysdeps/unix/sysv/linux/aarch64/bits/ipc.h: New file.
+ unsigned long int __glibc_reserved1;
+ unsigned long int __glibc_reserved2;
+};
Comments
On 21 October 2014 09:35, Alan Hayward <alan.hayward@arm.com> wrote:
> Copied sysdeps/unix/sysv/linux/mips/bits/ipc.h to (new file)
We are currently using sysdeps/unix/sysv/linux/bits/ipc.h, I'd prefer
we took a copy of that file and changed:
unsigned short int mode; /* Read/write permission. */
unsigned short int __pad1;
->
unsigned int mode; /* Read/write permission. */
Rather than take the other changes present in the mips file.
Cheers
/Marcus
On Tue, Oct 21, 2014 at 10:52:44AM +0100, Marcus Shawcroft wrote:
> On 21 October 2014 09:35, Alan Hayward <alan.hayward@arm.com> wrote:
> > Copied sysdeps/unix/sysv/linux/mips/bits/ipc.h to (new file)
>
> We are currently using sysdeps/unix/sysv/linux/bits/ipc.h, I'd prefer
> we took a copy of that file and changed:
>
> unsigned short int mode; /* Read/write permission. */
> unsigned short int __pad1;
> ->
> unsigned int mode; /* Read/write permission. */
>
> Rather than take the other changes present in the mips file.
Given that Marcus mentioned that all 64-bit BE Linux targets want
a 32-bit mode field, how about making a change to
sysdeps/unix/sysv/linux/bits/ipc.h so that it can be used as-is for all
these targets?
On 21/10/2014 11:31, "Christoph Hellwig" <hch@lst.de> wrote:
>On Tue, Oct 21, 2014 at 10:52:44AM +0100, Marcus Shawcroft wrote:
>> On 21 October 2014 09:35, Alan Hayward <alan.hayward@arm.com> wrote:
>> > Copied sysdeps/unix/sysv/linux/mips/bits/ipc.h to (new file)
>>
>> We are currently using sysdeps/unix/sysv/linux/bits/ipc.h, I'd prefer
>> we took a copy of that file and changed:
>>
>> unsigned short int mode; /* Read/write permission. */
>> unsigned short int __pad1;
>> ->
>> unsigned int mode; /* Read/write permission. */
>>
>> Rather than take the other changes present in the mips file.
>
>Given that Marcus mentioned that all 64-bit BE Linux targets want
>a 32-bit mode field, how about making a change to
>sysdeps/unix/sysv/linux/bits/ipc.h so that it can be used as-is for all
>these targets?
I did initially think about this, but each architecture seemed to have
subtly different version of the file, even though I think ultimately they
all end up with the same shaped structure. Also, I was shying away from
making a change where I was actively changing many different targets.
I can take a look into creating a unified version.
Alan.
On 21/10/2014 12:01, "Alan Hayward" <alan.hayward@arm.com> wrote:
>
>On 21/10/2014 11:31, "Christoph Hellwig" <hch@lst.de> wrote:
>
>>On Tue, Oct 21, 2014 at 10:52:44AM +0100, Marcus Shawcroft wrote:
>>> On 21 October 2014 09:35, Alan Hayward <alan.hayward@arm.com> wrote:
>>> > Copied sysdeps/unix/sysv/linux/mips/bits/ipc.h to (new file)
>>>
>>> We are currently using sysdeps/unix/sysv/linux/bits/ipc.h, I'd prefer
>>> we took a copy of that file and changed:
>>>
>>> unsigned short int mode; /* Read/write permission. */
>>> unsigned short int __pad1;
>>> ->
>>> unsigned int mode; /* Read/write permission. */
>>>
>>> Rather than take the other changes present in the mips file.
>>
>>Given that Marcus mentioned that all 64-bit BE Linux targets want
>>a 32-bit mode field, how about making a change to
>>sysdeps/unix/sysv/linux/bits/ipc.h so that it can be used as-is for all
>>these targets?
>
>I did initially think about this, but each architecture seemed to have
>subtly different version of the file, even though I think ultimately they
>all end up with the same shaped structure. Also, I was shying away from
>making a change where I was actively changing many different targets.
>
>I can take a look into creating a unified version.
>
Ok, looking into this a little further:
Power, ia64
*Has own file with __mode_t mode.
Hppa, sparc, s390
*Has own file. If 32bits, Short int mode else __mode_t mode
Mips, alpha
*Has own file with int mode.
*Also, additional fields use unsigned int instead of _uid_t/_gid_t etc
i386/x86_64/x86, arm, m68k, anything else:
*Use default file. Unsigned short int mode
Which breaks down to: 32bit targets use short int, 64bit targets use int.
Expect for the odd case out of x86_64, which always uses short int.
I think therefore we should probably:
1) Add a #if __WORDSIZE == 32 check into the generic ipc.h
2) Remove all specific ipc.h files
3) Add a specific x86_64 version for the odd case out
However, I would like to delay that change to a future patch, as it has
the potential to break in ways I might not immediately catch.
For this patch I would just make the changes Marcus suggested.
Does that make sense?
Alan.
On 10/21/2014 11:30 AM, Alan Hayward wrote:
> For this patch I would just make the changes Marcus suggested.
>
> Does that make sense?
Yes.
Though the cleanup and single ipc.h is where I'd like to see us go.
c.
On 24/10/2014 15:33, "Carlos O'Donell" <carlos@redhat.com> wrote:
>On 10/21/2014 11:30 AM, Alan Hayward wrote:
>> For this patch I would just make the changes Marcus suggested.
>>
>> Does that make sense?
>
>Yes.
>
>Though the cleanup and single ipc.h is where I'd like to see us go.
>
>c.
I’ve spent some more time investigating this, and I now no longer think we
can easily reduce the number of ipc.h files.
There were a couple of things I didn’t appreciate previously:
* There are 8 architectures which currently use the common file.
* We can’t just start switching between, say __uid_t and unsigned int,
even if in the general case for a particular architecture these types are
exactly the same size.
* A change in a file might not break any test cases, but it’s an ABI
change and could subtly break existing external applications that might
depend on exact structure orders and pad names.
* This will require regressions testing on all architectures, which is
going to very tricky.
I could only reasonably create a new common file if the structure was
identical for many architectures. But we do have subtle differences
between each of the 64bit targets.
i.e. - mips/alpha uses unsigned ints instead of typedefs; hppa has
different explicit named padding; power pc has extra defines at the end of
the file; the __WORDSIZE == 32 checks mean different architectures match
on 32bit than they do on 64bit
This results in no one common format between them. In the end the most
common version of the file is the one which currently common version. Even
if I added #if __WORDSIZE == 32 checks into the common file then I could
break existing 64bit architectures.
For the benefit of reducing a few files, this is going to carry a lot of
hidden risk.
As a reference, here they all are:
hppa-*-linux-gnu Not currently functional without patches.
struct ipc_perm
{
__key_t __key; /* Key. */
__uid_t uid; /* Owner's user ID. */
__gid_t gid; /* Owner's group ID. */
__uid_t cuid; /* Creator's user ID. */
__gid_t cgid; /* Creator's group ID. */
#if __WORDSIZE == 32
unsigned short int __pad1;
unsigned short int mode; /* Read/write permission. */
unsigned short int __pad2;
#else
__mode_t mode; /* Read/write permission. */
unsigned short int __pad2;
#endif
unsigned short int __seq; /* Sequence number. */
unsigned int __pad3;
__extension__ unsigned long long int __glibc_reserved1;
__extension__ unsigned long long int __glibc_reserved2;
};
ia64-*-linux-gnu
struct ipc_perm
{
__key_t __key; /* Key. */
__uid_t uid; /* Owner's user ID. */
__gid_t gid; /* Owner's group ID. */
__uid_t cuid; /* Creator's user ID. */
__gid_t cgid; /* Creator's group ID. */
__mode_t mode; /* Read/write permission. */
unsigned short int __seq; /* Sequence number. */
unsigned short int __pad1;
unsigned long int __glibc_reserved1;
unsigned long int __glibc_reserved2;
};
mips-*-linux-gnu
mips64-*-linux-gnu
alpha*-*-linux-gnu
struct 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. */
unsigned short int __pad1;
unsigned long int __glibc_reserved1;
unsigned long int __glibc_reserved2;
};
sparc*-*-linux-gnu
sparc64*-*-linux-gnu
struct ipc_perm
{
__key_t __key; /* Key. */
__uid_t uid; /* Owner's user ID. */
__gid_t gid; /* Owner's group ID. */
__uid_t cuid; /* Creator's user ID. */
__gid_t cgid; /* Creator's group ID. */
#if __WORDSIZE == 32
unsigned short int __pad1;
unsigned short int mode; /* Read/write permission. */
unsigned short int __pad2;
#else
__mode_t mode; /* Read/write permission. */
unsigned short int __pad1;
#endif
unsigned short int __seq; /* Sequence number. */
__extension__ unsigned long long int __glibc_reserved1;
__extension__ unsigned long long int __glibc_reserved2;
};
powerpc-*-linux-gnu
powerpc64*-*-linux-gnu
struct ipc_perm
{
__key_t __key; /* Key. */
__uid_t uid; /* Owner's user ID. */
__gid_t gid; /* Owner's group ID. */
__uid_t cuid; /* Creator's user ID. */
__gid_t cgid; /* Creator's group ID. */
__mode_t mode; /* Read/write permission. */
__uint32_t __seq; /* Sequence number. */
__uint32_t __pad1;
__uint64_t __glibc_reserved1;
__uint64_t __glibc_reserved2;
};
...also includes extra ipc and semop defines
s390-*-linux-gnu
s390x-*-linux-gnu
struct ipc_perm
{
__key_t __key; /* Key. */
__uid_t uid; /* Owner's user ID. */
__gid_t gid; /* Owner's group ID. */
__uid_t cuid; /* Creator's user ID. */
__gid_t cgid; /* Creator's group ID. */
#if __WORDSIZE == 64
__mode_t mode; /* Read/write permission. */
#else
unsigned short int mode; /* Read/write permission. */
unsigned short int __pad1;
#endif
unsigned short int __seq; /* Sequence number. */
unsigned short int __pad2;
unsigned long int __glibc_reserved1;
unsigned long int __glibc_reserved2;
};
aarch64*-*-linux-gnu
struct ipc_perm
{
__key_t __key; /* Key. */
__uid_t uid; /* Owner's user ID. */
__gid_t gid; /* Owner's group ID. */
__uid_t cuid; /* Creator's user ID. */
__gid_t cgid; /* Creator's group ID. */
unsigned int mode; /* Read/write permission. */
unsigned short int __seq; /* Sequence number. */
unsigned short int __pad1;
__syscall_ulong_t __glibc_reserved1;
__syscall_ulong_t __glibc_reserved2;
};
Common version
arm-*-linux-gnueabi
i[4567]86-*-linux-gnu
x86_64-*-linux-gnu Can build either x86_64 or x32
m68k-*-linux-gnu
microblaze*-*-linux-gnu
sh[34]-*-linux-gnu
tilegx-*-linux-gnu
tilepro-*-linux-gnu
struct ipc_perm
{
__key_t __key; /* Key. */
__uid_t uid; /* Owner's user ID. */
__gid_t gid; /* Owner's group ID. */
__uid_t cuid; /* Creator's user ID. */
__gid_t cgid; /* Creator's group ID. */
unsigned short int mode; /* Read/write permission. */
unsigned short int __pad1;
unsigned short int __seq; /* Sequence number. */
unsigned short int __pad2;
__syscall_ulong_t __glibc_reserved1;
__syscall_ulong_t __glibc_reserved2;
};
Alan.
On 11/10/2014 08:08 AM, Alan Hayward wrote:
>
>
> On 24/10/2014 15:33, "Carlos O'Donell" <carlos@redhat.com> wrote:
>
>> On 10/21/2014 11:30 AM, Alan Hayward wrote:
>>> For this patch I would just make the changes Marcus suggested.
>>>
>>> Does that make sense?
>>
>> Yes.
>>
>> Though the cleanup and single ipc.h is where I'd like to see us go.
>>
> For the benefit of reducing a few files, this is going to carry a lot of
> hidden risk.
That's a fine result to reach, but at least you attempted it
and I appreciate that. Thanks for going over the arch differences.
Cheers,
Carlos.
On 10 November 2014 13:08, Alan Hayward <alan.hayward@arm.com> wrote:
> I’ve spent some more time investigating this, and I now no longer think we
> can easily reduce the number of ipc.h files.
Alan, Thanks for looking into this. Your proposed patch is fine. The
inline patch doesn't apply cleanly, looks like the ARM email system
rewrote it. Can you send me the patch as an attachment and I'll apply
it.
Thanks
/Marcus
On 11/11/2014 15:45, "Marcus Shawcroft" <marcus.shawcroft@gmail.com> wrote:
>On 10 November 2014 13:08, Alan Hayward <alan.hayward@arm.com> wrote:
>
>> I’ve spent some more time investigating this, and I now no longer think
>>we
>> can easily reduce the number of ipc.h files.
>
>Alan, Thanks for looking into this. Your proposed patch is fine. The
>inline patch doesn't apply cleanly, looks like the ARM email system
>rewrote it. Can you send me the patch as an attachment and I'll apply
>it.
>Thanks
>/Marcus
Attached.
Thanks for applying.
Cheers,
Alan.
b/sysdeps/unix/sysv/linux/aarch64/bits/ipc.h
new file mode 100644
@@ -0,0 +1,54 @@
+/* Copyright (C) 1995-2014 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/>. */
+
+#ifndef _SYS_IPC_H
+# error "Never use <bits/ipc.h> directly; include <sys/ipc.h> instead."
+#endif
+
+#include <bits/types.h>
+
+/* Mode bits for `msgget', `semget', and `shmget'. */
+#define IPC_CREAT 01000 /* Create key if key does not
exist. */
+#define IPC_EXCL 02000 /* Fail if key exists. */
+#define IPC_NOWAIT 04000 /* Return error on wait. */
+
+/* Control commands for `msgctl', `semctl', and `shmctl'. */
+#define IPC_RMID 0 /* Remove identifier. */
+#define IPC_SET 1 /* Set `ipc_perm' options.
*/
+#define IPC_STAT 2 /* Get `ipc_perm' options. */
+#ifdef __USE_GNU
+# define IPC_INFO 3 /* See ipcs. */
+#endif
+
+/* Special key values. */
+#define IPC_PRIVATE ((__key_t) 0) /* Private key. */
+
+
+/* Data structure used to pass permission information to IPC operations.
*/
+struct 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. */
+ unsigned short int __pad1;