RFR: 8338884: java/nio/file/attribute/BasicFileAttributeView/CreationTime.java#tmp fails on alinux3 [v10]

[Severin Gehwolf]

On Fri, 6 Sep 2024 01:11:31 GMT, SendaoYan <syan at openjdk.org> wrote:

>> Hi all,
>> On alinux3(alibaba cloud linux version 3) system, the `/tmp` disk partition is mounted as tmpfs filesystem type, this filesystem type doesn't support create time(birth time).
>> 
>> Before this PR, this test [check](https://github.com/openjdk/jdk/blob/master/test/jdk/java/nio/file/attribute/BasicFileAttributeView/CreationTime.java#L110) if there is `statx` system call present or not to determise the test environment support birth time or not. I think it's not enough when the tested filesystem type is `tmpfs`. When the tested filesystem type is `tmpfs`, then the tested file doesn't support birth time.
>> 
>> On RHEL 8 tmpfs doesn't seem to support birth time, but on F39 tmpfs does seem to support birth time. Looks like this might be related to the kernel version. It's difficult to enumerate all the combination of file system type and linux kernel version to determine the testd file support birth time or not. So in this PR, I get the result from `statx` linux syscall, to determine the testd file support birth time or not.
>> 
>> Test fix only, the change has been verified, risk is low.
>> 
>> Additional test:
>> 
>> - [x] Alinux3 glibc:2.32
>> 1.  /tmp/file supportsCreationTimeRead == false
>> 2. ./file supportsCreationTimeRead == true
>> - [x] CentOS7 docker container glibc:2.17
>> 1.  /tmp/file supportsCreationTimeRead == false
>> 2. ./file supportsCreationTimeRead == false
>
> SendaoYan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix typo support to supported

test/jdk/java/nio/file/attribute/BasicFileAttributeView/CreationTime.java line 83:

> 81:             System.out.println("creationTime.toMillis() == " + creationTime.toMillis());
> 82:             // If the file system doesn't support birth time, then skip this test
> 83:             if (creationTime.toMillis() == 0) {

Is this still useful? After https://bugs.openjdk.org/browse/JDK-8338696 we'd never get the epoch (`0`). If birth time is not supported, the last modified time is being returned. I think this can go and the `SkippedException` branch removed as well.

test/jdk/java/nio/file/attribute/BasicFileAttributeView/CreationTimeHelper.java line 51:

> 49:             return (boolean)methodHandle.invokeExact(s);
> 50:         }
> 51:     }

We should only do the downcall if we know `statx` is available. I.e. return `false` early when `!abi.defaultLookup().find("statx").isPresent()`

test/jdk/java/nio/file/attribute/BasicFileAttributeView/libCreationTimeHelper.c line 30:

> 28:   #define true 1
> 29:   #define false 0
> 30: #endif

Do we really need this? Since https://openjdk.org/jeps/347 test libraries should be compiled with `--std=c11`.

test/jdk/java/nio/file/attribute/BasicFileAttributeView/libCreationTimeHelper.c line 46:

> 44: #ifndef _GNU_SOURCE
> 45: #define _GNU_SOURCE
> 46: #endif

Why is this needed?

test/jdk/java/nio/file/attribute/BasicFileAttributeView/libCreationTimeHelper.c line 121:

> 119:         return false;
> 120:     }
> 121:     if (stx.stx_mask & STATX_BTIME)

`if (stx.stx_mask & STAX_BTIME) != 0)` please. Also, please add a comment mentioning that "on some systems where `statx` is available birth time might still not be supported as it's file system specific. The only reliable way to check for support is looking at the filled in STATX_BTIME bit in the returned statx buffer mask."

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20687#discussion_r1746798514
PR Review Comment: https://git.openjdk.org/jdk/pull/20687#discussion_r1746745709
PR Review Comment: https://git.openjdk.org/jdk/pull/20687#discussion_r1746761279
PR Review Comment: https://git.openjdk.org/jdk/pull/20687#discussion_r1746779743
PR Review Comment: https://git.openjdk.org/jdk/pull/20687#discussion_r1746775412