| Message ID | CAMe9rOpqM3QsZpFnCb57gcWMwFR6acdVCvofV=U40avD78nG8w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 18/03/15 11:59, H.J. Lu wrote: > Any particular reason to add > > sysdeps/unix/sysv/linux/generic/bits/statfs.h > > when there is > > sysdeps/unix/sysv/linux/bits/statfs.h > > already? Why can't it be used? Here is a patch to remove it. OK > for master? > the two does not seem to be the same on 32bit archs when 32bit off_t is used the first tries to use a layout that is compatible with the 64bit off_t version (so it needs endianness dependent padding) the second uses the legacy syscall for 32bit off_t mode which has no paddings i'm not an expert in the legacy 32bit off_t hacks, but if you remove the first then all the related functions from sysdeps/unix/sysv/linux/generic/wordsize-32/* (which do 32bit vs 64bit overflow checks etc)
On 03/18/2015 07:59 AM, H.J. Lu wrote: > Any particular reason to add > sysdeps/unix/sysv/linux/generic/bits/statfs.h when there is > sysdeps/unix/sysv/linux/bits/statfs.h already? Why can't it be used? The distinction lies with the the fields defined by __field64 in the "generic" version. For the case without __USE_FILE_OFFSET64, but when ILP32 mode is involved, the base linux version uses plain __fsblkcnt_t, i.e. unsigned long, so just 32 bits. However, it needs to be a 64-bit field, where either the first or second 32-bit subfield is padding depending on endianness, and the field value is the other 32-bit subfield. We would need to adopt the __field64 scheme for the base version in some way that allowed platforms to turn it on as needed (depending on whether statfs is filled with a statfs64 syscall or a plain 32-bit statfs syscall on older, non asm-generic 32-bit kernels). This is doable but fiddly to get exactly right. Another difference I see is easy to work around. The "generic" linux version uses __SWORD_TYPE where the base version uses __fsword_t (changed by hjl in 2012). If the generic platforms (tile, aarch64, nios2) switched, this would be OK since the types are the same on those platforms. (Only alpha and x32 define them differently.)
On Thu, Mar 19, 2015 at 9:31 AM, Chris Metcalf <cmetcalf@ezchip.com> wrote: > On 03/18/2015 07:59 AM, H.J. Lu wrote: >> >> Any particular reason to add sysdeps/unix/sysv/linux/generic/bits/statfs.h >> when there is sysdeps/unix/sysv/linux/bits/statfs.h already? Why can't it be >> used? > > > The distinction lies with the the fields defined by __field64 in the > "generic" version. For the case without __USE_FILE_OFFSET64, but when ILP32 > mode is involved, the base linux version uses plain __fsblkcnt_t, i.e. > unsigned long, so just 32 bits. However, it needs to be a 64-bit field, > where either the first or second 32-bit subfield is padding depending on > endianness, and the field value is the other 32-bit subfield. We would need > to adopt the __field64 scheme for the base version in some way that allowed > platforms to turn it on as needed (depending on whether statfs is filled > with a statfs64 syscall or a plain 32-bit statfs syscall on older, non > asm-generic 32-bit kernels). This is doable but fiddly to get exactly > right. > > Another difference I see is easy to work around. The "generic" linux > version uses __SWORD_TYPE where the base version uses __fsword_t (changed by > hjl in 2012). If the generic platforms (tile, aarch64, nios2) switched, > this would be OK since the types are the same on those platforms. (Only > alpha and x32 define them differently.) There are sysdeps/unix/sysv/linux/mips/bits/statfs.h sysdeps/unix/sysv/linux/alpha/bits/statfs.h sysdeps/unix/sysv/linux/bits/statfs.h sysdeps/unix/sysv/linux/s390/bits/statfs.h If sysdeps/unix/sysv/linux/bits/statfs.h can't be used for a Linux platform, it should define its own statfs.h, like alpha, mips and s390. Having both sysdeps/unix/sysv/linux/generic/bits/statfs.h and sysdeps/unix/sysv/linux/bits/statfs.h is very strange. H.J.
On 03/19/2015 12:44 PM, H.J. Lu wrote: > On Thu, Mar 19, 2015 at 9:31 AM, Chris Metcalf <cmetcalf@ezchip.com> wrote: >> On 03/18/2015 07:59 AM, H.J. Lu wrote: >>> Any particular reason to add sysdeps/unix/sysv/linux/generic/bits/statfs.h >>> when there is sysdeps/unix/sysv/linux/bits/statfs.h already? Why can't it be >>> used? >> >> The distinction lies with the the fields defined by __field64 in the >> "generic" version. For the case without __USE_FILE_OFFSET64, but when ILP32 >> mode is involved, the base linux version uses plain __fsblkcnt_t, i.e. >> unsigned long, so just 32 bits. However, it needs to be a 64-bit field, >> where either the first or second 32-bit subfield is padding depending on >> endianness, and the field value is the other 32-bit subfield. We would need >> to adopt the __field64 scheme for the base version in some way that allowed >> platforms to turn it on as needed (depending on whether statfs is filled >> with a statfs64 syscall or a plain 32-bit statfs syscall on older, non >> asm-generic 32-bit kernels). This is doable but fiddly to get exactly >> right. >> >> Another difference I see is easy to work around. The "generic" linux >> version uses __SWORD_TYPE where the base version uses __fsword_t (changed by >> hjl in 2012). If the generic platforms (tile, aarch64, nios2) switched, >> this would be OK since the types are the same on those platforms. (Only >> alpha and x32 define them differently.) > There are > > sysdeps/unix/sysv/linux/mips/bits/statfs.h > sysdeps/unix/sysv/linux/alpha/bits/statfs.h > sysdeps/unix/sysv/linux/bits/statfs.h > sysdeps/unix/sysv/linux/s390/bits/statfs.h > > If sysdeps/unix/sysv/linux/bits/statfs.h can't be used for > a Linux platform, it should define its own statfs.h, like > alpha, mips and s390. The linux/generic layer is essentially an intermediate layer for all new Linux ports that use the asm-generic syscall interface. Rather than "inheriting" directly from the sysdeps/unix/sysv/linux/, newer ports inherit from sysdeps/unix/sysv/linux/generic/, which tweaks whatever needs tweaking to support the fact that a far smaller set of syscalls is available on such platforms. This means tile, aarch64, and nios2 don't have to each provide their own replacement statfs.h. > Having both > > sysdeps/unix/sysv/linux/generic/bits/statfs.h > > and > > sysdeps/unix/sysv/linux/bits/statfs.h > > is very strange. Yes, I don't love the naming either, but no one could come up with anything better when this was initially introduced. It may be the case that we can do some unification along the lines I suggested in my previous email, but the tricky part is figuring out programmatically which platforms are using the new API (and thus don't have a version of statfs with 32-bit fsword_t) and which are using the old API. Or, moving forward, which platforms are using the LP64 version of the syscall even in ILP32 mode (x32, aarch64:ILP32). Since this is a public header we don't have kernel-features.h. I'm not sure where else we would normally place this kind of information in the headers.
On Thu, Mar 19, 2015 at 9:59 AM, Chris Metcalf <cmetcalf@ezchip.com> wrote: > On 03/19/2015 12:44 PM, H.J. Lu wrote: >> >> On Thu, Mar 19, 2015 at 9:31 AM, Chris Metcalf <cmetcalf@ezchip.com> >> wrote: >>> >>> On 03/18/2015 07:59 AM, H.J. Lu wrote: >>>> >>>> Any particular reason to add >>>> sysdeps/unix/sysv/linux/generic/bits/statfs.h >>>> when there is sysdeps/unix/sysv/linux/bits/statfs.h already? Why can't >>>> it be >>>> used? >>> >>> >>> The distinction lies with the the fields defined by __field64 in the >>> "generic" version. For the case without __USE_FILE_OFFSET64, but when >>> ILP32 >>> mode is involved, the base linux version uses plain __fsblkcnt_t, i.e. >>> unsigned long, so just 32 bits. However, it needs to be a 64-bit field, >>> where either the first or second 32-bit subfield is padding depending on >>> endianness, and the field value is the other 32-bit subfield. We would >>> need >>> to adopt the __field64 scheme for the base version in some way that >>> allowed >>> platforms to turn it on as needed (depending on whether statfs is filled >>> with a statfs64 syscall or a plain 32-bit statfs syscall on older, non >>> asm-generic 32-bit kernels). This is doable but fiddly to get exactly >>> right. >>> >>> Another difference I see is easy to work around. The "generic" linux >>> version uses __SWORD_TYPE where the base version uses __fsword_t (changed >>> by >>> hjl in 2012). If the generic platforms (tile, aarch64, nios2) switched, >>> this would be OK since the types are the same on those platforms. (Only >>> alpha and x32 define them differently.) >> >> There are >> >> sysdeps/unix/sysv/linux/mips/bits/statfs.h >> sysdeps/unix/sysv/linux/alpha/bits/statfs.h >> sysdeps/unix/sysv/linux/bits/statfs.h >> sysdeps/unix/sysv/linux/s390/bits/statfs.h >> >> If sysdeps/unix/sysv/linux/bits/statfs.h can't be used for >> a Linux platform, it should define its own statfs.h, like >> alpha, mips and s390. > > > The linux/generic layer is essentially an intermediate layer for all new > Linux ports that use the asm-generic syscall interface. Rather than > "inheriting" directly from the sysdeps/unix/sysv/linux/, newer ports inherit > from sysdeps/unix/sysv/linux/generic/, which tweaks whatever needs tweaking > to support the fact that a far smaller set of syscalls is available on such > platforms. This means tile, aarch64, and nios2 don't have to each provide > their own replacement statfs.h. > >> Having both >> >> sysdeps/unix/sysv/linux/generic/bits/statfs.h >> >> and >> >> sysdeps/unix/sysv/linux/bits/statfs.h >> >> is very strange. > > > Yes, I don't love the naming either, but no one could come up with anything > better when this was initially introduced. > How about sysdeps/unix/sysv/linux/asm-generic/bits/statfs.h?
On 03/19/2015 01:04 PM, H.J. Lu wrote: >>> Having both >>> >> >>> >>sysdeps/unix/sysv/linux/generic/bits/statfs.h >>> >> >>> >>and >>> >> >>> >>sysdeps/unix/sysv/linux/bits/statfs.h >>> >> >>> >>is very strange. >> > >> > >> >Yes, I don't love the naming either, but no one could come up with anything >> >better when this was initially introduced. >> > > How about sysdeps/unix/sysv/linux/asm-generic/bits/statfs.h? As I recall the original discussion, the idea was that going forward the asm-generic stuff would correspond to new "generic" Linux support. It's not clear the "asm-" really adds anything to the name. That said, I don't feel strongly about it, and if there is consensus to change the name, I'd be happy to accept a patch to do so. There are 80+ files in that hierarchy, but I think some simple grep work should suffice to find any references to linux/generic.
On Thu, Mar 19, 2015 at 10:12 AM, Chris Metcalf <cmetcalf@ezchip.com> wrote: > On 03/19/2015 01:04 PM, H.J. Lu wrote: >>>> >>>> Having both >>>> >> >>>> >>sysdeps/unix/sysv/linux/generic/bits/statfs.h >>>> >> >>>> >>and >>>> >> >>>> >>sysdeps/unix/sysv/linux/bits/statfs.h >>>> >> >>>> >>is very strange. >>> >>> > >>> > >>> >Yes, I don't love the naming either, but no one could come up with >>> > anything >>> >better when this was initially introduced. >>> > >> >> How about sysdeps/unix/sysv/linux/asm-generic/bits/statfs.h? > > > As I recall the original discussion, the idea was that going forward the > asm-generic stuff would correspond to new "generic" Linux support. It's not > clear the "asm-" really adds anything to the name. > > That said, I don't feel strongly about it, and if there is consensus to > change the name, I'd be happy to accept a patch to do so. There are 80+ > files in that hierarchy, but I think some simple grep work should suffice to > find any references to linux/generic. > There are many files in linux/generic. I don't know if it is worth to change. It is just very annoying. Is the linux/generic scheme documented in glibc tree? Should there be a README in linux/generic directory?
On 19/03/15 16:59, Chris Metcalf wrote: > On 03/19/2015 12:44 PM, H.J. Lu wrote: >> On Thu, Mar 19, 2015 at 9:31 AM, Chris Metcalf <cmetcalf@ezchip.com> wrote: >>> On 03/18/2015 07:59 AM, H.J. Lu wrote: >>>> Any particular reason to add sysdeps/unix/sysv/linux/generic/bits/statfs.h >>>> when there is sysdeps/unix/sysv/linux/bits/statfs.h already? Why can't it be >>>> used? >>> >>> The distinction lies with the the fields defined by __field64 in the >>> "generic" version. For the case without __USE_FILE_OFFSET64, but when ILP32 >>> mode is involved, the base linux version uses plain __fsblkcnt_t, i.e. >>> unsigned long, so just 32 bits. However, it needs to be a 64-bit field, >>> where either the first or second 32-bit subfield is padding depending on >>> endianness, and the field value is the other 32-bit subfield. We would need >>> to adopt the __field64 scheme for the base version in some way that allowed >>> platforms to turn it on as needed (depending on whether statfs is filled >>> with a statfs64 syscall or a plain 32-bit statfs syscall on older, non >>> asm-generic 32-bit kernels). This is doable but fiddly to get exactly >>> right. >>> >>> Another difference I see is easy to work around. The "generic" linux >>> version uses __SWORD_TYPE where the base version uses __fsword_t (changed by >>> hjl in 2012). If the generic platforms (tile, aarch64, nios2) switched, >>> this would be OK since the types are the same on those platforms. (Only >>> alpha and x32 define them differently.) >> There are >> >> sysdeps/unix/sysv/linux/mips/bits/statfs.h >> sysdeps/unix/sysv/linux/alpha/bits/statfs.h >> sysdeps/unix/sysv/linux/bits/statfs.h >> sysdeps/unix/sysv/linux/s390/bits/statfs.h >> >> If sysdeps/unix/sysv/linux/bits/statfs.h can't be used for >> a Linux platform, it should define its own statfs.h, like >> alpha, mips and s390. > > The linux/generic layer is essentially an intermediate layer for all > new Linux ports that use the asm-generic syscall interface. Rather than > "inheriting" directly from the sysdeps/unix/sysv/linux/, newer ports > inherit from sysdeps/unix/sysv/linux/generic/, which tweaks whatever > needs tweaking to support the fact that a far smaller set of syscalls is > available on such platforms. This means tile, aarch64, and nios2 don't > have to each provide their own replacement statfs.h. > i think aarch64 does not want 32bit blkcnt, that breaks on a large fs (not even with ilp32) i'm surprised it is in use on newer 32bit archs at all (if it is then i think linux asm-generic needs a fix) >> Having both >> >> sysdeps/unix/sysv/linux/generic/bits/statfs.h >> >> and >> >> sysdeps/unix/sysv/linux/bits/statfs.h >> >> is very strange. > > Yes, I don't love the naming either, but no one could come up with > anything better when this was initially introduced. > > It may be the case that we can do some unification along the lines I > suggested in my previous email, but the tricky part is figuring out > programmatically which platforms are using the new API (and thus don't > have a version of statfs with 32-bit fsword_t) and which are using the > old API. Or, moving forward, which platforms are using the LP64 version > of the syscall even in ILP32 mode (x32, aarch64:ILP32). Since this is a > public header we don't have kernel-features.h. I'm not sure where else > we would normally place this kind of information in the headers. >
On 03/19/2015 01:20 PM, Szabolcs Nagy wrote: > i think aarch64 does not want 32bit blkcnt, that breaks on > a large fs (not even with ilp32) > > i'm surprised it is in use on newer 32bit archs at all > (if it is then i think linux asm-generic needs a fix) Well, things work fine on ILP32 if you build with __FILE_OFFSET_BITS=64, of course. And that is pretty much the standard, these days. It's true that it would be nice if you didn't have to do that. But the way we set it up matched the expected behavior from older Linuxes, and matching that behavior made sense five years ago when we were doing this work at Tilera; after all, that's how i386 compatibility behaves on 64-bit x86, which was pretty much our benchmark for what approach to use.
From 5a8487ddd51e23b0786681ecbc86dc2b9081b5ae Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 18 Mar 2015 04:54:03 -0700
Subject: [PATCH] Remove sysdeps/unix/sysv/linux/generic/bits/statfs.h
sysdeps/unix/sysv/linux/generic/bits/statfs.h was added by
commit 4372980f5881e7d537a52e3c49588ce116088061
Author: Chris Metcalf <cmetcalf@tilera.com>
Date: Mon Feb 10 10:54:47 2014 -0500
Move tilegx, tilepro, and linux-generic from ports to libc.
I've moved the TILE-Gx and TILEPro ports to the main sysdeps
hierarchy,
along with the linux-generic ports infrastructure. Beyond the
README
update, the move was just
git mv ports/sysdeps/tile sysdeps/tile
git mv ports/sysdeps/unix/sysv/linux/tile \
sysdeps/unix/sysv/linux/tile
git mv ports/sysdeps/unix/sysv/linux/generic \
sysdeps/unix/sysv/linux/generic
I updated the relevant ChangeLogs along the lines of the ARM move
in commit c6bfe5c4d75 and tested the 64-bit tilegx build to confirm
that
there were no changes in "objdump -dr" output in the shared objects.
But there is sysdeps/unix/sysv/linux/bits/statfs.h already. We should
update it if needed.
---
sysdeps/unix/sysv/linux/generic/bits/statfs.h | 86 ---------------------------
1 file changed, 86 deletions(-)
delete mode 100644 sysdeps/unix/sysv/linux/generic/bits/statfs.h
diff --git a/sysdeps/unix/sysv/linux/generic/bits/statfs.h b/sysdeps/unix/sysv/linux/generic/bits/statfs.h
deleted file mode 100644
index 363b7f8..0000000
--- a/sysdeps/unix/sysv/linux/generic/bits/statfs.h
+++ /dev/null
@@ -1,86 +0,0 @@
-/* Copyright (C) 2011-2015 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
- Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
-
- 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_STATFS_H
-# error "Never include <bits/statfs.h> directly; use <sys/statfs.h> instead."
-#endif
-
-#include <endian.h>
-#include <bits/types.h>
-#include <bits/wordsize.h>
-
-/* 64-bit libc uses the kernel's 'struct statfs', accessed via the
- statfs() syscall; 32-bit libc uses the kernel's 'struct statfs64'
- and accesses it via the statfs64() syscall. All the various
- APIs offered by libc use the kernel shape for their struct statfs
- structure; the only difference is that 32-bit programs not
- using __USE_FILE_OFFSET64 only see the low 32 bits of some
- of the fields (the __fsblkcnt_t and __fsfilcnt_t fields). */
-
-#if defined __USE_FILE_OFFSET64
-# define __field64(type, type64, name) type64 name
-#elif __WORDSIZE == 64
-# define __field64(type, type64, name) type name
-#elif __BYTE_ORDER == __LITTLE_ENDIAN
-# define __field64(type, type64, name) \
- type name __attribute__((__aligned__ (__alignof__ (type64)))); int __##name##_pad
-#else
-# define __field64(type, type64, name) \
- int __##name##_pad __attribute__((__aligned__ (__alignof__ (type64)))); type name
-#endif
-
-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];
- };
-
-#undef __field64
-
-#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];
- };
-#endif
-
-/* Tell code we have these members. */
-#define _STATFS_F_NAMELEN
-#define _STATFS_F_FRSIZE
-#define _STATFS_F_FLAGS
--
2.1.0