struct statfs/statfs64 in linux-generic

struct statfs/statfs64 in linux-generic

[Chung-Lin Tang]

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;

Re: struct statfs/statfs64 in linux-generic

[Andrew Pinski]

[-- 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
 

Re: struct statfs/statfs64 in linux-generic

[Chung-Lin Tang]

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

Re: struct statfs/statfs64 in linux-generic

[Chris Metcalf]

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

Re: struct statfs/statfs64 in linux-generic

[Chung-Lin Tang]

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

Re: struct statfs/statfs64 in linux-generic

[Joseph S. Myers]

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

Re: struct statfs/statfs64 in linux-generic

[Chung-Lin Tang]

[-- 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.  */

Re: struct statfs/statfs64 in linux-generic

[Joseph S. Myers]

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

Re: struct statfs/statfs64 in linux-generic

[Chung-Lin Tang]

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

Re: struct statfs/statfs64 in linux-generic

[Joseph S. Myers]

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

Re: struct statfs/statfs64 in linux-generic

[Chris Metcalf]

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

Re: struct statfs/statfs64 in linux-generic

[Chung-Lin Tang]

[-- 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.  */

Re: struct statfs/statfs64 in linux-generic

[Chris Metcalf]

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

Re: struct statfs/statfs64 in linux-generic

[Chung-Lin Tang]

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

Re: struct statfs/statfs64 in linux-generic

[Chris Metcalf]

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