Linux 4.11 compat: statx support #12432

ghost · 2021-07-26T16:26:03Z

Motivation and Context

Samba wants to know birthtime. We have it, we just need to fill it in.

Description

Rebase #8509 and add a simple test for ZTS.

How Has This Been Tested?

Added a test case to ZTS which stats a file for its birth time.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the OpenZFS code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

module/os/linux/zfs/zfs_vnops_os.c

adamdmoss · 2021-07-26T18:13:50Z

(See also: #8507 #8509 )

module/os/linux/zfs/zfs_vnops_os.c

allanjude

Reviewed By: Allan Jude [email protected]

ghost · 2021-07-26T19:21:48Z

I had not seen #8509 but that does seem like a better way of doing this. I'll go back and pick things up from there instead.

include/os/linux/zfs/sys/zfs_znode_impl.h

module/os/linux/zfs/zpl_inode.c

ghost · 2021-07-27T11:45:50Z

Should I worry about skipping the test on older kernels?

behlendorf

You can skip the new tests on older kernels in the same way it's done for functional/limits/filesystem_limit.ksh.

if is_linux; then
        if [[ $(linux_version) -lt $(linux_version "4.11") ]]; then
                log_unsupported "Requires statx(2) kernel support"
        fi
fi

module/os/linux/zfs/zpl_inode.c

ghost · 2021-07-27T20:47:27Z

Rebased
zp __maybe_unused in zfs_getattr_impl

behlendorf

Looks good, but I think you'll also need to add the new test case to zts-report.py.in to expect the test to be skipped on older kernels.

Linux 4.11 added a new statx system call that allows us to expose crtime as btime. We do this by caching crtime in the znode to match how atime, ctime and mtime are cached in the inode. statx also introduced a new way of reporting whether the immutable, append and nodump bits have been set. It adds support for reporting compression and encryption, but the semantics on other filesystems is not just to report compression/encryption, but to allow it to be turned on/off at the file level. We do not support that. We could implement semantics where we refuse to allow user modification of the bit, but we would need to do a dnode_hold() in zfs_znode_alloc() to find out encryption/compression information. That would introduce locking that will have a minor (although unmeasured) performance cost. It also would be inferior to zdb, which reports far more detailed information. We therefore omit reporting of encryption/compression through statx in favor of recommending that users interested in such information use zdb. Signed-off-by: Richard Yao <[email protected]> Closes openzfs#8507

ghost · 2021-07-28T15:03:27Z

Something seems to be missing in this implementation. Debian 10 has kernel 4.19 and genuinely failed the test:

22:30:09.58 Incorrect crtime (0 != 1627425009)

And it's not the only one to fail in this way.

ghost · 2021-07-30T16:17:54Z

The issue turns out to be that coreutils stat(1) doesn't support statx(2) until version 8.31, while buster provides version 8.30. I expect there is a similar reason for the other failed bots as well, so I'll see about checking the stat(1) version in the test as well.

Signed-off-by: Ryan Moeller <[email protected]>

ghost · 2021-07-30T16:31:34Z

Also check stat(1) version in crtime test

Reviewed-by: Tony Nguyen <[email protected]> Reviewed-by: Allan Jude <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Ryan Moeller <[email protected]> Closes openzfs#12432

Reviewed-by: Tony Nguyen <[email protected]> Reviewed-by: Allan Jude <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Ryan Moeller <[email protected]> Closes #12432

Reviewed-by: Tony Nguyen <[email protected]> Reviewed-by: Allan Jude <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Ryan Moeller <[email protected]> Closes openzfs#12432

amotin reviewed Jul 26, 2021

View reviewed changes

module/os/linux/zfs/zfs_vnops_os.c Outdated Show resolved Hide resolved

behlendorf requested changes Jul 26, 2021

View reviewed changes

module/os/linux/zfs/zfs_vnops_os.c Outdated Show resolved Hide resolved

behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 26, 2021

allanjude approved these changes Jul 26, 2021

View reviewed changes

ghost force-pushed the birthtime branch from 91f9388 to 877c43e Compare July 26, 2021 19:00

ghost force-pushed the birthtime branch from 877c43e to 6aade7b Compare July 26, 2021 20:19

ghost changed the title ~~Linux: Populate btime in zfs_getattr_fast~~ Linux 4.11 compat: statx support Jul 26, 2021

behlendorf reviewed Jul 26, 2021

View reviewed changes

include/os/linux/zfs/sys/zfs_znode_impl.h Outdated Show resolved Hide resolved

module/os/linux/zfs/zpl_inode.c Show resolved Hide resolved

module/os/linux/zfs/zpl_inode.c Show resolved Hide resolved

module/os/linux/zfs/zpl_inode.c Outdated Show resolved Hide resolved

ghost force-pushed the birthtime branch from 6aade7b to 5e48bfe Compare July 27, 2021 11:41

behlendorf reviewed Jul 27, 2021

View reviewed changes

module/os/linux/zfs/zpl_inode.c Show resolved Hide resolved

ghost force-pushed the birthtime branch 2 times, most recently from c5dbb39 to 607bf05 Compare July 27, 2021 20:44

ahrens assigned tonynguien Jul 27, 2021

ghost force-pushed the birthtime branch from 607bf05 to 6ddd7e3 Compare July 27, 2021 20:46

behlendorf approved these changes Jul 27, 2021

View reviewed changes

behlendorf added the Status: Work in Progress Not yet ready for general review label Jul 28, 2021

ghost force-pushed the birthtime branch from 6ddd7e3 to 5d903f1 Compare July 29, 2021 17:16

ZTS: Add tests for creation time

5ddd5c0

Signed-off-by: Ryan Moeller <[email protected]>

ghost force-pushed the birthtime branch from 5d903f1 to 5ddd5c0 Compare July 30, 2021 16:30

ghost removed the Status: Work in Progress Not yet ready for general review label Aug 2, 2021

ghost requested a review from tonynguien August 3, 2021 11:04

ghost mentioned this pull request Aug 5, 2021

NAS-109929: 21.08: Linux 4.11 compat: statx support truenas/zfs#18

Merged

amotin approved these changes Aug 5, 2021

View reviewed changes

tonynguien approved these changes Aug 5, 2021

View reviewed changes

tonynguien added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Aug 5, 2021

behlendorf closed this in 8ae86e2 Aug 17, 2021

ghost deleted the birthtime branch August 17, 2021 17:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Linux 4.11 compat: statx support #12432

Linux 4.11 compat: statx support #12432

Uh oh!

ghost commented Jul 26, 2021 •

edited by ghost

Loading

Uh oh!

Uh oh!

adamdmoss commented Jul 26, 2021

Uh oh!

Uh oh!

allanjude left a comment

Uh oh!

ghost commented Jul 26, 2021 •

edited by ghost

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Jul 27, 2021

Uh oh!

behlendorf left a comment

Uh oh!

Uh oh!

ghost commented Jul 27, 2021

Uh oh!

behlendorf left a comment

Uh oh!

ghost commented Jul 28, 2021

Uh oh!

ghost commented Jul 30, 2021 •

edited by ghost

Loading

Uh oh!

ghost commented Jul 30, 2021

Uh oh!

Uh oh!

Linux 4.11 compat: statx support #12432

Linux 4.11 compat: statx support #12432

Uh oh!

Conversation

ghost commented Jul 26, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

Uh oh!

adamdmoss commented Jul 26, 2021

Uh oh!

Uh oh!

allanjude left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Jul 26, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Jul 27, 2021

Uh oh!

behlendorf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ghost commented Jul 27, 2021

Uh oh!

behlendorf left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Jul 28, 2021

Uh oh!

ghost commented Jul 30, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jul 30, 2021

Uh oh!

Uh oh!

ghost commented Jul 26, 2021 •

edited by ghost

Loading

ghost commented Jul 26, 2021 •

edited by ghost

Loading

ghost commented Jul 30, 2021 •

edited by ghost

Loading