* struct statfs/statfs64 in linux-generic @ 2013-11-04 7:18 Chung-Lin Tang 2013-11-04 8:19 ` Andrew Pinski 2013-11-07 19:55 ` Chris Metcalf 0 siblings, 2 replies; 15+ messages in thread From: Chung-Lin Tang @ 2013-11-04 7:18 UTC (permalink / raw) To: libc-ports; +Cc: Chris Metcalf Hi, I'm currently working on the glibc port for Altera Nios II, and per the kernel upstreaming requirements, we're making it a user of linux-generic in glibc. I've come across an issue about struct statfs/statfs64, where I've listed the struct definitions in both glibc and the kernel below. In linux-generic, both struct statfs/statfs64 has some fields as 64-bit words, padded properly if need on 32-bit targets. Effectively, they have to be same-format, to pass to the statfs64 syscall uniformly. __SWORD_TYPE appears to be int/long on 32/64-bit targets. This means that for 32-bit targets, struct statfs[64] will be made of 32-bit words, with a few 64-bit fields embedded in the middle. In the kernel however, the fields of importance in struct statfs is made up of entirely '__statfs_word', defined to be 64-bit or 32-bit depending on target (putting aside signedness for now). Notice how that, while this does work for 64-bit targets, 32-bit targets cannot properly define a compatible struct statfs[64], at least not with simple typedefs. Chris, as you seem the original developer/maintainer of linux-generic and the tile ports (as I observed from the mail archives), I'm curious if you saw any similar problems with tilegx -m32? The only users of linux-generic appears to be only aarch64 and tile, and nios2 will be another 32-bit user. The exact FAIL I saw was posix/tst-getconf, which I traced to a failed statfs64 call in pathconf(), due to a failed sizeof(struct statfs64) check inside the kernel syscall code. Thanks, Chung-Lin In glibc, ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h: struct statfs { __SWORD_TYPE f_type; __SWORD_TYPE f_bsize; __field64(__fsblkcnt_t, __fsblkcnt64_t, f_blocks); __field64(__fsblkcnt_t, __fsblkcnt64_t, f_bfree); __field64(__fsblkcnt_t, __fsblkcnt64_t, f_bavail); __field64(__fsfilcnt_t, __fsfilcnt64_t, f_files); __field64(__fsfilcnt_t, __fsfilcnt64_t, f_ffree); __fsid_t f_fsid; __SWORD_TYPE f_namelen; __SWORD_TYPE f_frsize; __SWORD_TYPE f_flags; __SWORD_TYPE f_spare[4]; } __attribute__((__aligned__(8))); #ifdef __USE_LARGEFILE64 struct statfs64 { __SWORD_TYPE f_type; __SWORD_TYPE f_bsize; __fsblkcnt64_t f_blocks; __fsblkcnt64_t f_bfree; __fsblkcnt64_t f_bavail; __fsfilcnt64_t f_files; __fsfilcnt64_t f_ffree; __fsid_t f_fsid; __SWORD_TYPE f_namelen; __SWORD_TYPE f_frsize; __SWORD_TYPE f_flags; __SWORD_TYPE f_spare[4]; } __attribute__((__aligned__(8))); #endif In Linux kernel, include/uapi/asm-generic/statfs.h: #ifndef __statfs_word #if __BITS_PER_LONG == 64 #define __statfs_word long #else #define __statfs_word __u32 #endif #endif struct statfs { __statfs_word f_type; __statfs_word f_bsize; __statfs_word f_blocks; __statfs_word f_bfree; __statfs_word f_bavail; __statfs_word f_files; __statfs_word f_ffree; __kernel_fsid_t f_fsid; __statfs_word f_namelen; __statfs_word f_frsize; __statfs_word f_flags; __statfs_word f_spare[4]; }; #ifndef ARCH_PACK_STATFS64 #define ARCH_PACK_STATFS64 #endif struct statfs64 { __statfs_word f_type; __statfs_word f_bsize; __u64 f_blocks; __u64 f_bfree; __u64 f_bavail; __u64 f_files; __u64 f_ffree; __kernel_fsid_t f_fsid; __statfs_word f_namelen; __statfs_word f_frsize; __statfs_word f_flags; __statfs_word f_spare[4]; } ARCH_PACK_STATFS64; ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-04 7:18 struct statfs/statfs64 in linux-generic Chung-Lin Tang @ 2013-11-04 8:19 ` Andrew Pinski 2013-11-05 4:23 ` Chung-Lin Tang 2013-11-07 19:55 ` Chris Metcalf 1 sibling, 1 reply; 15+ messages in thread From: Andrew Pinski @ 2013-11-04 8:19 UTC (permalink / raw) To: Chung-Lin Tang; +Cc: libc-ports, Chris Metcalf [-- Attachment #1: Type: text/plain, Size: 3993 bytes --] On Sun, Nov 3, 2013 at 11:18 PM, Chung-Lin Tang <cltang@codesourcery.com> wrote: > Hi, > > I'm currently working on the glibc port for Altera Nios II, and per the > kernel upstreaming requirements, we're making it a user of linux-generic > in glibc. > > I've come across an issue about struct statfs/statfs64, where I've > listed the struct definitions in both glibc and the kernel below. > > In linux-generic, both struct statfs/statfs64 has some fields as 64-bit > words, padded properly if need on 32-bit targets. Effectively, they have > to be same-format, to pass to the statfs64 syscall uniformly. > > __SWORD_TYPE appears to be int/long on 32/64-bit targets. This means > that for 32-bit targets, struct statfs[64] will be made of 32-bit words, > with a few 64-bit fields embedded in the middle. > > > In the kernel however, the fields of importance in struct statfs is made > up of entirely '__statfs_word', defined to be 64-bit or 32-bit depending > on target (putting aside signedness for now). > > Notice how that, while this does work for 64-bit targets, 32-bit targets > cannot properly define a compatible struct statfs[64], at least not with > simple typedefs. > > Chris, as you seem the original developer/maintainer of linux-generic > and the tile ports (as I observed from the mail archives), I'm curious > if you saw any similar problems with tilegx -m32? The only users of > linux-generic appears to be only aarch64 and tile, and nios2 will be > another 32-bit user. > > The exact FAIL I saw was posix/tst-getconf, which I traced to a failed > statfs64 call in pathconf(), due to a failed sizeof(struct statfs64) > check inside the kernel syscall code. Yes I ran into this same issue when I was working on ILP32 for AARCH64. Attached is the patch which I used. Thanks, Andrew Pinski > > Thanks, > Chung-Lin > > > In glibc, ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h: > struct statfs > { > __SWORD_TYPE f_type; > __SWORD_TYPE f_bsize; > __field64(__fsblkcnt_t, __fsblkcnt64_t, f_blocks); > __field64(__fsblkcnt_t, __fsblkcnt64_t, f_bfree); > __field64(__fsblkcnt_t, __fsblkcnt64_t, f_bavail); > __field64(__fsfilcnt_t, __fsfilcnt64_t, f_files); > __field64(__fsfilcnt_t, __fsfilcnt64_t, f_ffree); > __fsid_t f_fsid; > __SWORD_TYPE f_namelen; > __SWORD_TYPE f_frsize; > __SWORD_TYPE f_flags; > __SWORD_TYPE f_spare[4]; > } __attribute__((__aligned__(8))); > > #ifdef __USE_LARGEFILE64 > struct statfs64 > { > __SWORD_TYPE f_type; > __SWORD_TYPE f_bsize; > __fsblkcnt64_t f_blocks; > __fsblkcnt64_t f_bfree; > __fsblkcnt64_t f_bavail; > __fsfilcnt64_t f_files; > __fsfilcnt64_t f_ffree; > __fsid_t f_fsid; > __SWORD_TYPE f_namelen; > __SWORD_TYPE f_frsize; > __SWORD_TYPE f_flags; > __SWORD_TYPE f_spare[4]; > } __attribute__((__aligned__(8))); > #endif > > > In Linux kernel, include/uapi/asm-generic/statfs.h: > > #ifndef __statfs_word > #if __BITS_PER_LONG == 64 > #define __statfs_word long > #else > #define __statfs_word __u32 > #endif > #endif > > struct statfs { > __statfs_word f_type; > __statfs_word f_bsize; > __statfs_word f_blocks; > __statfs_word f_bfree; > __statfs_word f_bavail; > __statfs_word f_files; > __statfs_word f_ffree; > __kernel_fsid_t f_fsid; > __statfs_word f_namelen; > __statfs_word f_frsize; > __statfs_word f_flags; > __statfs_word f_spare[4]; > }; > > #ifndef ARCH_PACK_STATFS64 > #define ARCH_PACK_STATFS64 > #endif > > struct statfs64 { > __statfs_word f_type; > __statfs_word f_bsize; > __u64 f_blocks; > __u64 f_bfree; > __u64 f_bavail; > __u64 f_files; > __u64 f_ffree; > __kernel_fsid_t f_fsid; > __statfs_word f_namelen; > __statfs_word f_frsize; > __statfs_word f_flags; > __statfs_word f_spare[4]; > } ARCH_PACK_STATFS64; [-- Attachment #2: fixstatfs.diff.txt --] [-- Type: text/plain, Size: 3244 bytes --] commit 82c1e9bd5e105de7a97d29f07148da81a0349d3e Author: Andrew Pinski <apinski@cavium.com> Date: Sun Aug 11 13:16:02 2013 -0700 2013-08-11 Andrew Pinski <apinski@cavium.com> * ports/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h (__FSWORD_T_TYPE): Change to be __SYSCALL_SLONG_TYPE. * ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h (statfs): Update to use __fsword_t. (statfs64): Likewise. diff --git a/ChangeLog.aarch64 b/ChangeLog.aarch64 index 920fb0f..bc44a2a 100644 --- a/ChangeLog.aarch64 +++ b/ChangeLog.aarch64 @@ -1,3 +1,10 @@ +2013-08-11 Andrew Pinski <apinski@cavium.com> + + * ports/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h (__FSWORD_T_TYPE): Change to + be __SYSCALL_SLONG_TYPE. + * ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h (statfs): Update to use __fsword_t. + (statfs64): Likewise. + 2013-08-10 Andrew Pinski <apinski@cavium.com> * ports/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h (__TIME_T_64_BITS): New define. diff --git a/ports/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/ports/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h index 74576ab..f3a51e8 100644 --- a/ports/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h +++ b/ports/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h @@ -53,7 +53,7 @@ #define __FSBLKCNT64_T_TYPE __UQUAD_TYPE #define __FSFILCNT_T_TYPE __ULONGWORD_TYPE #define __FSFILCNT64_T_TYPE __UQUAD_TYPE -#define __FSWORD_T_TYPE __SWORD_TYPE +#define __FSWORD_T_TYPE __SYSCALL_SLONG_TYPE #define __ID_T_TYPE __U32_TYPE #define __CLOCK_T_TYPE __SYSCALL_SLONG_TYPE #define __TIME_T_TYPE __SYSCALL_SLONG_TYPE diff --git a/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h b/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h index bf479e8..afadbf1 100644 --- a/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h +++ b/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h @@ -46,18 +46,18 @@ struct statfs { - __SWORD_TYPE f_type; - __SWORD_TYPE f_bsize; + __fsword_t f_type; + __fsword_t f_bsize; __field64(__fsblkcnt_t, __fsblkcnt64_t, f_blocks); __field64(__fsblkcnt_t, __fsblkcnt64_t, f_bfree); __field64(__fsblkcnt_t, __fsblkcnt64_t, f_bavail); __field64(__fsfilcnt_t, __fsfilcnt64_t, f_files); __field64(__fsfilcnt_t, __fsfilcnt64_t, f_ffree); __fsid_t f_fsid; - __SWORD_TYPE f_namelen; - __SWORD_TYPE f_frsize; - __SWORD_TYPE f_flags; - __SWORD_TYPE f_spare[4]; + __fsword_t f_namelen; + __fsword_t f_frsize; + __fsword_t f_flags; + __fsword_t f_spare[4]; } __attribute__((__aligned__(8))); #undef __field64 @@ -65,18 +65,18 @@ struct statfs #ifdef __USE_LARGEFILE64 struct statfs64 { - __SWORD_TYPE f_type; - __SWORD_TYPE f_bsize; + __fsword_t f_type; + __fsword_t f_bsize; __fsblkcnt64_t f_blocks; __fsblkcnt64_t f_bfree; __fsblkcnt64_t f_bavail; __fsfilcnt64_t f_files; __fsfilcnt64_t f_ffree; __fsid_t f_fsid; - __SWORD_TYPE f_namelen; - __SWORD_TYPE f_frsize; - __SWORD_TYPE f_flags; - __SWORD_TYPE f_spare[4]; + __fsword_t f_namelen; + __fsword_t f_frsize; + __fsword_t f_flags; + __fsword_t f_spare[4]; } __attribute__((__aligned__(8))); #endif ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-04 8:19 ` Andrew Pinski @ 2013-11-05 4:23 ` Chung-Lin Tang 0 siblings, 0 replies; 15+ messages in thread From: Chung-Lin Tang @ 2013-11-05 4:23 UTC (permalink / raw) To: Andrew Pinski; +Cc: libc-ports, Chris Metcalf On 2013/11/4 04:19 PM, Andrew Pinski wrote: > --- a/ports/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h > +++ b/ports/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h > @@ -53,7 +53,7 @@ > #define __FSBLKCNT64_T_TYPE __UQUAD_TYPE > #define __FSFILCNT_T_TYPE __ULONGWORD_TYPE > #define __FSFILCNT64_T_TYPE __UQUAD_TYPE > -#define __FSWORD_T_TYPE __SWORD_TYPE > +#define __FSWORD_T_TYPE __SYSCALL_SLONG_TYPE > #define __ID_T_TYPE __U32_TYPE > #define __CLOCK_T_TYPE __SYSCALL_SLONG_TYPE > #define __TIME_T_TYPE __SYSCALL_SLONG_TYPE I'm guessing you re-defined __SYSCALL_SLONG_TYPE to a 64-bit signed type as well? The default seems to be __SLONGWORD_TYPE, which is a 4-byte long int. Still, I'm not sure if using full 64-bit fields should be the intended solution. A full 32-bit target should be able to have those fields as 32-bit (which means a kernel patch to differentiate the current uses of '__statfs_word' into 32/64-bit words) Chung-Lin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-04 7:18 struct statfs/statfs64 in linux-generic Chung-Lin Tang 2013-11-04 8:19 ` Andrew Pinski @ 2013-11-07 19:55 ` Chris Metcalf 2013-11-10 8:24 ` Chung-Lin Tang 1 sibling, 1 reply; 15+ messages in thread From: Chris Metcalf @ 2013-11-07 19:55 UTC (permalink / raw) To: Chung-Lin Tang, libc-ports, Andrew Pinski On 11/4/2013 2:18 AM, Chung-Lin Tang wrote: > Hi, > > I'm currently working on the glibc port for Altera Nios II, and per the > kernel upstreaming requirements, we're making it a user of linux-generic > in glibc. > > I've come across an issue about struct statfs/statfs64, where I've > listed the struct definitions in both glibc and the kernel below. > > In linux-generic, both struct statfs/statfs64 has some fields as 64-bit > words, padded properly if need on 32-bit targets. Effectively, they have > to be same-format, to pass to the statfs64 syscall uniformly. > > __SWORD_TYPE appears to be int/long on 32/64-bit targets. This means > that for 32-bit targets, struct statfs[64] will be made of 32-bit words, > with a few 64-bit fields embedded in the middle. > > > In the kernel however, the fields of importance in struct statfs is made > up of entirely '__statfs_word', defined to be 64-bit or 32-bit depending > on target (putting aside signedness for now). I have to carefully swap all this stuff back into my head every time I look at this issue :-) For native 32-bit (for us that means tilepro) the constraint is that the kernel's "struct statfs64" has to match the two structs in glibc's <bits/statfs.h>. Most of the statfs types are fixed-size (fsid_t and the __u64 types), with __statfs_word/__SWORD_TYPE being the exception. For tilepro both of those kernel and glibc types are 32-bit types. For 32-bit userspace in a 64-bit kernel (for us that means tilegx -m32) the constraint is that the kernel's "struct compat_statfs64" match the <bits/statfs.h> types, which again, it seems that they should. (Note that tilegx -m32 is explicitly a "compat" syscall model.) A quick look suggests that nios2 is a pure 32-bit architecture, so more like the tilepro model. Andrew Pinski's email was a bit confusing since __SYSCALL_SLONG_TYPE seems to be the same size as __SWORD_TYPE, and I don't see any redefinitions of it for aarch64. I believe his usecase is the 32-bit userspace on 64-bit kernel model. One thing to watch out for is that some architectures (like x32) actually use the 64-bit syscalls natively (no compat_statfs64) so they need to use "long long" types to match the 64-bit fields, etc., so it's a different problem. In retrospect that might have been a cleaner model for tilegx -m32, but I suspect that ship has sailed for us. In your followup email you write: > Still, I'm not sure if using full 64-bit fields should be the intended > solution. A full 32-bit target should be able to have those fields as > 32-bit (which means a kernel patch to differentiate the current uses of > '__statfs_word' into 32/64-bit words) I have a guess as to what the problem you're seeing is: are you looking at "struct statfs" in the kernel? That structure is completely unused on 32-bit platforms, since it's only relevant for the 64-bit platform's sys_statfs/sys_fstatfs syscalls, whereas on 32-bit platforms you only get sys_statfs64/sys_fstatfs64. In other words there is no syscall that fills in a structure with 32-bit "f_blocks" (for example). See the __NR3264_statfs macro in <asm-generic/unistd.h> and how it resolves with the __SC_COMP_3264() macro, to the __SC_3264() macro, to the 32-bit version of the syscall. -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-07 19:55 ` Chris Metcalf @ 2013-11-10 8:24 ` Chung-Lin Tang 2013-11-11 17:48 ` Joseph S. Myers 0 siblings, 1 reply; 15+ messages in thread From: Chung-Lin Tang @ 2013-11-10 8:24 UTC (permalink / raw) To: Chris Metcalf, libc-ports, Andrew Pinski On 2013/11/8 03:55 AM, Chris Metcalf wrote: > On 11/4/2013 2:18 AM, Chung-Lin Tang wrote: >> Hi, >> >> I'm currently working on the glibc port for Altera Nios II, and per the >> kernel upstreaming requirements, we're making it a user of linux-generic >> in glibc. >> >> I've come across an issue about struct statfs/statfs64, where I've >> listed the struct definitions in both glibc and the kernel below. >> >> In linux-generic, both struct statfs/statfs64 has some fields as 64-bit >> words, padded properly if need on 32-bit targets. Effectively, they have >> to be same-format, to pass to the statfs64 syscall uniformly. >> >> __SWORD_TYPE appears to be int/long on 32/64-bit targets. This means >> that for 32-bit targets, struct statfs[64] will be made of 32-bit words, >> with a few 64-bit fields embedded in the middle. >> >> >> In the kernel however, the fields of importance in struct statfs is made >> up of entirely '__statfs_word', defined to be 64-bit or 32-bit depending >> on target (putting aside signedness for now). > > I have to carefully swap all this stuff back into my head every time I look at this issue :-) Thanks for caring to do that :) > For native 32-bit (for us that means tilepro) the constraint is that the kernel's "struct statfs64" has to match the two structs in glibc's <bits/statfs.h>. Most of the statfs types are fixed-size (fsid_t and the __u64 types), with __statfs_word/__SWORD_TYPE being the exception. For tilepro both of those kernel and glibc types are 32-bit types. > > For 32-bit userspace in a 64-bit kernel (for us that means tilegx -m32) the constraint is that the kernel's "struct compat_statfs64" match the <bits/statfs.h> types, which again, it seems that they should. (Note that tilegx -m32 is explicitly a "compat" syscall model.) > > A quick look suggests that nios2 is a pure 32-bit architecture, so more like the tilepro model. > > Andrew Pinski's email was a bit confusing since __SYSCALL_SLONG_TYPE seems to be the same size as __SWORD_TYPE, and I don't see any redefinitions of it for aarch64. I believe his usecase is the 32-bit userspace on 64-bit kernel model. One thing to watch out for is that some architectures (like x32) actually use the 64-bit syscalls natively (no compat_statfs64) so they need to use "long long" types to match the 64-bit fields, etc., so it's a different problem. In retrospect that might have been a cleaner model for tilegx -m32, but I suspect that ship has sailed for us. > > In your followup email you write: > >> Still, I'm not sure if using full 64-bit fields should be the intended >> solution. A full 32-bit target should be able to have those fields as >> 32-bit (which means a kernel patch to differentiate the current uses of >> '__statfs_word' into 32/64-bit words) > > I have a guess as to what the problem you're seeing is: are you looking at "struct statfs" in the kernel? That structure is completely unused on 32-bit platforms, since it's only relevant for the 64-bit platform's sys_statfs/sys_fstatfs syscalls, whereas on 32-bit platforms you only get sys_statfs64/sys_fstatfs64. In other words there is no syscall that fills in a structure with 32-bit "f_blocks" (for example). See the __NR3264_statfs macro in <asm-generic/unistd.h> and how it resolves with the __SC_COMP_3264() macro, to the __SC_3264() macro, to the 32-bit version of the syscall. I think I found my problem, which is more simpler than I thought: unlike the majority of targets, 64-bit types are 4-byte aligned under Nios II, which causes a 4-byte difference between the glibc/kernel structure definitions because the kernel one doesn't have __attribute__((aligned(8))) forced on to it. Stack alignment is 4-bytes as well in nios2, so I'll probably be maintaining that and using a specific nios2/bits/statfs.h header without the double-word alignment bits. Thanks, Chung-Lin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-10 8:24 ` Chung-Lin Tang @ 2013-11-11 17:48 ` Joseph S. Myers 2013-11-13 8:17 ` Chung-Lin Tang 0 siblings, 1 reply; 15+ messages in thread From: Joseph S. Myers @ 2013-11-11 17:48 UTC (permalink / raw) To: Chung-Lin Tang; +Cc: Chris Metcalf, libc-ports, Andrew Pinski On Sun, 10 Nov 2013, Chung-Lin Tang wrote: > I think I found my problem, which is more simpler than I thought: unlike > the majority of targets, 64-bit types are 4-byte aligned under Nios II, > which causes a 4-byte difference between the glibc/kernel structure > definitions because the kernel one doesn't have > __attribute__((aligned(8))) forced on to it. > > Stack alignment is 4-bytes as well in nios2, so I'll probably be > maintaining that and using a specific nios2/bits/statfs.h header without > the double-word alignment bits. Are you sure this is the only place in the kernel/userspace ABI where linux-generic is assuming natural alignment? -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-11 17:48 ` Joseph S. Myers @ 2013-11-13 8:17 ` Chung-Lin Tang 2013-11-13 12:59 ` Joseph S. Myers 0 siblings, 1 reply; 15+ messages in thread From: Chung-Lin Tang @ 2013-11-13 8:17 UTC (permalink / raw) To: Joseph S. Myers; +Cc: Chris Metcalf, libc-ports, Andrew Pinski [-- Attachment #1: Type: text/plain, Size: 1644 bytes --] On 13/11/12 ä¸å1:48, Joseph S. Myers wrote: > On Sun, 10 Nov 2013, Chung-Lin Tang wrote: > >> I think I found my problem, which is more simpler than I thought: unlike >> the majority of targets, 64-bit types are 4-byte aligned under Nios II, >> which causes a 4-byte difference between the glibc/kernel structure >> definitions because the kernel one doesn't have >> __attribute__((aligned(8))) forced on to it. >> >> Stack alignment is 4-bytes as well in nios2, so I'll probably be >> maintaining that and using a specific nios2/bits/statfs.h header without >> the double-word alignment bits. > > Are you sure this is the only place in the kernel/userspace ABI where > linux-generic is assuming natural alignment? > generic/bits/stat.h also has use of __attribute__(aligned()), though the field elements happen to amount to an even number of words, so no such inconsistency there; just that user code will enforce additional alignment. The problem I think is the use of the hard coded "8", so instead of requiring ports like nios2 to possess nearly identical header files (without the alignment attributes), how about the attached patch that changes it to use __alignof__(type64) instead? Thanks, Chung-Lin 2013-11-13 Chung-Lin Tang <cltang@codesourcery.com> ports/ * sysdeps/unix/sysv/linux/generic/bits/stat.h (__field64): Use __alignof__(type64) in alignment attribute instead of 8. * sysdeps/unix/sysv/linux/generic/bits/statfs.h (__field64): Use __alignof__(type64) in alignment attribute instead of 8. (struct statfs): Use __alignof__(__u64) in alignment attribute instead of 8. (struct statfs64): Likewise. [-- Attachment #2: x.diff --] [-- Type: text/plain, Size: 2069 bytes --] diff --git a/ports/sysdeps/unix/sysv/linux/generic/bits/stat.h b/ports/sysdeps/unix/sysv/linux/generic/bits/stat.h index 6e74cec..feb5f2b 100644 --- a/ports/sysdeps/unix/sysv/linux/generic/bits/stat.h +++ b/ports/sysdeps/unix/sysv/linux/generic/bits/stat.h @@ -46,10 +46,10 @@ # define __field64(type, type64, name) type name #elif __BYTE_ORDER == __LITTLE_ENDIAN # define __field64(type, type64, name) \ - type name __attribute__((__aligned__(8))); int __##name##_pad + type name __attribute__((__aligned__ (__alignof__ (type64)))); int __##name##_pad #else # define __field64(type, type64, name) \ - int __##name##_pad __attribute__((__aligned__(8))); type name + int __##name##_pad __attribute__((__aligned__ (__alignof__ (type64)))); type name #endif struct stat diff --git a/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h b/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h index 7063c7a..54edeb5 100644 --- a/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h +++ b/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h @@ -38,10 +38,10 @@ # define __field64(type, type64, name) type name #elif __BYTE_ORDER == __LITTLE_ENDIAN # define __field64(type, type64, name) \ - type name __attribute__((__aligned__(8))); int __##name##_pad + type name __attribute__((__aligned__ (__alignof__ (type64)))); int __##name##_pad #else # define __field64(type, type64, name) \ - int __##name##_pad __attribute__((__aligned__(8))); type name + int __##name##_pad __attribute__((__aligned__ (__alignof__ (type64)))); type name #endif struct statfs @@ -58,7 +58,7 @@ struct statfs __SWORD_TYPE f_frsize; __SWORD_TYPE f_flags; __SWORD_TYPE f_spare[4]; - } __attribute__((__aligned__(8))); + } __attribute__((__aligned__ (__alignof__ (__u64)))); #undef __field64 @@ -77,7 +77,7 @@ struct statfs64 __SWORD_TYPE f_frsize; __SWORD_TYPE f_flags; __SWORD_TYPE f_spare[4]; - } __attribute__((__aligned__(8))); + } __attribute__((__aligned__ (__alignof__ (__u64)))); #endif /* Tell code we have these members. */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-13 8:17 ` Chung-Lin Tang @ 2013-11-13 12:59 ` Joseph S. Myers 2013-11-14 4:34 ` Chung-Lin Tang 0 siblings, 1 reply; 15+ messages in thread From: Joseph S. Myers @ 2013-11-13 12:59 UTC (permalink / raw) To: Chung-Lin Tang; +Cc: Chris Metcalf, libc-ports, Andrew Pinski On Wed, 13 Nov 2013, Chung-Lin Tang wrote: > (struct statfs): Use __alignof__(__u64) in alignment attribute > instead of 8. > (struct statfs64): Likewise. I don't like the use of __u64 here - that's a kernel type, not a glibc one, and glibc headers shouldn't be using it. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-13 12:59 ` Joseph S. Myers @ 2013-11-14 4:34 ` Chung-Lin Tang 2013-11-14 13:36 ` Joseph S. Myers 2013-11-14 18:28 ` Chris Metcalf 0 siblings, 2 replies; 15+ messages in thread From: Chung-Lin Tang @ 2013-11-14 4:34 UTC (permalink / raw) To: Joseph S. Myers; +Cc: Chris Metcalf, libc-ports, Andrew Pinski On 13/11/13 8:58 PM, Joseph S. Myers wrote: > On Wed, 13 Nov 2013, Chung-Lin Tang wrote: > >> (struct statfs): Use __alignof__(__u64) in alignment attribute >> instead of 8. >> (struct statfs64): Likewise. > > I don't like the use of __u64 here - that's a kernel type, not a glibc > one, and glibc headers shouldn't be using it. > Sure, how about __U64_TYPE? This seems in line with using __SWORD_TYPE from <bits/types.h> Thanks, Chung-Lin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-14 4:34 ` Chung-Lin Tang @ 2013-11-14 13:36 ` Joseph S. Myers 2013-11-14 18:28 ` Chris Metcalf 1 sibling, 0 replies; 15+ messages in thread From: Joseph S. Myers @ 2013-11-14 13:36 UTC (permalink / raw) To: Chung-Lin Tang; +Cc: Chris Metcalf, libc-ports, Andrew Pinski On Thu, 14 Nov 2013, Chung-Lin Tang wrote: > On 13/11/13 8:58 PM, Joseph S. Myers wrote: > > On Wed, 13 Nov 2013, Chung-Lin Tang wrote: > > > >> (struct statfs): Use __alignof__(__u64) in alignment attribute > >> instead of 8. > >> (struct statfs64): Likewise. > > > > I don't like the use of __u64 here - that's a kernel type, not a glibc > > one, and glibc headers shouldn't be using it. > > > > Sure, how about __U64_TYPE? This seems in line with using __SWORD_TYPE > from <bits/types.h> That seems reasonable to me (though it's Chris you need to reach agreement with). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-14 4:34 ` Chung-Lin Tang 2013-11-14 13:36 ` Joseph S. Myers @ 2013-11-14 18:28 ` Chris Metcalf 2013-11-15 7:39 ` Chung-Lin Tang 1 sibling, 1 reply; 15+ messages in thread From: Chris Metcalf @ 2013-11-14 18:28 UTC (permalink / raw) To: Chung-Lin Tang, Joseph S. Myers; +Cc: libc-ports, Andrew Pinski On 11/13/2013 11:34 PM, Chung-Lin Tang wrote: > On 13/11/13 8:58 PM, Joseph S. Myers wrote: >> On Wed, 13 Nov 2013, Chung-Lin Tang wrote: >> >>> (struct statfs): Use __alignof__(__u64) in alignment attribute >>> instead of 8. >>> (struct statfs64): Likewise. >> I don't like the use of __u64 here - that's a kernel type, not a glibc >> one, and glibc headers shouldn't be using it. >> > Sure, how about __U64_TYPE? This seems in line with using __SWORD_TYPE > from <bits/types.h> Looking at this, I wonder if we need the alignment attribute on the structure at all. Given that the __field64 macro fields have that alignment attribute, the structure as a whole should also automatically have it, so it seems superfluous. Does it work on nios2 if you remove the struct alignment directive? The kernel version of the structure doesn't have any forced alignment on it. -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-14 18:28 ` Chris Metcalf @ 2013-11-15 7:39 ` Chung-Lin Tang 2013-11-18 23:39 ` Chris Metcalf 0 siblings, 1 reply; 15+ messages in thread From: Chung-Lin Tang @ 2013-11-15 7:39 UTC (permalink / raw) To: Chris Metcalf, Joseph S. Myers; +Cc: libc-ports, Andrew Pinski [-- Attachment #1: Type: text/plain, Size: 1489 bytes --] On 13/11/15 2:28 AM, Chris Metcalf wrote: > On 11/13/2013 11:34 PM, Chung-Lin Tang wrote: >> On 13/11/13 8:58 PM, Joseph S. Myers wrote: >>> On Wed, 13 Nov 2013, Chung-Lin Tang wrote: >>> >>>> (struct statfs): Use __alignof__(__u64) in alignment attribute >>>> instead of 8. >>>> (struct statfs64): Likewise. >>> I don't like the use of __u64 here - that's a kernel type, not a glibc >>> one, and glibc headers shouldn't be using it. >>> >> Sure, how about __U64_TYPE? This seems in line with using __SWORD_TYPE >> from <bits/types.h> > > Looking at this, I wonder if we need the alignment attribute on the structure at all. Given that the __field64 macro fields have that alignment attribute, the structure as a whole should also automatically have it, so it seems superfluous. Does it work on nios2 if you remove the struct alignment directive? > > The kernel version of the structure doesn't have any forced alignment on it. > Yes, that works for nios2. Appears that struct stat/stat64 in bits/stat.h doesn't have the struct alignment attributes either. Updated patch attached. Chung-Lin 2013-11-13 Chung-Lin Tang <cltang@codesourcery.com> ports/ * sysdeps/unix/sysv/linux/generic/bits/stat.h (__field64): Use __alignof__(type64) in alignment attribute instead of 8. * sysdeps/unix/sysv/linux/generic/bits/statfs.h (__field64): Use __alignof__(type64) in alignment attribute instead of 8. (struct statfs): Remove alignment attribute. (struct statfs64): Likewise. [-- Attachment #2: x.diff --] [-- Type: text/plain, Size: 1967 bytes --] diff --git a/ports/sysdeps/unix/sysv/linux/generic/bits/stat.h b/ports/sysdeps/unix/sysv/linux/generic/bits/stat.h index 6e74cec..feb5f2b 100644 --- a/ports/sysdeps/unix/sysv/linux/generic/bits/stat.h +++ b/ports/sysdeps/unix/sysv/linux/generic/bits/stat.h @@ -46,10 +46,10 @@ # define __field64(type, type64, name) type name #elif __BYTE_ORDER == __LITTLE_ENDIAN # define __field64(type, type64, name) \ - type name __attribute__((__aligned__(8))); int __##name##_pad + type name __attribute__((__aligned__ (__alignof__ (type64)))); int __##name##_pad #else # define __field64(type, type64, name) \ - int __##name##_pad __attribute__((__aligned__(8))); type name + int __##name##_pad __attribute__((__aligned__ (__alignof__ (type64)))); type name #endif struct stat diff --git a/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h b/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h index 7063c7a..8aecb04 100644 --- a/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h +++ b/ports/sysdeps/unix/sysv/linux/generic/bits/statfs.h @@ -38,10 +38,10 @@ # define __field64(type, type64, name) type name #elif __BYTE_ORDER == __LITTLE_ENDIAN # define __field64(type, type64, name) \ - type name __attribute__((__aligned__(8))); int __##name##_pad + type name __attribute__((__aligned__ (__alignof__ (type64)))); int __##name##_pad #else # define __field64(type, type64, name) \ - int __##name##_pad __attribute__((__aligned__(8))); type name + int __##name##_pad __attribute__((__aligned__ (__alignof__ (type64)))); type name #endif struct statfs @@ -58,7 +58,7 @@ struct statfs __SWORD_TYPE f_frsize; __SWORD_TYPE f_flags; __SWORD_TYPE f_spare[4]; - } __attribute__((__aligned__(8))); + }; #undef __field64 @@ -77,7 +77,7 @@ struct statfs64 __SWORD_TYPE f_frsize; __SWORD_TYPE f_flags; __SWORD_TYPE f_spare[4]; - } __attribute__((__aligned__(8))); + }; #endif /* Tell code we have these members. */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-15 7:39 ` Chung-Lin Tang @ 2013-11-18 23:39 ` Chris Metcalf 2013-11-19 13:50 ` Chung-Lin Tang 0 siblings, 1 reply; 15+ messages in thread From: Chris Metcalf @ 2013-11-18 23:39 UTC (permalink / raw) To: Chung-Lin Tang, Joseph S. Myers Cc: libc-ports, Andrew Pinski, Marcus Shawcroft On 11/15/2013 2:39 AM, Chung-Lin Tang wrote: > Yes, that works for nios2. Appears that struct stat/stat64 in > bits/stat.h doesn't have the struct alignment attributes either. > Updated patch attached. > > Chung-Lin > > 2013-11-13 Chung-Lin Tang <cltang@codesourcery.com> > > ports/ > * sysdeps/unix/sysv/linux/generic/bits/stat.h (__field64): Use > __alignof__(type64) in alignment attribute instead of 8. > * sysdeps/unix/sysv/linux/generic/bits/statfs.h (__field64): Use > __alignof__(type64) in alignment attribute instead of 8. > (struct statfs): Remove alignment attribute. > (struct statfs64): Likewise. The patch works fine for tilegx, so I think it should be OK to commit. Marcus, did you want to verify that it works OK for AArch64 first? My guess it that it should be OK for you. Chung-Lin Tang, do you have commit access, or would you like me to commit the change for you? -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-18 23:39 ` Chris Metcalf @ 2013-11-19 13:50 ` Chung-Lin Tang 2013-11-22 2:43 ` Chris Metcalf 0 siblings, 1 reply; 15+ messages in thread From: Chung-Lin Tang @ 2013-11-19 13:50 UTC (permalink / raw) To: Chris Metcalf, Joseph S. Myers Cc: libc-ports, Andrew Pinski, Marcus Shawcroft On 13/11/19 2:20 AM, Chris Metcalf wrote: > On 11/15/2013 2:39 AM, Chung-Lin Tang wrote: >> Yes, that works for nios2. Appears that struct stat/stat64 in >> bits/stat.h doesn't have the struct alignment attributes either. >> Updated patch attached. >> >> Chung-Lin >> >> 2013-11-13 Chung-Lin Tang <cltang@codesourcery.com> >> >> ports/ >> * sysdeps/unix/sysv/linux/generic/bits/stat.h (__field64): Use >> __alignof__(type64) in alignment attribute instead of 8. >> * sysdeps/unix/sysv/linux/generic/bits/statfs.h (__field64): Use >> __alignof__(type64) in alignment attribute instead of 8. >> (struct statfs): Remove alignment attribute. >> (struct statfs64): Likewise. > > The patch works fine for tilegx, so I think it should be OK to commit. > > Marcus, did you want to verify that it works OK for AArch64 first? > My guess it that it should be OK for you. > > Chung-Lin Tang, do you have commit access, or would you like me to > commit the change for you? No, I don't think I have commit access for glibc (it's separate from binutils/gdb I think?) Please commit for me. Thanks, Chung-Lin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: struct statfs/statfs64 in linux-generic 2013-11-19 13:50 ` Chung-Lin Tang @ 2013-11-22 2:43 ` Chris Metcalf 0 siblings, 0 replies; 15+ messages in thread From: Chris Metcalf @ 2013-11-22 2:43 UTC (permalink / raw) To: Chung-Lin Tang, Joseph S. Myers Cc: libc-ports, Andrew Pinski, Marcus Shawcroft On 11/18/2013 10:28 PM, Chung-Lin Tang wrote: > On 13/11/19 2:20 AM, Chris Metcalf wrote: >> Chung-Lin Tang, do you have commit access, or would you like me to >> commit the change for you? > No, I don't think I have commit access for glibc (it's separate from > binutils/gdb I think?) Please commit for me. Committed as 7cf8ac4c3179. -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-11-20 21:15 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-11-04 7:18 struct statfs/statfs64 in linux-generic Chung-Lin Tang 2013-11-04 8:19 ` Andrew Pinski 2013-11-05 4:23 ` Chung-Lin Tang 2013-11-07 19:55 ` Chris Metcalf 2013-11-10 8:24 ` Chung-Lin Tang 2013-11-11 17:48 ` Joseph S. Myers 2013-11-13 8:17 ` Chung-Lin Tang 2013-11-13 12:59 ` Joseph S. Myers 2013-11-14 4:34 ` Chung-Lin Tang 2013-11-14 13:36 ` Joseph S. Myers 2013-11-14 18:28 ` Chris Metcalf 2013-11-15 7:39 ` Chung-Lin Tang 2013-11-18 23:39 ` Chris Metcalf 2013-11-19 13:50 ` Chung-Lin Tang 2013-11-22 2:43 ` Chris Metcalf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).