[Qemu-devel] [PATCH] linux-user: Pass through nanosecond timestamp components for stat syscalls

Chen-Yu Tsai posted 1 patch 10 weeks ago
Test asan passed
Test checkpatch failed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190514145346.20758-1-wens@kernel.org
Maintainers: Laurent Vivier <laurent@vivier.eu>, Riku Voipio <riku.voipio@iki.fi>
linux-user/syscall.c      | 18 ++++++++++++++++++
linux-user/syscall_defs.h | 36 ++++++++++++++++++------------------
2 files changed, 36 insertions(+), 18 deletions(-)

[Qemu-devel] [PATCH] linux-user: Pass through nanosecond timestamp components for stat syscalls

Posted by Chen-Yu Tsai 10 weeks ago
From: Chen-Yu Tsai <wens@csie.org>

Since Linux 2.6 the stat syscalls have mostly supported nanosecond
components for each of the file-related timestamps.

QEMU user mode emulation currently does not pass through the nanosecond
portion of the timestamp, even when the host system fills in the value.
This results in a mismatch when run on subsecond resolution filesystems
such as ext4 or XFS.

An example of this leading to inconsistency is cross-debootstraping a
full desktop root filesystem of Debian Buster. Recent versions of
fontconfig store the full timestamp (instead of just the second portion)
of the directory in its per-directory cache file, and checks this against
the directory to see if the cache is up-to-date. With QEMU user mode
emulation, the timestamp stored is incorrect, and upon booting the rootfs
natively, fontconfig discovers the mismatch, and proceeds to rebuild the
cache on the comparatively slow machine (low-power ARM vs x86). This
stalls the first attempt to open whatever application that incorporates
fontconfig.

This patch renames the "unused" padding trailing each timestamp element
to its nanosecond counterpart name if such an element exists in the
kernel sources for the given platform. Not all do. Then have the syscall
wrapper fill in the nanosecond portion if the host supports it, as
specified by the _POSIX_C_SOURCE and _XOPEN_SOURCE feature macros.

Recent versions of glibc only use stat64 and newfstatat syscalls on
32-bit and 64-bit platforms respectively. The changes in this patch
were tested by directly calling the stat, stat64 and newfstatat syscalls
directly, in addition to the glibc wrapper, on arm and aarch64 little
endian targets.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>

---

This issue was found while integrating some software that uses newer
versions of fontconfig into Raspbian images. We found that the first
launch of said software always stalls with fontconfig regenerating its
font cache files. Upon closer examination I found the timestamps were
not matching. The rest is explained above. Currently we're just working
around the problem by patching the correct timestamps into the cache
files after the fact.

Please consider this a drive-by scratch-my-own-itch contribution, but I
will stick around to deal with any comments raised during review. I'm
not on the mailing lists either, so please keep me in CC.

checkpatch returns "ERROR: code indent should never use tabs" for
linux-user/syscall_defs.h, however as far as I can tell the whole file
is indented with tabs. I'm not sure what to make of this.

Finally, I think this could be worth backporting to older versions.
---
 linux-user/syscall.c      | 18 ++++++++++++++++++
 linux-user/syscall_defs.h | 36 ++++++++++++++++++------------------
 2 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f5ff6f5dc8..dcd6f5d806 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6408,6 +6408,11 @@ static inline abi_long host_to_target_stat64(void *cpu_env,
         __put_user(host_st->st_atime, &target_st->target_st_atime);
         __put_user(host_st->st_mtime, &target_st->target_st_mtime);
         __put_user(host_st->st_ctime, &target_st->target_st_ctime);
+#if _POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700
+        __put_user(host_st->st_atim.tv_nsec, &target_st->target_st_atime_nsec);
+        __put_user(host_st->st_mtim.tv_nsec, &target_st->target_st_mtime_nsec);
+        __put_user(host_st->st_ctim.tv_nsec, &target_st->target_st_ctime_nsec);
+#endif
         unlock_user_struct(target_st, target_addr, 1);
     } else
 #endif
@@ -6438,6 +6443,11 @@ static inline abi_long host_to_target_stat64(void *cpu_env,
         __put_user(host_st->st_atime, &target_st->target_st_atime);
         __put_user(host_st->st_mtime, &target_st->target_st_mtime);
         __put_user(host_st->st_ctime, &target_st->target_st_ctime);
+#if _POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700
+        __put_user(host_st->st_atim.tv_nsec, &target_st->target_st_atime_nsec);
+        __put_user(host_st->st_mtim.tv_nsec, &target_st->target_st_mtime_nsec);
+        __put_user(host_st->st_ctim.tv_nsec, &target_st->target_st_ctime_nsec);
+#endif
         unlock_user_struct(target_st, target_addr, 1);
     }
 
@@ -8866,6 +8876,14 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
                 __put_user(st.st_atime, &target_st->target_st_atime);
                 __put_user(st.st_mtime, &target_st->target_st_mtime);
                 __put_user(st.st_ctime, &target_st->target_st_ctime);
+#if _POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700
+                __put_user(st.st_atim.tv_nsec,
+                           &target_st->target_st_atime_nsec);
+                __put_user(st.st_mtim.tv_nsec,
+                           &target_st->target_st_mtime_nsec);
+                __put_user(st.st_ctim.tv_nsec,
+                           &target_st->target_st_ctime_nsec);
+#endif
                 unlock_user_struct(target_st, arg2, 1);
             }
         }
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 12c8407144..252e69b76e 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1192,11 +1192,11 @@ struct target_stat {
 	abi_ulong  st_blksize;
 	abi_ulong  st_blocks;
 	abi_ulong  target_st_atime;
-	abi_ulong  __unused1;
+	abi_ulong  target_st_atime_nsec;
 	abi_ulong  target_st_mtime;
-	abi_ulong  __unused2;
+	abi_ulong  target_st_mtime_nsec;
 	abi_ulong  target_st_ctime;
-	abi_ulong  __unused3;
+	abi_ulong  target_st_ctime_nsec;
 	abi_ulong  __unused4;
 	abi_ulong  __unused5;
 };
@@ -1228,13 +1228,13 @@ struct target_stat64 {
 	abi_ulong	__pad4;		/* future possible st_blocks high bits */
 
 	abi_ulong	target_st_atime;
-	abi_ulong	__pad5;
+	abi_ulong	target_st_atime_nsec;
 
 	abi_ulong	target_st_mtime;
-	abi_ulong	__pad6;
+	abi_ulong	target_st_mtime_nsec;
 
 	abi_ulong	target_st_ctime;
-	abi_ulong	__pad7;		/* will be high 32 bits of ctime someday */
+	abi_ulong	target_st_ctime_nsec;
 
 	unsigned long long	st_ino;
 } QEMU_PACKED;
@@ -1313,13 +1313,13 @@ struct target_stat64 {
 	unsigned int	st_blocks;
 
 	abi_ulong	target_st_atime;
-	abi_ulong	__unused1;
+	abi_ulong	target_st_atime_nsec;
 
 	abi_ulong	target_st_mtime;
-	abi_ulong	__unused2;
+	abi_ulong	target_st_mtime_nsec;
 
 	abi_ulong	target_st_ctime;
-	abi_ulong	__unused3;
+	abi_ulong	target_st_ctime_nsec;
 
 	abi_ulong	__unused4[3];
 };
@@ -1336,14 +1336,14 @@ struct target_stat {
 	unsigned short	st_rdev;
 	abi_long	st_size;
 	abi_long	target_st_atime;
-	abi_ulong	__unused1;
+	abi_ulong	target_st_atime_nsec;
 	abi_long	target_st_mtime;
-	abi_ulong	__unused2;
+	abi_ulong	target_st_mtime_nsec;
 	abi_long	target_st_ctime;
-	abi_ulong	__unused3;
+	abi_ulong	target_st_ctime_nsec;
 	abi_long	st_blksize;
 	abi_long	st_blocks;
-	abi_ulong	__unused4[2];
+	abi_ulong	__unused1[2];
 };
 
 #define TARGET_HAS_STRUCT_STAT64
@@ -1371,16 +1371,16 @@ struct target_stat64 {
 	unsigned int	st_blocks;
 
 	unsigned int	target_st_atime;
-	unsigned int	__unused1;
+	unsigned int	target_st_atime_nsec;
 
 	unsigned int	target_st_mtime;
-	unsigned int	__unused2;
+	unsigned int	target_st_mtime_nsec;
 
 	unsigned int	target_st_ctime;
-	unsigned int	__unused3;
+	unsigned int	target_st_ctime_nsec;
 
-	unsigned int	__unused4;
-	unsigned int	__unused5;
+	unsigned int	__unused1;
+	unsigned int	__unused2;
 };
 
 #elif defined(TARGET_PPC)
-- 
2.20.1


Re: [Qemu-devel] [PATCH] linux-user: Pass through nanosecond timestamp components for stat syscalls

Posted by Laurent Vivier 8 weeks ago
On 14/05/2019 16:53, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> Since Linux 2.6 the stat syscalls have mostly supported nanosecond
> components for each of the file-related timestamps.
> 
> QEMU user mode emulation currently does not pass through the nanosecond
> portion of the timestamp, even when the host system fills in the value.
> This results in a mismatch when run on subsecond resolution filesystems
> such as ext4 or XFS.
> 
> An example of this leading to inconsistency is cross-debootstraping a
> full desktop root filesystem of Debian Buster. Recent versions of
> fontconfig store the full timestamp (instead of just the second portion)
> of the directory in its per-directory cache file, and checks this against
> the directory to see if the cache is up-to-date. With QEMU user mode
> emulation, the timestamp stored is incorrect, and upon booting the rootfs
> natively, fontconfig discovers the mismatch, and proceeds to rebuild the
> cache on the comparatively slow machine (low-power ARM vs x86). This
> stalls the first attempt to open whatever application that incorporates
> fontconfig.
> 
> This patch renames the "unused" padding trailing each timestamp element
> to its nanosecond counterpart name if such an element exists in the
> kernel sources for the given platform. Not all do. Then have the syscall
> wrapper fill in the nanosecond portion if the host supports it, as
> specified by the _POSIX_C_SOURCE and _XOPEN_SOURCE feature macros.
> 
> Recent versions of glibc only use stat64 and newfstatat syscalls on
> 32-bit and 64-bit platforms respectively. The changes in this patch
> were tested by directly calling the stat, stat64 and newfstatat syscalls
> directly, in addition to the glibc wrapper, on arm and aarch64 little
> endian targets.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> 
> ---
> 
> This issue was found while integrating some software that uses newer
> versions of fontconfig into Raspbian images. We found that the first
> launch of said software always stalls with fontconfig regenerating its
> font cache files. Upon closer examination I found the timestamps were
> not matching. The rest is explained above. Currently we're just working
> around the problem by patching the correct timestamps into the cache
> files after the fact.
> 
> Please consider this a drive-by scratch-my-own-itch contribution, but I
> will stick around to deal with any comments raised during review. I'm
> not on the mailing lists either, so please keep me in CC.
> 
> checkpatch returns "ERROR: code indent should never use tabs" for
> linux-user/syscall_defs.h, however as far as I can tell the whole file
> is indented with tabs. I'm not sure what to make of this.

Yes, the file is entirely indented with tabs, so you can let this as-is.
Anyway, I plan to split the file in several ones so we will be able to 
swap the tabs with spaces.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

> 
> Finally, I think this could be worth backporting to older versions.
> ---
>   linux-user/syscall.c      | 18 ++++++++++++++++++
>   linux-user/syscall_defs.h | 36 ++++++++++++++++++------------------
>   2 files changed, 36 insertions(+), 18 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index f5ff6f5dc8..dcd6f5d806 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -6408,6 +6408,11 @@ static inline abi_long host_to_target_stat64(void *cpu_env,
>           __put_user(host_st->st_atime, &target_st->target_st_atime);
>           __put_user(host_st->st_mtime, &target_st->target_st_mtime);
>           __put_user(host_st->st_ctime, &target_st->target_st_ctime);
> +#if _POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700
> +        __put_user(host_st->st_atim.tv_nsec, &target_st->target_st_atime_nsec);
> +        __put_user(host_st->st_mtim.tv_nsec, &target_st->target_st_mtime_nsec);
> +        __put_user(host_st->st_ctim.tv_nsec, &target_st->target_st_ctime_nsec);
> +#endif
>           unlock_user_struct(target_st, target_addr, 1);
>       } else
>   #endif
> @@ -6438,6 +6443,11 @@ static inline abi_long host_to_target_stat64(void *cpu_env,
>           __put_user(host_st->st_atime, &target_st->target_st_atime);
>           __put_user(host_st->st_mtime, &target_st->target_st_mtime);
>           __put_user(host_st->st_ctime, &target_st->target_st_ctime);
> +#if _POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700
> +        __put_user(host_st->st_atim.tv_nsec, &target_st->target_st_atime_nsec);
> +        __put_user(host_st->st_mtim.tv_nsec, &target_st->target_st_mtime_nsec);
> +        __put_user(host_st->st_ctim.tv_nsec, &target_st->target_st_ctime_nsec);
> +#endif
>           unlock_user_struct(target_st, target_addr, 1);
>       }
>   
> @@ -8866,6 +8876,14 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>                   __put_user(st.st_atime, &target_st->target_st_atime);
>                   __put_user(st.st_mtime, &target_st->target_st_mtime);
>                   __put_user(st.st_ctime, &target_st->target_st_ctime);
> +#if _POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700
> +                __put_user(st.st_atim.tv_nsec,
> +                           &target_st->target_st_atime_nsec);
> +                __put_user(st.st_mtim.tv_nsec,
> +                           &target_st->target_st_mtime_nsec);
> +                __put_user(st.st_ctim.tv_nsec,
> +                           &target_st->target_st_ctime_nsec);
> +#endif
>                   unlock_user_struct(target_st, arg2, 1);
>               }
>           }
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 12c8407144..252e69b76e 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -1192,11 +1192,11 @@ struct target_stat {
>   	abi_ulong  st_blksize;
>   	abi_ulong  st_blocks;
>   	abi_ulong  target_st_atime;
> -	abi_ulong  __unused1;
> +	abi_ulong  target_st_atime_nsec;
>   	abi_ulong  target_st_mtime;
> -	abi_ulong  __unused2;
> +	abi_ulong  target_st_mtime_nsec;
>   	abi_ulong  target_st_ctime;
> -	abi_ulong  __unused3;
> +	abi_ulong  target_st_ctime_nsec;
>   	abi_ulong  __unused4;
>   	abi_ulong  __unused5;
>   };
> @@ -1228,13 +1228,13 @@ struct target_stat64 {
>   	abi_ulong	__pad4;		/* future possible st_blocks high bits */
>   
>   	abi_ulong	target_st_atime;
> -	abi_ulong	__pad5;
> +	abi_ulong	target_st_atime_nsec;
>   
>   	abi_ulong	target_st_mtime;
> -	abi_ulong	__pad6;
> +	abi_ulong	target_st_mtime_nsec;
>   
>   	abi_ulong	target_st_ctime;
> -	abi_ulong	__pad7;		/* will be high 32 bits of ctime someday */
> +	abi_ulong	target_st_ctime_nsec;
>   
>   	unsigned long long	st_ino;
>   } QEMU_PACKED;
> @@ -1313,13 +1313,13 @@ struct target_stat64 {
>   	unsigned int	st_blocks;
>   
>   	abi_ulong	target_st_atime;
> -	abi_ulong	__unused1;
> +	abi_ulong	target_st_atime_nsec;
>   
>   	abi_ulong	target_st_mtime;
> -	abi_ulong	__unused2;
> +	abi_ulong	target_st_mtime_nsec;
>   
>   	abi_ulong	target_st_ctime;
> -	abi_ulong	__unused3;
> +	abi_ulong	target_st_ctime_nsec;
>   
>   	abi_ulong	__unused4[3];
>   };
> @@ -1336,14 +1336,14 @@ struct target_stat {
>   	unsigned short	st_rdev;
>   	abi_long	st_size;
>   	abi_long	target_st_atime;
> -	abi_ulong	__unused1;
> +	abi_ulong	target_st_atime_nsec;
>   	abi_long	target_st_mtime;
> -	abi_ulong	__unused2;
> +	abi_ulong	target_st_mtime_nsec;
>   	abi_long	target_st_ctime;
> -	abi_ulong	__unused3;
> +	abi_ulong	target_st_ctime_nsec;
>   	abi_long	st_blksize;
>   	abi_long	st_blocks;
> -	abi_ulong	__unused4[2];
> +	abi_ulong	__unused1[2];
>   };
>   
>   #define TARGET_HAS_STRUCT_STAT64
> @@ -1371,16 +1371,16 @@ struct target_stat64 {
>   	unsigned int	st_blocks;
>   
>   	unsigned int	target_st_atime;
> -	unsigned int	__unused1;
> +	unsigned int	target_st_atime_nsec;
>   
>   	unsigned int	target_st_mtime;
> -	unsigned int	__unused2;
> +	unsigned int	target_st_mtime_nsec;
>   
>   	unsigned int	target_st_ctime;
> -	unsigned int	__unused3;
> +	unsigned int	target_st_ctime_nsec;
>   
> -	unsigned int	__unused4;
> -	unsigned int	__unused5;
> +	unsigned int	__unused1;
> +	unsigned int	__unused2;
>   };
>   
>   #elif defined(TARGET_PPC)
> 


Re: [Qemu-devel] [PATCH] linux-user: Pass through nanosecond timestamp components for stat syscalls

Posted by Chen-Yu Tsai 8 weeks ago
On Wed, May 22, 2019 at 5:08 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> On 14/05/2019 16:53, Chen-Yu Tsai wrote:
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > Since Linux 2.6 the stat syscalls have mostly supported nanosecond
> > components for each of the file-related timestamps.
> >
> > QEMU user mode emulation currently does not pass through the nanosecond
> > portion of the timestamp, even when the host system fills in the value.
> > This results in a mismatch when run on subsecond resolution filesystems
> > such as ext4 or XFS.
> >
> > An example of this leading to inconsistency is cross-debootstraping a
> > full desktop root filesystem of Debian Buster. Recent versions of
> > fontconfig store the full timestamp (instead of just the second portion)
> > of the directory in its per-directory cache file, and checks this against
> > the directory to see if the cache is up-to-date. With QEMU user mode
> > emulation, the timestamp stored is incorrect, and upon booting the rootfs
> > natively, fontconfig discovers the mismatch, and proceeds to rebuild the
> > cache on the comparatively slow machine (low-power ARM vs x86). This
> > stalls the first attempt to open whatever application that incorporates
> > fontconfig.
> >
> > This patch renames the "unused" padding trailing each timestamp element
> > to its nanosecond counterpart name if such an element exists in the
> > kernel sources for the given platform. Not all do. Then have the syscall
> > wrapper fill in the nanosecond portion if the host supports it, as
> > specified by the _POSIX_C_SOURCE and _XOPEN_SOURCE feature macros.
> >
> > Recent versions of glibc only use stat64 and newfstatat syscalls on
> > 32-bit and 64-bit platforms respectively. The changes in this patch
> > were tested by directly calling the stat, stat64 and newfstatat syscalls
> > directly, in addition to the glibc wrapper, on arm and aarch64 little
> > endian targets.
> >
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >
> > ---
> >
> > This issue was found while integrating some software that uses newer
> > versions of fontconfig into Raspbian images. We found that the first
> > launch of said software always stalls with fontconfig regenerating its
> > font cache files. Upon closer examination I found the timestamps were
> > not matching. The rest is explained above. Currently we're just working
> > around the problem by patching the correct timestamps into the cache
> > files after the fact.
> >
> > Please consider this a drive-by scratch-my-own-itch contribution, but I
> > will stick around to deal with any comments raised during review. I'm
> > not on the mailing lists either, so please keep me in CC.
> >
> > checkpatch returns "ERROR: code indent should never use tabs" for
> > linux-user/syscall_defs.h, however as far as I can tell the whole file
> > is indented with tabs. I'm not sure what to make of this.
>
> Yes, the file is entirely indented with tabs, so you can let this as-is.
> Anyway, I plan to split the file in several ones so we will be able to
> swap the tabs with spaces.
>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Thanks. Unfortunately this patch has some issues. It fails to build for
targets that don't have the *_nsec fields, such as Alpha or M68K.

I'll spin a v2 with a new macro TARGET_STAT_HAS_NSEC defined for targets
that have the fields, added before each struct stat definition. The hunk
below will gain a check against said macro. This is pretty much how the
kernel deals with the difference as well, as I just found out.

> > @@ -8866,6 +8876,14 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
> >                   __put_user(st.st_atime, &target_st->target_st_atime);
> >                   __put_user(st.st_mtime, &target_st->target_st_mtime);
> >                   __put_user(st.st_ctime, &target_st->target_st_ctime);
> > +#if _POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700
> > +                __put_user(st.st_atim.tv_nsec,
> > +                           &target_st->target_st_atime_nsec);
> > +                __put_user(st.st_mtim.tv_nsec,
> > +                           &target_st->target_st_mtime_nsec);
> > +                __put_user(st.st_ctim.tv_nsec,
> > +                           &target_st->target_st_ctime_nsec);
> > +#endif
> >                   unlock_user_struct(target_st, arg2, 1);
> >               }
> >           }

If that sounds good to you I'll keep your reviewed-by for v2.

Thanks
ChenYu

Re: [Qemu-devel] [PATCH] linux-user: Pass through nanosecond timestamp components for stat syscalls

Posted by Laurent Vivier 8 weeks ago
On 22/05/2019 11:45, Chen-Yu Tsai wrote:
> On Wed, May 22, 2019 at 5:08 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> On 14/05/2019 16:53, Chen-Yu Tsai wrote:
>>> From: Chen-Yu Tsai <wens@csie.org>
>>>
>>> Since Linux 2.6 the stat syscalls have mostly supported nanosecond
>>> components for each of the file-related timestamps.
>>>
>>> QEMU user mode emulation currently does not pass through the nanosecond
>>> portion of the timestamp, even when the host system fills in the value.
>>> This results in a mismatch when run on subsecond resolution filesystems
>>> such as ext4 or XFS.
>>>
>>> An example of this leading to inconsistency is cross-debootstraping a
>>> full desktop root filesystem of Debian Buster. Recent versions of
>>> fontconfig store the full timestamp (instead of just the second portion)
>>> of the directory in its per-directory cache file, and checks this against
>>> the directory to see if the cache is up-to-date. With QEMU user mode
>>> emulation, the timestamp stored is incorrect, and upon booting the rootfs
>>> natively, fontconfig discovers the mismatch, and proceeds to rebuild the
>>> cache on the comparatively slow machine (low-power ARM vs x86). This
>>> stalls the first attempt to open whatever application that incorporates
>>> fontconfig.
>>>
>>> This patch renames the "unused" padding trailing each timestamp element
>>> to its nanosecond counterpart name if such an element exists in the
>>> kernel sources for the given platform. Not all do. Then have the syscall
>>> wrapper fill in the nanosecond portion if the host supports it, as
>>> specified by the _POSIX_C_SOURCE and _XOPEN_SOURCE feature macros.
>>>
>>> Recent versions of glibc only use stat64 and newfstatat syscalls on
>>> 32-bit and 64-bit platforms respectively. The changes in this patch
>>> were tested by directly calling the stat, stat64 and newfstatat syscalls
>>> directly, in addition to the glibc wrapper, on arm and aarch64 little
>>> endian targets.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>>
>>> ---
>>>
>>> This issue was found while integrating some software that uses newer
>>> versions of fontconfig into Raspbian images. We found that the first
>>> launch of said software always stalls with fontconfig regenerating its
>>> font cache files. Upon closer examination I found the timestamps were
>>> not matching. The rest is explained above. Currently we're just working
>>> around the problem by patching the correct timestamps into the cache
>>> files after the fact.
>>>
>>> Please consider this a drive-by scratch-my-own-itch contribution, but I
>>> will stick around to deal with any comments raised during review. I'm
>>> not on the mailing lists either, so please keep me in CC.
>>>
>>> checkpatch returns "ERROR: code indent should never use tabs" for
>>> linux-user/syscall_defs.h, however as far as I can tell the whole file
>>> is indented with tabs. I'm not sure what to make of this.
>>
>> Yes, the file is entirely indented with tabs, so you can let this as-is.
>> Anyway, I plan to split the file in several ones so we will be able to
>> swap the tabs with spaces.
>>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> 
> Thanks. Unfortunately this patch has some issues. It fails to build for
> targets that don't have the *_nsec fields, such as Alpha or M68K.

ok, anyway I always build all linux-user target before my PR, I would 
have seend the problem :)

> 
> I'll spin a v2 with a new macro TARGET_STAT_HAS_NSEC defined for targets
> that have the fields, added before each struct stat definition. The hunk
> below will gain a check against said macro. This is pretty much how the
> kernel deals with the difference as well, as I just found out.
> 
>>> @@ -8866,6 +8876,14 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>>>                    __put_user(st.st_atime, &target_st->target_st_atime);
>>>                    __put_user(st.st_mtime, &target_st->target_st_mtime);
>>>                    __put_user(st.st_ctime, &target_st->target_st_ctime);
>>> +#if _POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700
>>> +                __put_user(st.st_atim.tv_nsec,
>>> +                           &target_st->target_st_atime_nsec);
>>> +                __put_user(st.st_mtim.tv_nsec,
>>> +                           &target_st->target_st_mtime_nsec);
>>> +                __put_user(st.st_ctim.tv_nsec,
>>> +                           &target_st->target_st_ctime_nsec);
>>> +#endif
>>>                    unlock_user_struct(target_st, arg2, 1);
>>>                }
>>>            }
> 
> If that sounds good to you I'll keep your reviewed-by for v2.

You can.

Thanks,
Laurent


Re: [Qemu-devel] [PATCH] linux-user: Pass through nanosecond timestamp components for stat syscalls

Posted by no-reply@patchew.org 10 weeks ago
Patchew URL: https://patchew.org/QEMU/20190514145346.20758-1-wens@kernel.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20190514145346.20758-1-wens@kernel.org
Subject: [Qemu-devel] [PATCH] linux-user: Pass through nanosecond timestamp components for stat syscalls

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
ecae7ee linux-user: Pass through nanosecond timestamp components for stat syscalls

=== OUTPUT BEGIN ===
ERROR: code indent should never use tabs
#94: FILE: linux-user/syscall_defs.h:1195:
+^Iabi_ulong  target_st_atime_nsec;$

ERROR: code indent should never use tabs
#97: FILE: linux-user/syscall_defs.h:1197:
+^Iabi_ulong  target_st_mtime_nsec;$

ERROR: code indent should never use tabs
#100: FILE: linux-user/syscall_defs.h:1199:
+^Iabi_ulong  target_st_ctime_nsec;$

ERROR: code indent should never use tabs
#109: FILE: linux-user/syscall_defs.h:1231:
+^Iabi_ulong^Itarget_st_atime_nsec;$

ERROR: code indent should never use tabs
#113: FILE: linux-user/syscall_defs.h:1234:
+^Iabi_ulong^Itarget_st_mtime_nsec;$

ERROR: code indent should never use tabs
#117: FILE: linux-user/syscall_defs.h:1237:
+^Iabi_ulong^Itarget_st_ctime_nsec;$

ERROR: code indent should never use tabs
#126: FILE: linux-user/syscall_defs.h:1316:
+^Iabi_ulong^Itarget_st_atime_nsec;$

ERROR: code indent should never use tabs
#130: FILE: linux-user/syscall_defs.h:1319:
+^Iabi_ulong^Itarget_st_mtime_nsec;$

ERROR: code indent should never use tabs
#134: FILE: linux-user/syscall_defs.h:1322:
+^Iabi_ulong^Itarget_st_ctime_nsec;$

ERROR: code indent should never use tabs
#143: FILE: linux-user/syscall_defs.h:1339:
+^Iabi_ulong^Itarget_st_atime_nsec;$

ERROR: code indent should never use tabs
#146: FILE: linux-user/syscall_defs.h:1341:
+^Iabi_ulong^Itarget_st_mtime_nsec;$

ERROR: code indent should never use tabs
#149: FILE: linux-user/syscall_defs.h:1343:
+^Iabi_ulong^Itarget_st_ctime_nsec;$

ERROR: code indent should never use tabs
#153: FILE: linux-user/syscall_defs.h:1346:
+^Iabi_ulong^I__unused1[2];$

ERROR: code indent should never use tabs
#162: FILE: linux-user/syscall_defs.h:1374:
+^Iunsigned int^Itarget_st_atime_nsec;$

ERROR: code indent should never use tabs
#166: FILE: linux-user/syscall_defs.h:1377:
+^Iunsigned int^Itarget_st_mtime_nsec;$

ERROR: code indent should never use tabs
#170: FILE: linux-user/syscall_defs.h:1380:
+^Iunsigned int^Itarget_st_ctime_nsec;$

ERROR: code indent should never use tabs
#174: FILE: linux-user/syscall_defs.h:1382:
+^Iunsigned int^I__unused1;$

ERROR: code indent should never use tabs
#175: FILE: linux-user/syscall_defs.h:1383:
+^Iunsigned int^I__unused2;$

total: 18 errors, 0 warnings, 121 lines checked

Commit ecae7ee00f55 (linux-user: Pass through nanosecond timestamp components for stat syscalls) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190514145346.20758-1-wens@kernel.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com