diff mbox

[v4,2/3] 32-bit ABIs: support stat syscall family

Message ID 20160830192853.GA5959@yury-N73SV
State New
Headers show

Commit Message

Yury Norov Aug. 30, 2016, 7:28 p.m. UTC
On Tue, Aug 30, 2016 at 05:15:44PM +0200, Arnd Bergmann wrote:
> On Tuesday 30 August 2016, Yury Norov wrote:
> > On Mon, Aug 29, 2016 at 10:34:38PM +0200, Arnd Bergmann wrote:
> > > On Monday 29 August 2016, Yury Norov wrote:
> > > > > 
> > > > > We've gone back and forth on the arm64+ilp32 definition of the
> > > > > existing stat64 , because that one cannot easily use the same
> > > > > generic structure. I originally thought that using the 64-bit
> > > > > layout of 'struct stat' was a good idea for arm64, but now
> > > > > I don't see much of an advantage either way (the existing arm32
> > > > > 'struct stat64' layout or the existing  arm64 'struct stat' layout).
> > > > > 
> > > > > Both of them are incompatible with the default layout for all
> > > > > other new 32-bit architectures in user space, so pick one of the
> > > > > two for the kernel interface, and convert it to the normal layout
> > > > > in glibc using __xstat_conv():
> > > > > 
> > > > > - if you use the arm64 'struct stat' layout (as in the current
> > > > >   kernel patches), you need a temporary buffer with the larger
> > > > >   size and then copy over the time fields one by one
> > > > > - if you instead use the arm32 'struct stat64' layout in the kernel,
> > > > >   the size is the same, and the conversion is a one-line
> > > > >   assignment of st_ino.
> > > > > 
> > > > >       Arnd
> > > > 
> > > > Ok. I'll do like this. I'd prefer 1st option, because 2nd implies
> > > > allocating big buffer on kernel side.
> > > 
> > > On the kernel side, both require allocating a buffer, the second
> > > one is just slightly bigger. The kernel uses its own internal
> > > 'struct kstat', which gets copied into one of the other structures.
> > > 
> > > 	Arnd
> > 
> > Ah, and in kernel too...
> > 
> > Anyway, using the arm32 'struct stat64' cannot help because
> > __xstat{,32,64}_conv() assumes copying of the kernel buffer to user
> > buffer.
> > 
> > We can, of course, write custom __xstat_conv() to avoid that copying. 
> 
> Right, that would work. No idea if that would be preferred in glibc
> or not: the existing __xstat_conv() obviously works already, while
> the simpler replacement would mean extra code.
> 
> > But then we'd write custom syscalls for stat family for aarch64/ilp32.
> > More simple and straightforward way then is to introduce custom layout
> > for struct stat, and avoid all conversions and memory wasting at all.
> > And this is what I suggested in the very first series of ilp32 support.
> > But Joseph declined it as non-generic solution.
> 
> Ok. 
> 
> > So for me there are 3 options:
> >  - __xstat_conv() - hacky, complex and ineffective. I work on it right
> >    now, and I'm not happy.
> >  - current version with conversion of time fields only - adds new
> >    fresh portion of hacks, but little more effective.
> 
> + new version with conversion of st_ino field only, a different new hack
>   but simpler.
> 
> >  - initial version. Effective, simple, free of hacks but (because of)
> >    non-generic.
> 
> I can't find that version now, can you post a copy of that original
> patch or a link to an archived discussion?
> 

It seems I never published it. It was based on arm32 syscall handlers,
and in kernel list it was decided to use aarch64 wrappers, so I
dropped it. Briefly, there was custom struct stat that was identical
to struct stat64, and identical to kernel one. The rest is like in
previous version except there were no code that converts timespecs.

> The layout we want is exactly what we have for stat64
> sysdeps/unix/sysv/linux/bits/stat.h (not
> sysdeps/unix/sysv/linux/generic/bits/stat.h), right?
> 
> 	Arnd

This is the implementation of fxstat() and lxstat() syscalls that use standard
struct stat layout, and convert kernel_stat to user struct stat with
__xstat{,64}_conv(). Here's used standard mechanism, but I had to move all
syscalls under platform directory. Se the body of the patch for details.

I see no reason to continue this work as it's both ineffective and non-generic.
Tomorrow I'll try the idea of Arnd - roll back to arm32 handlers in kernel, and
try arm32 wrappers in glibc, or custom wrappers to avoid useless copying. I
think there will be as much platform code, as in this version.

Notice that it was already discussed in linux mail list, and aarch64 syscalls
were called preferable because arm32 versions are broken (st_ino field), and
other reasons.

---
 sysdeps/unix/sysv/linux/aarch64/ilp32/fxstat.c     |  1 +
 sysdeps/unix/sysv/linux/aarch64/ilp32/fxstat64.c   | 46 +++++++++++++++
 sysdeps/unix/sysv/linux/aarch64/ilp32/fxstatat.c   |  1 +
 sysdeps/unix/sysv/linux/aarch64/ilp32/fxstatat64.c | 42 ++++++++++++++
 .../unix/sysv/linux/aarch64/ilp32/kernel_stat.h    | 25 ++++++++
 sysdeps/unix/sysv/linux/aarch64/ilp32/lxstat.c     | 40 +++++++++++++
 sysdeps/unix/sysv/linux/aarch64/ilp32/lxstatat64.c | 39 +++++++++++++
 sysdeps/unix/sysv/linux/aarch64/ilp32/xstatconv.c  | 66 ++++++++++++++++++++++
 8 files changed, 260 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/ilp32/fxstat.c
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/ilp32/fxstat64.c
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/ilp32/fxstatat.c
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/ilp32/fxstatat64.c
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/ilp32/kernel_stat.h
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/ilp32/lxstat.c
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/ilp32/lxstatat64.c
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/ilp32/xstatconv.c

Comments

Arnd Bergmann Aug. 30, 2016, 8:04 p.m. UTC | #1
On Tuesday 30 August 2016, Yury Norov wrote:
> On Tue, Aug 30, 2016 at 05:15:44PM +0200, Arnd Bergmann wrote:
> > On Tuesday 30 August 2016, Yury Norov wrote:
> > > On Mon, Aug 29, 2016 at 10:34:38PM +0200, Arnd Bergmann wrote:
> > 
> > + new version with conversion of st_ino field only, a different new hack
> >   but simpler.
> > 
> > >  - initial version. Effective, simple, free of hacks but (because of)
> > >    non-generic.
> > 
> > I can't find that version now, can you post a copy of that original
> > patch or a link to an archived discussion?
> > 
> 
> It seems I never published it. It was based on arm32 syscall handlers,
> and in kernel list it was decided to use aarch64 wrappers, so I
> dropped it. Briefly, there was custom struct stat that was identical
> to struct stat64, and identical to kernel one. The rest is like in
> previous version except there were no code that converts timespecs.
> 
> > The layout we want is exactly what we have for stat64
> > sysdeps/unix/sysv/linux/bits/stat.h (not
> > sysdeps/unix/sysv/linux/generic/bits/stat.h), right?
> > 
> > 	Arnd
> 
> This is the implementation of fxstat() and lxstat() syscalls that use standard
> struct stat layout, and convert kernel_stat to user struct stat with
> __xstat{,64}_conv(). Here's used standard mechanism, but I had to move all
> syscalls under platform directory. Se the body of the patch for details.
> 
> I see no reason to continue this work as it's both ineffective and non-generic.
> Tomorrow I'll try the idea of Arnd - roll back to arm32 handlers in kernel, and
> try arm32 wrappers in glibc, or custom wrappers to avoid useless copying. I
> think there will be as much platform code, as in this version.

Ok, sounds good.

> Notice that it was already discussed in linux mail list, and aarch64 syscalls
> were called preferable because arm32 versions are broken (st_ino field), and
> other reasons.

I'm sure I was agreeing to that at the time, but after the long discussions
we have had since, working around the st_ino problem seems much simpler than
working around the incompatible timestamps.

	Arnd
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstat.c b/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstat.c
new file mode 100644
index 0000000..6fa13ad
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstat.c
@@ -0,0 +1 @@ 
+#include <sysdeps/unix/sysv/linux/fxstat.c>
diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstat64.c b/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstat64.c
new file mode 100644
index 0000000..a970029
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstat64.c
@@ -0,0 +1,46 @@ 
+//#include <sysdeps/unix/sysv/linux/fxstat64.c>
+/* fxstat64 using Linux fstat64 system call.
+   Copyright (C) 1997-2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <stddef.h>
+#include <sys/stat.h>
+#include <kernel_stat.h>
+
+#include <sysdep.h>
+#include <sys/syscall.h>
+
+#include <kernel-features.h>
+#include <xstatconv.h>
+
+/* Get information about the file FD in BUF.  */
+
+int
+___fxstat64 (int vers, int fd, struct stat64 *buf)
+{
+  struct kernel_stat kbuf;
+  int result;
+
+  result = INLINE_SYSCALL (fstat64, 2, fd, &kbuf);
+  if (result)
+    return result;
+
+  return __xstat64_conv(vers, &kbuf, buf);
+}
+strong_alias (___fxstat64, __fxstat64)
+hidden_def (__fxstat64)
diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstatat.c b/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstatat.c
new file mode 100644
index 0000000..72806b9
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstatat.c
@@ -0,0 +1 @@ 
+#include <sysdeps/unix/sysv/linux/fxstatat.c>
diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstatat64.c b/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstatat64.c
new file mode 100644
index 0000000..8ee8063
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/fxstatat64.c
@@ -0,0 +1,42 @@ 
+/* Copyright (C) 2005-2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <kernel_stat.h>
+#include <xstatconv.h>
+
+#include <sysdep.h>
+#include <sys/syscall.h>
+
+/* Get information about the file NAME in BUF.  */
+int
+__fxstatat64 (int vers, int fd, const char *file, struct stat64 *st, int flag)
+{
+  int result;
+  struct kernel_stat kst;
+  result = INTERNAL_SYSCALL (fstatat64, err, 4, fd, file, &kst, flag);
+  if (result)
+    return 0;
+
+  return __xstat64_conv (vers, &kst, st);
+}
+libc_hidden_def (__fxstatat64)
diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/kernel_stat.h b/sysdeps/unix/sysv/linux/aarch64/ilp32/kernel_stat.h
new file mode 100644
index 0000000..9721bf6
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/kernel_stat.h
@@ -0,0 +1,25 @@ 
+#define __NR_fstat	__NR_fstat64
+#define __NR_newfstatat	__NR_fstatat64
+
+struct kernel_stat {
+	unsigned long long	st_dev;		/* Device.  */
+	unsigned long long	st_ino;		/* File serial number.  */
+	unsigned int		st_mode;	/* File mode.  */
+	unsigned int		st_nlink;	/* Link count.  */
+	unsigned int		st_uid;		/* User ID of the file's owner.  */
+	unsigned int		st_gid;		/* Group ID of the file's group. */
+	unsigned long long	st_rdev;	/* Device number, if device.  */
+	unsigned long long	__pad1;
+	long long		st_size;	/* Size of file, in bytes.  */
+	int			st_blksize;	/* Optimal block size for I/O.  */
+	int			__pad2;
+	long long		st_blocks;	/* Number 512-byte blocks allocated. */
+	long long		st_atime_sec;	/* Time of last access.  */
+	unsigned long long	st_atime_nsec;
+	long long		st_mtime_sec;	/* Time of last modification.  */
+	unsigned long long	st_mtime_nsec;
+	long long		st_ctime_sec;	/* Time of last status change.  */
+	unsigned long long	st_ctime_nsec;
+	unsigned int		__unused4;
+	unsigned int		__unused5;
+};
diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/lxstat.c b/sysdeps/unix/sysv/linux/aarch64/ilp32/lxstat.c
new file mode 100644
index 0000000..ff29cf7
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/lxstat.c
@@ -0,0 +1,40 @@ 
+/* Get information about the file NAME in BUF.
+
+   Copyright (C) 2011-2016 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/>.  */
+
+#include <errno.h>
+#include <stddef.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <kernel_stat.h>
+
+#include <sysdep.h>
+#include <sys/syscall.h>
+
+#include "xstatconv.h"
+
+int
+__lxstat (int vers, const char *name, struct stat *buf)
+{
+  struct kernel_stat kbuf;
+  int rc = INLINE_SYSCALL (fstatat64, 4, AT_FDCWD, name, &kbuf,
+			   AT_SYMLINK_NOFOLLOW);
+  return rc ?: __xstat_conv (vers, &kbuf, buf);
+}
+hidden_def (__lxstat)
diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/lxstatat64.c b/sysdeps/unix/sysv/linux/aarch64/ilp32/lxstatat64.c
new file mode 100644
index 0000000..36bb95f
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/lxstatat64.c
@@ -0,0 +1,39 @@ 
+/* lxstat64 using Linux lstat64 system call.
+   Copyright (C) 1997-2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <stddef.h>
+#include <sys/stat.h>
+#include <kernel_stat.h>
+
+#include <sysdep.h>
+#include <sys/syscall.h>
+
+#include "xstatconv.h"
+
+/* Get information about the file NAME in BUF.  */
+int
+___lxstat64 (int vers, const char *name, struct stat64 *buf)
+{
+  struct kernel_stat kbuf;
+  int rc = INLINE_SYSCALL (fstatat64, 4, AT_FDCWD, name, &kbuf,
+			   AT_SYMLINK_NOFOLLOW);
+  return rc ?: __xstat64_conv (vers, &kbuf, buf);
+}
+strong_alias (___lxstat64, __lxstat64);
+hidden_def (__lxstat64)
diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/xstatconv.c b/sysdeps/unix/sysv/linux/aarch64/ilp32/xstatconv.c
new file mode 100644
index 0000000..0f9709f
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/xstatconv.c
@@ -0,0 +1,66 @@ 
+/* Convert between the kernel's `struct stat' format, and libc's.
+   Copyright (C) 1991-2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <sys/stat.h>
+#include <kernel_stat.h>
+
+int
+__xstat_conv (int vers, struct kernel_stat *kbuf, struct stat *buf)
+{
+  buf->st_dev = kbuf->st_dev;
+  buf->st_ino = kbuf->st_ino;
+  buf->st_mode = kbuf->st_mode;
+  buf->st_nlink = kbuf->st_nlink;
+  buf->st_uid = kbuf->st_uid;
+  buf->st_gid = kbuf->st_gid;
+  buf->st_rdev = kbuf->st_rdev;
+  buf->st_size = kbuf->st_size;
+  buf->st_blksize = kbuf->st_blksize;
+  buf->st_blocks = kbuf->st_blocks;
+  buf->st_atim.tv_sec = kbuf->st_atime_sec;
+  buf->st_atim.tv_nsec = kbuf->st_atime_nsec;
+  buf->st_mtim.tv_sec = kbuf->st_mtime_sec;
+  buf->st_mtim.tv_nsec = kbuf->st_mtime_nsec;
+  buf->st_ctim.tv_sec = kbuf->st_ctime_sec;
+  buf->st_ctim.tv_nsec = kbuf->st_ctime_nsec;
+
+  return 0;
+}
+
+int
+__xstat64_conv (int vers, struct kernel_stat *kbuf, struct stat64 *buf)
+{
+  buf->st_dev = kbuf->st_dev;
+  buf->st_ino = kbuf->st_ino;
+  buf->st_mode = kbuf->st_mode;
+  buf->st_nlink = kbuf->st_nlink;
+  buf->st_uid = kbuf->st_uid;
+  buf->st_gid = kbuf->st_gid;
+  buf->st_rdev = kbuf->st_rdev;
+  buf->st_size = kbuf->st_size;
+  buf->st_blksize = kbuf->st_blksize;
+  buf->st_blocks = kbuf->st_blocks;
+  buf->st_atim.tv_sec = kbuf->st_atime_sec;
+  buf->st_atim.tv_nsec = kbuf->st_atime_nsec;
+  buf->st_mtim.tv_sec = kbuf->st_mtime_sec;
+  buf->st_mtim.tv_nsec = kbuf->st_mtime_nsec;
+  buf->st_ctim.tv_sec = kbuf->st_ctime_sec;
+  buf->st_ctim.tv_nsec = kbuf->st_ctime_nsec;
+
+  return 0;
+}